\nOne mark of punctuation can make all the difference.\n<\/p>\n
\nOne program was encountering a stack overflow exception in\na function that didn’t appear to be doing anything particularly\nstack-hungry.\nThe following code illustrates the problem:\n<\/p>\n
\nbool TestResults::IsEqual(TestResults& expected)\n{\n if (m_testMask != expected.m_testMask) {\n return false;\n }\n bool result = true;\n if (result && (m_testMask & AbcTestType)) {\n result = CompareAbc(expected);\n }\n if (result && (m_testMask & DefTestType)) {\n result = CompareDef(expected);\n }\n if (result && (m_testMask & GhiTestType)) {\n result = CompareGhi(expected);\n }\n if (result && (m_testMask & JklTestType)) {\n result = CompareJkl(expected);\n }\n return result;\n}\n<\/pre>\n\n(In reality, the algorithm for comparing two tests results\nwas much more complicated, but that’s irrelevant to this\ndiscussion.)\n<\/p>\n
\nAnd yet on entry to this function, a stack overflow was raised.\n<\/p>\n
\nThe first thing to note is that this problem occurred only\non the x64 build of the test.\nThe x86 version ran fine, or at least appeared to.\nIt so happens that the x64 compiler aggressively inlines functions,\nwhich as it turned out was a major exacerbator of the problem.\n<\/p>\n
\nThe title of this entry probably tipped you off to what happened:\nThe helper functions accepted the test results parameter by value\nnot by reference:\n<\/p>\n
\nbool TestResults::CompareAbc(TestResults expected);\nbool TestResults::CompareDef(TestResults expected);\nbool TestResults::CompareGhi(TestResults expected);\nbool TestResults::CompareJkl(TestResults expected);\n<\/pre>\n\nand those comparison functions in turn called other\ncomparison functions, which also passed the TestResults<\/code>\nby value.\nSince the test results were passed by value, a temporary\ncopy was made on the stack and passed to the comparison function.\nIt so happened that the TestResults<\/code> class was\na very large one, a hundred kilobytes or so, and the\nTestResults::IsEqual<\/code> function therefore needed\nto reserve room for a large number of such temporary copies,\none for each call to a comparison function in each of the\ninlined functions.\nA dozen temporary copies times a hundred kilobytes per copy\ncomes out to over a megabyte of temporary variables,\nwhich exceeded the default one megabyte stack size\nand therefore resulted in a stack overflow exception on entry\nto the TestResults::IsEqual<\/code> function.\n<\/p>\n\nThis code appeared to run fine when compiled for the x86 architecture\nbecause the x86-targetting compiler did not inline quite as\naggressively, so the large temporaries were not reserved on the\nstack until the helper comparison was actually called.\nSince the comparisons went only three levels deep,\nthere were only three temporary copies of the\nexpected<\/code> parameter, which fit within the one megabyte\ndefault stack.\nIt was still bad code—consuming a few hundred kilobytes\nof stack for no reason—but it wasn’t bad enough to cause\na problem.\nThe fix, of course, was to change the comparison functions to\naccept the parameter by reference.\n<\/p>\n\nbool TestResults::IsEqual(const TestResults& expected) const;\nbool TestResults::CompareAbc(const TestResults& expected) const;\nbool TestResults::CompareDef(const TestResults& expected) const;\nbool TestResults::CompareGhi(const TestResults& expected) const;\nbool TestResults::CompareJkl(const TestResults& expected) const;\n<\/pre>\n\nFor good measure, the parameter was changed to a\nconst<\/code> reference, and the function was\ntagged as itself const<\/code> to emphasize that\nneither the object nor the expected value will be modified\nas part of the comparison,\nthereby ensuring that changing from a copy to a const<\/code> reference\ndidn’t change the previous behavior.\nWithout the const<\/code> reference,\nthere was a possibility that somewhere deep inside the comparison\nfunctions, they made a change to the expected<\/code>\nparameter.\nUnder the old pass-by-value declaration, this change was discarded\nwhen the function returned since the change was made to a copy.\nIf we had left off the const<\/code> from the reference,\nthen we would have changed the behavior: The change to the\nexpected<\/code> parameter would have modified the original\nTestResults<\/code>.\nMaking the parameter const<\/code> reassures us that an\nattempt to modify expected<\/code> would be flagged by the\ncompiler and therefore brought to our attention.\n<\/p>\n\n(This technique is not foolproof, however.\nSomebody could always cast away const<\/code>-ness and\nmodify the original,\nbut we were being reckless and assuming that nobody would be that\ncrazy.)<\/p>\n","protected":false},"excerpt":{"rendered":"One mark of punctuation can make all the difference. One program was encountering a stack overflow exception in a function that didn’t appear to be doing anything particularly stack-hungry. The following code illustrates the problem: bool TestResults::IsEqual(TestResults& expected) { if (m_testMask != expected.m_testMask) { return false; } bool result = true; if (result && (m_testMask […]<\/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-31733","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
One mark of punctuation can make all the difference. One program was encountering a stack overflow exception in a function that didn’t appear to be doing anything particularly stack-hungry. The following code illustrates the problem: bool TestResults::IsEqual(TestResults& expected) { if (m_testMask != expected.m_testMask) { return false; } bool result = true; if (result && (m_testMask […]<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/31733","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=31733"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/31733\/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=31733"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=31733"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=31733"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}