-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/link: nil pointer dereference crash when building with an Android NDK toolchain #34788
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
Comments
I'll take look. |
(My previous comment is wrong.) |
This appears to be related to the recent changes to the way TLS works on Android (see issue 29674). As part of Go 1.13 we transitioned to a new TLS access sequence for Android that loads up the newly defined "runtime.tls_g" symbol. If you compile a Go source file with GOARCH=386 GOOS=android go tool compile x.go Here is what the prolog looks like (from go tool objdump):
Note the R_TLS_LE relocation. Here is the description from
At the point of the crash in the linker, we're trying to process an R_TLS_LE relocation on a text symbol (regular Go function), but the relocation's Xsym field is nil, so it tries to execute this line: Errorf(s, "missing xsym in relocation %#v %#v", r.Sym.Name, s) which faults because r.Sym is also nil. The code that ordinarily would have filled the r.Sym is here: cmd/link/internal/ld/data.go:230
however ctxt.Tlsg is now also nil due to CL 169618. Not quite sure what the right fix is here-- as I understand it we are no longer using the "local exec" model for TLS (is this right?) in which case we should not have the R_TLS_RE in this case? |
Thanks @thanm for looking into this!
I think this is the right fix. |
OK, I have a tentative fix, which I will send shortly. I am wondering where would be the place to put a test for this... I will consult with the team. |
Change https://golang.org/cl/200337 mentions this issue: |
Great! Should the build succeed after this patch, or simply error without a panic? |
@mvdan the build should succeed. |
@gopherbot Please open a backport to 1.13. |
Backport issue(s) opened: #34825 (for 1.13). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
1.12 only receives security updates at this point, so I don't think this fix qualifies. Also, I'm pretty sure the bug only appeared in 1.13. |
I agree with @mvdan , this change depends on other things in 1.13 -- don't think it would be easily back-portable to 1.12. |
Both 1.12 and 1.13 are equally supported. Thank you for confirming that the fix isn't necessary for 1.12. |
The only requisite is a version of Go and the NDK r18 installed at
$ANROID_NDK
. I can reproduce ongo version devel +a38a917aee Wed Oct 9 06:14:44 2019 +0000 linux/amd64
andgo version go1.13.1 linux/amd64
.Below is the script to reproduce the crash:
Here is the output from Go master:
And from 1.13.1:
It seems like the line it fails on is an error line, so I assume this should error gracefully instead of panicking. I'm not sure though.
My intuition is that the build should succeed; this project in question (@grote's) was previously using Go 1.11.x without an issue. If the build should succeed here too, I'd suggest we backport a fix for 1.13.x.
/cc @cherrymui @mdempsky @ianlancetaylor @thanm as per the owners doc.
The text was updated successfully, but these errors were encountered: