That message is so helpful. 9_9

That's definitely been around forever-ish like you say. I vaguely recall fiddling with it a while back and I decided it was a compiler bug (because it went away with trivial code movement) but I don't think I got any farther than that.

I'll see if I can see something obvious we could report/change.

We saw this in  crbug.com/114067  (it has been fixed by getting rid of some includes)

Yeah, for example adding \0 at the end of this fprintf string makes it go away

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/yasm/source/patched-yasm/modules/arch/lc3b/lc3barch.c&q=lc3b_reg_print&sq=package:chromium&l=137

But so does modifying the body of many other functions.

Well, I think that the problem is in the 'lc3b_get_fill' function... It declares the following array:

static const unsigned char *fill[16] = {
        NULL,           /* unused */
        NULL,           /* 1 - illegal; all opcodes are 2 bytes long */
        (const unsigned char *)
        "\x00\x00",                     /* 4 */
        NULL,                           /* 3 - illegal */
        (const unsigned char *)
        "\x00\x00\x00\x00",             /* 4 */
        NULL,                           /* 5 - illegal */
        (const unsigned char *)
        "\x00\x00\x00\x00\x00\x00",     /* 6 */
        NULL,                           /* 7 - illegal */
        (const unsigned char *)
        "\x00\x00\x00\x00\x00\x00"      /* 8 */
        "\x00\x00",
        NULL,                           /* 9 - illegal */
        (const unsigned char *)
        "\x00\x00\x00\x00\x00\x00"      /* 10 */
        "\x00\x00\x00\x00",
        NULL,                           /* 11 - illegal */
        (const unsigned char *)
        "\x00\x00\x00\x00\x00\x00"      /* 12 */
        "\x00\x00\x00\x00\x00\x00",
        NULL,                           /* 13 - illegal */
        (const unsigned char *)
        "\x00\x00\x00\x00\x00\x00"      /* 14 */
        "\x00\x00\x00\x00\x00\x00\x00\x00",
        NULL                            /* 15 - illegal */
    };



changing fill[2] to "\x00\x00\x00\x00" (instead of "\x00\x00") fix the issue. It might be the right thing to do as the comment says "/* 4 */" (my guess is that this value is the number of noops to emit ?

(note that it doesn't explain why we're observing this warning)

Yeah, it seems changing a bunch of strings can make it go away, but I don't understand why.

(I have no idea about the code, but to me it looks like that comment is wrong and should say /* 2 */, given all the other monotonically increasing comments, rather than the code itself being wrong.)

Hum, I've tried changing the other strings in this file (lc3barch.c) but fill[2] is the only one that seems to fix the problem...

The error only makes sense for a string that is defined twice, in the two different source files, in an ODR violating way. A static string cannot (I don't think) violate ODR.

I like the compiler bug theory. If we can create a minimal repro - or an enormous link repro - we can file a bug.

Otherwise, turn off LTCG for those two files?

I can also fix this by changing this line in macho-objfmt.c [1]:

const char pad_data[3] = "\0\0\0";

by:

const char pad_data[3] = "\0\0";

or

const char pad_data[4] = "\0\0\0";

AFAIK it doesn't change anything ? The trailing \0 will still be appended to the original string ? Is it valid C to initialize the whole array with a string literal or should we always initialize only N-1 elements to leave enough room for the trailing null char ?

I'll try to turn off WPO for these files.


[1] https://code.google.com/p/chromium/codesearch#chromium/src/third_party/yasm/source/patched-yasm/modules/objfmts/macho/macho-objfmt.c&l=1045

You could also use an array instead of a string
const char pad_data[3] = { '\0', '\0', '\0' };

Yeah, but it's unclear why the existing code is causing this warning ?