Thread overview
[Issue 14459] String literal merge bug causes incorrect runtime program behavior
Apr 18, 2015
Dan Olson
Apr 19, 2015
yebblies
Apr 19, 2015
Ketmar Dark
Apr 19, 2015
Dan Olson
Apr 23, 2015
David Nadlinger
Aug 06, 2015
Kenji Hara
Aug 06, 2015
David Nadlinger
Aug 06, 2015
Kenji Hara
April 18, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

Dan Olson <gorox@comcast.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code

--
April 19, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies@gmail.com

--- Comment #1 from yebblies <yebblies@gmail.com> ---
Why do you think string literals must be merged?  I would guess it's in 'implementation defined' territory, and identity equality should not be relied on.

--
April 19, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

--- Comment #2 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
yet in the case when i'm assigning pointer to pointer i'm expecting that two pointers are the same. i'd never expect the following fail, under no circumstances:

    const char* s16 = "hi16";
    const(char)* p16 = s16;
    assert(p0 == s0);                        // fails

--
April 19, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

--- Comment #3 from Dan Olson <gorox@comcast.net> ---
(In reply to yebblies from comment #1)
> Why do you think string literals must be merged?  I would guess it's in 'implementation defined' territory, and identity equality should not be relied on.

Yes I agree in general it is implementation defined for any D compiler to merge string literals.  In this case, one pointer is initialized to the the value of another, so they do need to be equal.  The dmd front end code propagates const char* literals into unique StringExps when used.  Now the backend must merge StringExps with identical literals or else bad code can be generated.  In LDC which uses dmd front end, it was worse because string literals were merged late and even simpler code was wrong.  See https://github.com/ldc-developers/ldc/issues/898 for details.

In the snippet, it leads to p0 == s0, as they should be, then a few lines later, suddenly p0 != s0.

Anyway, it is in the backend el.c stable[16] where the cache is defined.  Also, the code in the frond end that does the const propagation of string literals is fromConstInitializer() in optimize.c

--
April 23, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

David Nadlinger <code@klickverbot.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |code@klickverbot.at

--- Comment #4 from David Nadlinger <code@klickverbot.at> ---
Indeed, p0 == s0 must never fail as they are both pointers and one is initialized from the other, irrespective of what we guarantee or don't guarantee about string literal merging.

--
August 06, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull
           Hardware|x86                         |All
                 OS|Mac OS X                    |All

--- Comment #5 from Kenji Hara <k.hara.pg@gmail.com> ---
https://github.com/D-Programming-Language/dmd/pull/4866(In reply to Dan Olson
from comment #3)
> Anyway, it is in the backend el.c stable[16] where the cache is defined. Also, the code in the frond end that does the const propagation of string literals is fromConstInitializer() in optimize.c

Fixed the bug in optimize.c: https://github.com/D-Programming-Language/dmd/pull/4866

Honestly I didn't know about this issue until now (my random bugzila walk fortunately hit). I'm curious why the detailed analysis result couldn't put out actual PR.

--
August 06, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

--- Comment #6 from David Nadlinger <code@klickverbot.at> ---
(In reply to Kenji Hara from comment #5)
> I'm curious why the detailed analysis result couldn't put out actual PR.

As for me, that's because Daniel already knew how to fix the issue, but I couldn't convince him that the behavior is actually wrong at DConf (or rather, the day after).

--
August 06, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

--- Comment #7 from Kenji Hara <k.hara.pg@gmail.com> ---
I think it's clearly a bug. Because:

1. The identiry of the string literal is saved into a variable, then copied to
one another variable. The two pointer variables comparison must match.
2. But the excessive front end 'optimization' didn't save the identity. It's a
problem.

A point is the variable s0 is a stack allocated variable. Its value is evaluated and determined at runtime. The optimization wrongly beyond the compile time/runtime evaluation boundary.

--
August 06, 2015
https://issues.dlang.org/show_bug.cgi?id=14459

--- Comment #8 from github-bugzilla@puremagic.com ---
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/3252ddcf36f53928d04b62a601c0b7bb9d911cba
fix Issue 14459 - String literal merge bug causes incorrect runtime program
behavior

https://github.com/D-Programming-Language/dmd/commit/fd8e527b2a950eaa437426e8201572e0da51000f Merge pull request #4866 from 9rnsr/fix14459

Issue 14459 - String literal merge bug causes incorrect runtime program behavior

--
July 22, 2017
https://issues.dlang.org/show_bug.cgi?id=14459

--- Comment #9 from github-bugzilla@puremagic.com ---
Commits pushed to dmd-cxx at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/3252ddcf36f53928d04b62a601c0b7bb9d911cba fix Issue 14459 - String literal merge bug causes incorrect runtime program behavior

https://github.com/dlang/dmd/commit/fd8e527b2a950eaa437426e8201572e0da51000f Merge pull request #4866 from 9rnsr/fix14459

--