Thread overview | |||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
January 12, 2018 [Issue 18232] string variable in toString method of Union: invalid code (crash/segfault) | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 ag0aep6g@gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |wrong-code CC| |ag0aep6g@gmail.com -- |
January 12, 2018 [Issue 18232] string variable in toString method of Union: invalid code (crash/segfault) | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 hsteoh@quickfur.ath.cx changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hsteoh@quickfur.ath.cx --- Comment #1 from hsteoh@quickfur.ath.cx --- The crash is caused by wrong codegen for the toString() method. Here's a comparison of a toString() method for a struct vs. for a union: Struct code: -------- struct Jack { float f; int i; string toString() { string s; return s; } } -------- Generated assembly for Jack.toString: -------- 447dc: 55 push %rbp 447dd: 48 8b ec mov %rsp,%rbp 447e0: 31 c0 xor %eax,%eax 447e2: 31 d2 xor %edx,%edx 447e4: 5d pop %rbp 447e5: c3 retq -------- As can be easily seen, this basically initializes the string s to have .ptr=null, .length=0, returned by value in %eax and %edx. This is the correct behaviour. Union code (basically the same as the struct code, just change 'struct' to 'union'): -------- union Jack { float f; int i; string toString() { string s; return s; } } -------- Generated assembly for Jack.toString: -------- 4471c: 55 push %rbp 4471d: 48 8b ec mov %rsp,%rbp 44720: 48 83 ec 10 sub $0x10,%rsp 44724: 48 8b 55 f8 mov -0x8(%rbp),%rdx 44728: 48 8b 45 f0 mov -0x10(%rbp),%rax 4472c: c9 leaveq 4472d: c3 retq -------- Here, the 'sub' line appears to be allocating space on the stack for a local variable, presumeably s. But then it fails to initialize s before loading it into the return registers %rdx and %rax. I'm assuming that this is probably why the allocation of the local variable never got elided, as it was in the struct case -- the optimizer doesn't see the missing initialization of s, so it cannot elide the loads from the stack. So it looks like the bug is caused by failure to initialize s when generating code for a union vs. a struct. Why, though, is a mystery to me, since toString never actually references `this`, so in theory whether `this` is a struct or union shouldn't matter. -- |
January 12, 2018 [Issue 18232] string variable in toString method of Union: invalid code (crash/segfault) | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 --- Comment #2 from hsteoh@quickfur.ath.cx --- Note also, the name `toString` is irrelevant to this bug. The same codegen bug appears if you rename the method to something else, like 'member'. Furthermore, the local variable doesn't have to be a string; any local variable without an initializer would also exhibit the same missing initialization, it doesn't initialize the local to .init, as it should. -- |
January 12, 2018 [Issue 18232] Union methods fail to initialize local variables to .init | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 hsteoh@quickfur.ath.cx changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|string variable in toString |Union methods fail to |method of Union: invalid |initialize local variables |code (crash/segfault) |to .init -- |
January 12, 2018 [Issue 18232] Union methods fail to initialize local variables to .init | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 --- Comment #3 from hsteoh@quickfur.ath.cx --- Minimized code: ------ union U { int method() { int x; return x; } } ------ The disassembly shows that x is never initialized to 0, and a garbage value is returned. -- |
January 12, 2018 [Issue 18232] Union methods fail to initialize local variables to .init | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 --- Comment #4 from hsteoh@quickfur.ath.cx --- More interesting clues: running a union method inside CTFE containing a local variable without an explicit initializer causes a CTFE error "cannot modify variable at compile time", whereas explicitly initializing the local variable works. ------ union U { int method() { int x; // causes CTFE failure unless explicitly initialized return x; } } enum y = U.init.method(); ------ -- |
January 12, 2018 [Issue 18232] Union methods fail to initialize local variables to .init | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 hsteoh@quickfur.ath.cx changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |pull --- Comment #5 from hsteoh@quickfur.ath.cx --- https://github.com/dlang/dmd/pull/7687 -- |
January 15, 2018 [Issue 18232] Union methods fail to initialize local variables to .init | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 --- Comment #6 from github-bugzilla@puremagic.com --- Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/29c1e309be359f871d4052c025f7f3144005d092 Fix issue 18232: local vars in union member functions needs initializer. The `isunion` condition is too broad, because it also excludes local variables declared inside union member functions. Moreoever, it is not necessary, because the `fd` condition later on already excludes union fields, since they would not be inside a function declaration. https://github.com/dlang/dmd/commit/4cf228eb33ee8e2b4b6856fa7c08990217039ac7 Merge pull request #7687 from quickfur/issue18232 Fix issue 18232: local vars in union member functions needs initializer. merged-on-behalf-of: Sebastian Wilzbach <sebi.wilzbach@gmail.com> -- |
January 15, 2018 [Issue 18232] Union methods fail to initialize local variables to .init | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=18232 github-bugzilla@puremagic.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED -- |
Copyright © 1999-2021 by the D Language Foundation