ms vc++ compiler optimizating away erroneous code

Question

I know its to do with compiler optimization, but I am looking for a real deep dive into how/why and how to ensure i am notified of this behaviour in real code.

I have this code

void swap(char *s)
{
    strcpy(s, "nope!");
    printf("Result: %s\n", s);
};

void main(){
...
swap("this should segfault");
...
}

obviously, it should segfault, but visual studio in release mode reduces it down to just inlining the printf.

This seems like the kind of thing that could really bite me in the ass later, so i would love it if you could shine some light on this for me.

for completeness sake here is the expected assembly

push    offset s        ; "this should segfault"
call    j_?swap@@YAXPAD@Z ; swap(char *)

and here is the generated assembly

push    offset s        ; "this should segfault"
push    offset Format   ; "Result: %s\n"
call    ds:__imp__printf

and here are the compiler options as requested in comments

/GS /GL /analyze- /W3 /Gy /Zc:wchar_t /Zi /Gm- /O2 /Fd"Release\vc120.pdb" /fp:precise /D "WIN32" /D "NDEBUG" /D "_CONSOLE" /D "_LIB" /D "_UNICODE" /D "UNICODE" /errorReport:prompt /WX- /Zc:forScope /Gd /Oy- /Oi /MD /Fa"Release\" /EHsc /nologo /Fo"Release\" /Fp"Release\scratchpad2.pch"

Why do you think this this code should crash? You haven't given enough details about the value of `s` for anyone but you to know that for sure. Is it null? Is it a buffer overflow? — Retired Ninja, Jun 29 '16 at 23:00
Why is it obvious that this should segfault? The code looks valid to me, as long as certain assumptions about `s` hold. And if they don't hold (as in, `s` doesn't point to a large enough writable buffer), then the code exhibits undefined behavior, and the compiler is free to produce any outcome. "Appears to work" is one possible manifestation of undefined behavior. — Igor Tandetnik, Jun 29 '16 at 23:00
@igor fair enough. added a sample call that would case the fault as well as compiler generated assembly — jhbh, Jun 29 '16 at 23:21
What version of VC++ and what optimization settings? Also, you may want to include more context around the assembly. You say `expected assembly: push offset s` but there is no symbol `s` at that point inside `main`. — dxiv, Jun 29 '16 at 23:36
@dxiv symbol `s` is from `swap(char* s)`. it is compiled using platform toolset v120 using default options for release mode — jhbh, Jun 29 '16 at 23:53
You may want to note, that a string literal is of type `const char[]`, and writing to it usually causes an *access violation* (there are no *segfaults* on Windows). It is UB, so a compiler is free to emit any code. While you say that an optimized build inlines the `printf` call, your assembly seems to imply, that it's also optimizing away the call to `strcpy`. Is that the case? This would be odd, as `strcpy` does have side effects. — IInspectable, Jun 30 '16 at 07:54
@IInspectable Oddly enough, the `strcpy` call is completely elided in this case if (a) it's optimized to `intrinsic` and (b) the destination is a literal string constant. — dxiv, Jun 30 '16 at 16:56

score 1 · Accepted Answer · edited May 23 '17 at 12:32

Behavior confirmed here with VC++ 2013 and 2015. @IgorTandetnik's comment is correct. The code exhibits UB (undefined behavior), and not crashing is one such possible behavior.

That said, VC++ is also at fault for not issuing (at least) a warning when allowing the string-literal-to-non-const-char-pointer conversion which has been deprecated since (I think) C++0x - see Why can a string literal be implicitly converted to char* only in certain case? and Why is passing a string literal into a char* argument only sometimes a compiler error?. You may consider filing a bug report at https://connect.microsoft.com/visualstudio about that.

I know its to do with compiler optimization, but I am looking for a real deep dive into how/why and how to ensure i am notified of this behaviour in real code.

I don't have an answer to the latter part of the question.

As to how/why, it appears to be a matter of strcpy behavior when optimized as an intrinsic with a read-only destination. A rather odd behavior, indeed, since it ultimately results in the entire strcpy being silently skipped.

Adding #pragma function(strcpy) before swap will cause it to always crash.
Changing the calling code to char z[] = "this should segfault"; swap(z); will remove the UB factor and make it always work.

[EDIT] Going back to the how to ensure i am notified of this behaviour in real code part, and until VC++ obliges with a compilation warning or error, the following could work around it by explicitly providing a const char * overload. Confirmed again with both VC++ 2013 and 2015.

#include <string.h>
#include <stdio.h>

#pragma intrinsic(strcpy)

static void swap(char *s)
{
    strcpy(s, "nope!");
    printf("OK: %s\n", s);
};

static void swap(const char *s)
{
    printf("No: %s?\n", s);
};

void main()
{
    char z[] = "this should segfault";
    swap(z);                      // prints 'OK: nope!'
    swap("this should segfault"); // prints 'No: this should segfault?'
}

ms vc++ compiler optimizating away erroneous code

1 Answers1

Linked