Bug 16408 – [REG2.065] left-to-right order of evaluation of function arguments not consistently followed
Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
All
Creation time
2016-08-21T08:14:00Z
Last change time
2017-08-07T13:15:45Z
Keywords
wrong-code
Assigned to
nobody
Creator
matthew.brennan.jones
Comments
Comment #0 by matthew.brennan.jones — 2016-08-21T08:14:16Z
This program prints WASD in debug mode as expected, but prints DDDD in release mode. My guess is std.conv.to!string is failing with char* for some reason.
DMD 2.071.1 on Linux and Windows 10.
Works as expected:
dub run --arch=x86_64 --build=debug
Wrong output:
dub run --arch=x86_64 --build=release
// maid.d
import std.stdio;
import std.string;
import std.conv;
import derelict.sdl2.sdl;
int main() {
DerelictSDL2.load(SharedLibVersion(2, 0, 2));
string footer = "%s%s%s%s".format(
to!string(SDL_GetKeyName(SDLK_w)),
to!string(SDL_GetKeyName(SDLK_a)),
to!string(SDL_GetKeyName(SDLK_s)),
to!string(SDL_GetKeyName(SDLK_d)),
);
stdout.writeln(footer);
return 0;
}
// dub.json
{
"name": "main",
"targetType": "executable",
"dependencies": {
"derelict-sdl2": "~>2.0",
},
"sourceFiles": [
"main.d"
],
}
Comment #1 by lodovico — 2016-08-21T10:31:20Z
(In reply to Matt Jones from comment #0)
> This program prints WASD in debug mode as expected, but prints DDDD in
> release mode. My guess is std.conv.to!string is failing with char* for some
> reason.
>
> [...]
Can I ask you to try with `std.string.fromStringz` instead of `std.conv.to!string` ?
This may help find out where the bug is.
Comment #2 by matthew.brennan.jones — 2016-08-21T11:48:48Z
Interesting. Changing from std.conv.to!string to std.string.fromStringz returns "DDDD" for both debug and release mode.
Comment #3 by ag0aep6g — 2016-08-21T12:44:39Z
This is triggered by the -inline switch.
This is a regression. With 2.064.2 it prints "WASD" as expected. Since 2.065 it prints "DDDD".
This seems to be related to SDL_GetKeyName reusing the buffer it returns. From its docs [1]:
> Returns a pointer to a UTF-8 string that stays valid at least until the next call to this function.
So when calling SDL_GetKeyName it multiple times, the previous results are strictly invalidated. In practice, they're overwritten with the newest value.
A reduction based on that:
----
import std.stdio;
import std.conv;
char[2] buffer = '\0';
const(char*) SDL_GetKeyName(char k)
{
buffer[0] = k;
return buffer.ptr;
}
void main()
{
/* prints "WASD" or "DDDD" depending on -inline */
writeln(
to!string(SDL_GetKeyName('W')),
to!string(SDL_GetKeyName('A')),
to!string(SDL_GetKeyName('S')),
to!string(SDL_GetKeyName('D')),
);
}
----
That test case works with 2.066 and fails since 2.067. Notice that those are different versions than for the original test case.
A further reduction reveals a wrong-code compiler bug:
----
char[1] SDL_GetKeyName_buffer;
const(char)[] SDL_GetKeyName(char k)
{
SDL_GetKeyName_buffer[0] = k;
return SDL_GetKeyName_buffer[];
}
void formatGeneric() {}
void formattedWrite(string strW, string strA)
{
auto fun = ()@trusted{ return &formatGeneric; }(); /* !? */
assert(strW == "W", strW); /* passes without -inline, fails with */
assert(strA == "A");
}
void main()
{
formattedWrite(
SDL_GetKeyName('W').idup,
SDL_GetKeyName('A').idup,
);
}
----
That one works with 2.065 and fails since 2.066. Yet again other versions.
I'm promoting this to a regression and to a wrong-code compiler bug. When fixing, all test cases should be checked, since they're might be multiple things going on here.
[1] https://wiki.libsdl.org/SDL_GetKeyName
Comment #4 by lodovico — 2016-08-21T13:03:15Z
(In reply to Matt Jones from comment #2)
> Interesting. Changing from std.conv.to!string to std.string.fromStringz
> returns "DDDD" for both debug and release mode.
ag0aep6g's comment (https://issues.dlang.org/show_bug.cgi?id=16408#c3) is correct.
The fact that SDL_GetKeyName always returns the same buffer explains why fromStringz gives the wrong answer in both debug and release.
So I agree that this is a wrong-code bug.
Comment #5 by matthew.brennan.jones — 2016-08-21T13:13:50Z
Excellent. Thanks for figuring this out. Now that I know what is wrong, it should be easy to code around.
Thanks.
Comment #6 by bugzilla — 2017-04-12T20:46:49Z
Compiles and runs fine with HEAD. I don't know when it was fixed.
Comment #7 by ag0aep6g — 2017-04-12T20:57:40Z
(In reply to Walter Bright from comment #6)
> Compiles and runs fine with HEAD. I don't know when it was fixed.
The test cases from comment #3 still fail for me. dmd 15b3ee1 on linux. Did you compile with -inline?
Comment #8 by bugzilla — 2017-04-12T21:09:10Z
Yes, you're right. It's still a bug. I had failed to specify -inline.
Comment #9 by bugzilla — 2017-04-16T03:45:52Z
What's going wrong is the compiler is moving expressions with side effects (SDL_GetKeyName('W'), SDL_GetKeyName('A')) to before the function call, which gets the side effects out of order. I seem to recall a PR to do this, I'll have to find it.