cmd/compile: optimization around runtime.throw breaks delve DAP

[hyangah]

New issue in the current tip/1.22.

From @aarzilli : go-delve/delve#3455 (comment)

go-delve/delve#2616 description explains how delve is currently extracting the reason info.

From @thanm : look into extending the existing x/debug/dwtest tests to catch this type of regression.

[thanm]

I dug into this a bit. In fact the DWARF for runtime.throw looks fine, I don't think that is the issue. What appears to be different is this CL: http://go.dev/cl/c/go/+/390420, which is changing the runtime to differentiate between 'user' throws and 'system' throws. As a result of this CL, runtime.newproc1 is invoking 'runtime.fatalinstead ofruntime.throw`.

It sounds as though any special treatment that Delve has for 'runtime.throw' may need to be extended to runtime.fatal as well.

[aarzilli]

I don't think that we do anything special for either, it's just reading the s argument, it's the same code for both and it's probably broken for both of them. I haven't bisected the problem but I would expect this to be caused by a compiler change.

Looking at this more it looks like the problem only happens when the program is compiled with -gcflags=all=-N -l (is -N applied to the runtime now?) where the location for s for runtime.fatal is DW_OP_call_frame_cfa (and nothing else). With optimizations enabled it gets a correct location list.

[thanm]

Bisecting seems to point the finger at https://go-review.googlesource.com/c/go/+/521699, which makes sense based on the commit message. I'll look a bit more tomorrow.

[thanm]

When building the runtime with "-gcflags="all=-l -N", it looks as though when we execute this code here:

https://go.googlesource.com/go/+/fbcf43c60ba5170309a238b0e42fd5879d419776/src/cmd/compile/internal/dwarfgen/dwarf.go#146

At that line there is a check made to see if "base.Ctxt.Flag_optimize" is set, this check is failing, which causes the code to drop back down into the ABI0 case. The problem from CL 521699 appears to be in this line here:

https://go.googlesource.com/go/+/fbcf43c60ba5170309a238b0e42fd5879d419776/src/cmd/compile/internal/base/flag.go#336

which sets "base.Flag.N" to 0, but doesn't correspondingly update base.Ctxt.Flag_optimize (which was set previously on line 274 based on the original value of -N).

I'll send a CL. Also kind of makes me wonder whether we should still have both "base.Flag.N" and "base.Ctxt.Flag_optimize"; I think the latter was important back before big rsc refactor.

[gopherbot]

Change https://go.dev/cl/527319 mentions this issue: cmd/compile/internal/base: keep Ctxt.Flag_optimize in sync with Flag.N

[gopherbot]

Change https://go.dev/cl/526836 mentions this issue: dwtest: add explicit testpoint for runtime.throw