-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/compile: misleading stack trace related to struct embedding. #33724
Comments
We have several other incorrect line number issues. This may likely be a duplicate of something else. |
Looking at the asm produced for the "correct" version of the code, there's at least one surprising thing. Forgive me if I'm being obtuse about something obvious, but I don't understand what
|
So this looks like it's setting AX to zero ( |
The |
I think this is an issue with one of the compiler's optimization passes. It doesn't seem like a runtime issue to me. Also, |
Slightly simpler repro:
Remove Lastly, it's nothing related to the wrapper method being compiler generated and its stack frame elided by the runtime. If you explicitly add:
then you see the same behavior, except with the extra stack frame. |
The problem is both a bit more benign and more interesting than that actually. Looking at But eventually we get to late nilcheck, and we have:
and LoweredNilCheck gets eliminated. This causes the panic to happen at the MOVQload at line 11 ( One option might be that if late nilcheck eliminates a nil check X because of subsequent load L, it should change L's position information to X's. This might be a little confusing to people who are single stepping through the assembly, but it would give better (correct) stack traces for normal production builds. |
It looks like /cc @dr2chase for IsStmt stuff |
I think @mdempsky 's idea of updating the load's line number works. We're effectively combining the nil check and the load into a single instruction. We can only pick one of the line numbers, so no matter what we're losing information. For panic backtraces, it always makes sense to use the line number of the nilcheck, because that's the semantic location of the panic. For things like pprof backtraces, it isn't so clear. Maybe we want the line numbers to indicate the load location, not the nilcheck location. But that's a pretty vague objection, panic backtraces are more important IMO. There are some trickier cases, like two nilchecks with different line numbers that are both made redundant by a subsequent load. We'd need to pick the first nilcheck in the store chain. |
Change https://golang.org/cl/200197 mentions this issue: |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
This code: https://play.golang.org/p/vYYtv0x4IyJ
Produces this error:
However, if you uncomment the
fmt.Printf
statement: https://play.golang.org/p/A0mCRgbR8OJThen, you get this, which seems to be the "correct" stack trace:
What did you expect to see?
The second error.
What did you see instead?
The first error.
The text was updated successfully, but these errors were encountered: