Bug 24534 – Having a label on a declaration makes it possible to skip it with goto

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P3
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2024-05-03T20:42:42Z
Last change time
2024-05-21T00:22:24Z
Keywords
accepts-invalid, pull
Assigned to
No Owner
Creator
Jonathan M Davis

Comments

Comment #0 by issues.dlang — 2024-05-03T20:42:42Z
This code compiles and runs: --- void main() { int x; goto Label; Dummy: int y; Label: int z; import std.stdio; writeln(y); } --- The value of y is then garbage (e.g. 539662115), because the declaration for y was skipped, and it wasn't actually initialized. On the other hand, if the Dummy label is removed, then we get an error such as --- q.d(4): Error: `goto` skips declaration of variable `q.main.y` q.d(5): declared here --- So, it would appear that the label on the variable is confusing the compiler and causing it to not understand that a declaration has been skipped.
Comment #1 by ben.james.jones — 2024-05-03T21:02:17Z
*** Issue 24535 has been marked as a duplicate of this issue. ***
Comment #2 by alphaglosined — 2024-05-03T21:09:06Z
After thinking about this, it's intentional. This allows you to have multiple blocks at the end of your function that handle things like success versus error. It is very C style, and is used in dmd. The solution of course would be to create a new scope so it doesn't bleed between the labels. ```d goto Lerror; Lsuccess: { } Lerror: { } ``` If this is fixed, it's going to need a very long deprecation. Or we could kill off this logic entirely and have type state analysis proper which works on a per-variable basis rather than a coarse reverse search which it is currently.
Comment #3 by issues.dlang — 2024-05-03T21:26:08Z
Well, it's in clear violation of the spec: https://dlang.org/spec/statement.html#goto-statement So, either the spec is wrong, or dmd is. Also, if you slap @safe on main, it compiles even though the initialization is skipped, which should never happen in @safe code. So, there's a bug here of some kind regardless.
Comment #4 by ben.james.jones — 2024-05-03T21:55:31Z
Gotta be a bug: void f2(){ //compiles fine int x2; goto Label2; Dummy2: int y2; Label2: int z2; } GotoStatement::semantic() for Label2 goto lastvar: x2 LabelStatement::semantic() for Dummy2 labelstatement lastvar: x2 LabelStatement::semantic() for Label2 labelstatement lastvar: x2 <--- WRONG, should be y2
Comment #5 by dlang-bot — 2024-05-17T20:06:12Z
@benjones updated dlang/dmd pull request #16510 "fix issue24534 : goto can skip declarations in labeled statements without error" fixing this issue: - Fix Bugzilla Issue 24534 : goto can skip declarations in labeled statements without error https://github.com/dlang/dmd/pull/16510
Comment #6 by issues.dlang — 2024-05-19T05:22:59Z
Since this is in violation of the spec, I'm changing this back to a bug. If it's decided that the current behavior is desirable for whatever reason, then the spec will need to be updated instead. And even if the spec is updated instead of making this code illegal like the spec says it should be, the fact that @safe works with this code in spite of the fact that the variable is not properly initialized means that there's a compiler bug either way. So, it's not an enhancement.
Comment #7 by alphaglosined — 2024-05-19T07:35:19Z
It is a bug in the compiler, I agree on that too. But, it is also desirable to skip variable declarations, as long as you don't touch them you do not violate the spec clause. Since this is used in C code quite often, and therefore ported code (such as dmd's backend). A shortcut was taken with the current reverse search to analyze it, and it doesn't work as a result. Solving this properly requires type state analysis as that is what this particular clause involves. Therefore any attempts to fix this that does not do type state analysis is unfortunately another shortcut.
Comment #8 by alphaglosined — 2024-05-19T08:22:37Z
I've done some playing around to see if backward goto's have an equivalent issue. It seems dmd is actually smart there, and I couldn't get that to not work. Generates a try/finally: ``` Label: SomethingWithSideEffects var = 0; try { if (uniform01() > 0.5) return 0; goto Label; } finally var.~this(); ``` This means only forward goto's have broken analysis.
Comment #9 by issues.dlang — 2024-05-19T08:37:46Z
(In reply to Richard (Rikki) Andrew Cattermole from comment #7) > But, it is also desirable to skip variable declarations, as long as you > don't touch them you do not violate the spec clause. The spec clause specifically disallows that: "It is illegal for a GotoStatement to be used to skip initializations." There's certainly an argument to be made that the rule could be more fine-grained and require that no variable be accessed whose declaration was skipped (which would also mean that the variable couldn't have a destructor), but as the rule currently stands, you can't legally skip variable declarations whether the variable is then used or not.
Comment #10 by ben.james.jones — 2024-05-20T16:54:12Z
"Since this is used in C code quite often, and therefore ported code (such as dmd's backend)." From what I can tell, there's only one place in the whole compiler that's affected by this bug, so I don't think this is something that was commonly relied on.
Comment #11 by dlang-bot — 2024-05-21T00:22:24Z
dlang/dmd pull request #16510 "fix issue24534 : goto can skip declarations in labeled statements without error" was merged into master: - 7f30b66d3dae37544c7629af1178f908984c2a93 by Ben Jones: Fix Bugzilla Issue 24534 : goto can skip declarations in labeled statements without error https://github.com/dlang/dmd/pull/16510