A developer was porting some old Visual Studio code to another platform and found that the code was behavior strangely. Here’s a simplified version of the code: <\/p>\n
\nclass refarray\n{\npublic:\n refarray(int length)\n {\n m_array = new int*[length];\n for (int i = 0; i < length; i++) {\n m_array[i] = NULL;\n }\n }\n\n int& operator[](int i)\n {\n return *m_array[i];\n }\n\n ... other members not relevant here...\n\nprivate:\n int** m_array;\n};\n<\/pre>\nThis class is an array of references to integers. Each slot starts out uninitialized, but you can use methods (not shown here) to make each slot point to a particular integer, and you use the array indexing operator to access the referenced integer. (You can tell this is old code because it’s not using unique_ptr<\/code> or reference_wrapper<\/code> or nullptr<\/code>.) <\/p>\nHere’s some typical code that didn’t work: <\/p>\n
\nrefarray frameCounts(NUM_RENDERERS);\n\nvoid refresh(int* frameCount)\n{\n .. a bunch of refresh code ..\n if (frameCount != NULL) ++*frameCount;\n}\n\nvoid refresh_and_count(int i)\n{\n refresh(&frameCounts[i]);\n}\n<\/pre>\nThe refresh<\/code> function performs a refresh and if the pointer is non-null, it assumes it’s a frame count and increments it. The refresh_<\/code>and_<\/code>count<\/code> function uses the refresh<\/code> function to perform an update and then increment the optional frame counter stored in the frameCounts<\/code> object. <\/p>\nThe developer found that if the slot was not set, the code crashed with a null pointer access violation at the ++*frameCount<\/code>, despite the code explicitly checking if (frameCount != NULL)<\/code> immediately prior. <\/p>\nFurther investigation showed that the code worked fine with optimization disabled, but once they started turning on optimizations, the null pointer check stopped working. <\/p>\n
The developer fell into the trap of the null reference, or more generally, the fact that undefined behavior can have strange effects. <\/p>\n
In the C++ language, there is no such thing as a null reference. All references are to valid objects. The expression frameCounts[i]<\/code> produces a reference, and therefore the expression &frameCounts[i]<\/code> can never legally produce a null pointer. The compiler optimized out the null test because it could prove that the resulting pointer could never legally be null. <\/p>\nThe code worked on the very old of Visual Studio because very old Visual Studio compilers did not implement this optimization. They generated the pointer and redundantly tested it against null, even though the only way to generate such a null pointer was to break one of the rules of the language. <\/p>\n
The new compiler on that other platform took advantage of the optimization: After one level of inlining, the compiler noticed that the pointer could not be null, so it removed the test. <\/p>\n
The fix is to repair the code so it doesn’t generate null references. <\/p>\n
I know that people will complain that the compiler should not be removing reundant tests, because the person who wrote the code presumably wrote the redundant tests for a reason. Or at least if the compiler removed the redundant test, it should emit a warning: “Removing provably false test.” <\/p>\n
But on the other hand, surely you would want the compiler to optimize out the test when you call it like this: <\/p>\n
\nint counter;\nvoid something()\n{\n refresh(&counter);\n}\n<\/pre>\nThis is another case where the pointer passed to refresh<\/code> is provably non-null. Do you want the compiler to generate the test anyway? If not, then it would presumably generate the “Removing provably false test” warning. Your code would probably generate tons of instances of this warning, and none of your options look appealing. <\/p>\nOne option is to duplicate the refresh<\/code> function into one version that supports a null pointer (and performs the test), and another version that requires a non-null pointer (and doesn’t perform the test). This sort of change can quickly infect your entire code, because callers of refresh<\/code> might in turn need to split into two versions, and pretty soon you have two versions of half of your program. <\/p>\n<\/p>\n The other option is to suppress the warning. <\/p>\n
In practice, you’re probably going to go for the second option. <\/p>\n
But there’s clearly no point in the compiler team implementing a warning that everybody suppresses. <\/p>\n","protected":false},"excerpt":{"rendered":"
Oops, that wasn’t allowed after all.<\/p>\n","protected":false},"author":1069,"featured_media":111744,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[1],"tags":[25],"class_list":["post-97635","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
Oops, that wasn’t allowed after all.<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/97635","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/users\/1069"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/comments?post=97635"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/97635\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media\/111744"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media?parent=97635"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=97635"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=97635"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}