Bug 19679 – variable escapes unnoticed when referenced in function called from function whose address is taken

Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2019-02-15T11:06:09Z
Last change time
2019-02-19T02:57:20Z
Keywords
pull, wrong-code
Assigned to
No Owner
Creator
FeepingCreature

Comments

Comment #0 by default_357-line — 2019-02-15T11:06:09Z
Consider this failing code: void delegate() foo() { size_t value = 0; void check() { assert(value == 0); } void nest1() { void nest2() { check(); } nest2(); } return &nest1; } void main() { foo()(); } What should happen is that foo allocates its stackframe on the heap. However, this does not occur, because foo doesn't realize that value is referenced at all. Speculation from staring at printfs for two hours: if nest2() is removed and check() is called directly, D notices that nest1() is a sibling caller for check(). However, since the call is hidden in a nested function of a nested function that is never called in the first place, D seems to lose the plot somewhere. I don't know where, the escape code is very wacky. Good luck.
Comment #1 by ibuclaw — 2019-02-16T07:57:13Z
I think the correct place to check would be checkEscapingSiblings. // After g.isThis() check. for (auto p = g; p; p = p.parent.isFuncDeclaration()) { // A parent of the sibling had its address taken. // Assume escaping of parent affects its children, so needs propagating. if (p.tookAddressOf) { markAsNeedingClosure(p, outerFunc); bAnyClosures = true; } } It's a bit of a weak check though. It would be a stronger guarantee to keep a reference of _all_ callers of a function, not just its siblings, so that nested functions that are unreferenced are correctly ignored.
Comment #2 by default_357-line — 2019-02-18T09:58:57Z
Seems a good start. Wanna open a PR so we can GTM? If they don't like it, they can always request changes. Most of all I want to get it into the testsuite, even with an inadequate fix, instead of waiting for a better one that may come whenever.
Comment #3 by dlang-bot — 2019-02-18T15:38:29Z
@FeepingCreature created dlang/dmd pull request #9372 "Fix issue 19679, add test" fixing this issue: - Fix issue 19679, add test: if the address of the parent of a sibling caller was taken, consider the caller escaped via that reference https://github.com/dlang/dmd/pull/9372
Comment #4 by dlang-bot — 2019-02-19T02:57:20Z
dlang/dmd pull request #9372 "Fix issue 19679, add test" was merged into master: - c3b725d0476edef0b291b4c34fc369a50b9d40c8 by Mathis Beer: Fix issue 19679, add test: if the address of the parent of a sibling caller was taken, consider the caller escaped via that reference https://github.com/dlang/dmd/pull/9372