cmd/link: remove text relocations from android libraries
gopherbot opened this issue · 51 comments
by momchil.dev:
What does 'go version' print? go version go1.4rc2 darwin/amd64 What happened? I am compiling a shared library for android/arm. I then use this library from a Java-based android application. When I run the application on a Nexus 5 device with Lollipop, I get a "libexample.so has text relocations. This is wasting memory and prevents security hardening. Please fix." If I run the same application on an old Kitkat Asus Transformer Pad TF301T, I don't get any errors at all and everything works as expected. Please provide any additional information below. I don't have much experience in developing native libraries but the fact that it works on one device and not the other seems strange. You can find the code for the library here: https://github.com/momchil-atanasov/go-android-lib You can find the code for the android app here: https://github.com/momchil-atanasov/go-android-app Hope this helps! Can't wait for the official release of 1.4.
I'm not sure what's involved in fixing this. Assigning it to myself to work out if it can be done for the 1.5 release.
I don't have NDK installed, could someone please attach a .so file compiled
by Go that triggers text relocation warnings? I want to take a look.
Here it is, I also saw this error.
http://sandbox.blokada.in.rs/libgojni.so
Actually this probably needs to be fixed in cmd/ld.
What is the output of "readelf -r" on the .so for which you are getting the warning?
Thanks. There are no text relocations in that list, so I am perplexed by the error message you reported.
Actually, I didn't opened this issue, but I saw same message as momchil.dev when running in lollipop emulator.
Looking at the readelf -r report more closely, there are indeed a lot of
text relocations. It's just the section name is .rel.dyn instead of .rel.text.
For example, the first entry of .rel.dyn is:
Offset Info Type Sym.Value Sym. Name
0010cb8c 00000017 R_ARM_RELATIVE
And the .text section does include that address:
[Nr] Name Type Addr Off Size ES Flg Lk Inf Al
[ 8] .text PROGBITS 001071d8 1071d8 271a8c 00 AX 0 0 4
Looking at the disassembly, we find out that there is indeed an absolute
address at 0x10cb8c:
10cb7c: e59fb008 ldr fp, [pc, #8] ; 10cb8c
10cb80: e5cb0000 strb r0, [fp]
10cb84: e49df004 pop {pc} ; (ldr pc, [sp], #4)
10cb88: eafffffe b 10cb88
10cb8c: 00589d6f ;; <--- this word is being relocated
It seems cmd/5l needs to know how to use GOT to access global data even for
Go data when -shared is active.
However, as the code is laid out by 5g, and 5g doesn't know the link mode
yet, I don't know how to easily solve this problem. Pass -shared to 5g?
This is probably too invasive.
Suggestions?
Thanks for checking me on the relocations.
In order to add full shared library support as outlined in https://docs.google.com/a/golang.org/document/d/1nr-TQHw_er6GOQRsF6T43GGhFDelrAP0NqSS_00RgZQ/edit#heading=h.fwmrrio0df0i I think that we are going to have to start passing a special option to all the gc compilers.
So, does this issue imply that Android support is currently unusable for Nexus devices?
I don't think that is implied. The android platform wants us to avoid text relocations, and we should move in that direction. But apps are producing a log warning, not an error.
Okay, that makes sense. I was experiencing issues on a Nexus where the application merely closes immediately after loading the Go code. I was thinking that this issue was related, but I will do more investigation into my setup before looking to Go itself as the culprit.
That sounds like an unrelated bug. Make sure the version of Go in your mobile docker image was built after bee8ae1 and watch adb logcat
for an error message. If you see something that looks like a bug, open a new issue.
momchil.dev: The so
that I was using can be found here:
https://github.com/momchil-atanasov/go-android-lib/releases/tag/v0.1.0
That repository also shows how I compile that so.
The behaviour that I observe is that the application dies immediately after start up. I don't get any error messages, just that text relocation warning which I am not sure if it is totally related. I don't get that warning on my Tablet (running older Android), where the so
works. It could be just that Lollipop has a new text relocation check, that the older one does, yet the issue to be caused by something else.
In fact, what i stumbled upon is this issue:
https://groups.google.com/forum/#!topic/golang-dev/NB6jPV9gddQ
It is possible that this is what is actually happening. I am not sure how to do the low level log tracing that they have done there in order to verify that.
P.S. After my issue was moved to GitHub I never received a notification that there was any activity going on. Sorry for the late reply!
I would recommend recreating the Docker image and trying again. The virtual memory issue should have been fixed by #9311.
Sorry, I was not aware that Docker had anything to do with Android.
I am testing on a real Nexus 5 device. Just yesterday I updated it to 5.0.1 and the issue is still there. Does the real device use Docker at all or is this how the Emulator runs Android?
Just to clarify a bit, running the so
on a Nexus 5 emulator actually works but not on the real Nexus 5 device.
Also, something else I observed is that when the process crashes, Android attempts to boot the process two more times (failing the same way) and then gives up.
Sorry, I should have been more clear. Android doesn't run Docker, but the Golang recommended method of compiled for Android (here) involves running the compilation inside a Docker container.
If you update the version of Go you were using, it should fix the issue with crashing before user code is loaded (and Android attempts to rerun it, yes) as per #9311. However, if you can confirm that some of your Go code is executing before the crash, then that is a different issue, and one that I am trying to figure out now.
I was not aware of the docker build process. This must be something new. I will give it a try and will document by findings.
As far as I know, no Go code manages to execute at all before the process dies. I think the process crashes during library load or initialization.
Thanks!
@huntaub thanks!
With the Docker build it now works on a Nexus 5 device. I still get the W/linker﹕ libexample.so has text relocations.
message but the app starts and the Go code runs without a problem.
This issue could still be left open until that warning is solved but the priority has surely dropped.
Android doesn't allow multiple instances of the same app, so there will be
at most two copies of the text section in memory (the real one used by the
process and the one in file system cache), so I don't think this is a
serious issue at the moment.
Is it possible that this bug is just as simple as the fact that since the move to liblink, all the code to generate pc relative code on arm doesn't fire because it is now part of 5g and 5g never sets the Flag_shared flag? In which case 530fce7 may fix this, largely by accident.
I encountered the same issue on android/x86_64 platform
@minux am I right to think that to remove constant pools we need to add an R_ADDRARM relocation along the lines of the R_ADDRPOWER and R_ADDRARM64 relocations? Is there anything that's going to make that particularly hard for ARM?
(I expect to be working on this during the 1.6 cycle fwiw)
No, really, I was right back in March: if you manage to get Flag_shared to be set when compiling, the relocations against code go away. You still get relocations against the text segment but I'm pretty sure that's the same reason as shared libraries on intel have TEXTREL set, i.e. #10914
@mwhudson while I think you're right that Flag_shared should be set, doing so still produces a (rather imprecise) warning on android:
W/linker ( 4924): libbasic.so has text relocations. This is wasting memory and prevents security hardening. Please fix.
So I think the relocations against the text segment need to be dealt with to fix this.
If you're planning to work on this, which would be really great, would you like to assign yourself this issue?
Yeah, I'm definitely planning to fix this in 1.6 (I think I have branches that do all of it scattered around actually). But I don't have permissions to assign the bug to myself afaics
CL https://golang.org/cl/10300 mentions this issue.
CL https://golang.org/cl/14306 mentions this issue.
Hi, I think this should be fixed in tip now, but am not in a position to check myself. Can someone do that?
Hi, I just checked and there is still warning about the text relocations.
Lib and readelf -r output are here:
http://sandbox.blokada.in.rs/libgojni.so.gz
http://sandbox.blokada.in.rs/readelf.txt
Yeah, that certainly still has text relocations. How did you make it? -shared needs to be passed to go tool compile, and go -buildmode=c-shared does that now (since 876b7cc), but if there is some other process going on, maybe something else needs to be fixed.
I compiled it with gomobile bind, this is from verbose output, -buildmode=c-shared is used:
GOOS=android GOARCH=arm GOARM=7 CC=$GOMOBILE/android-ndk-r10e/arm/bin/arm-linux-androideabi-gcc CXX=$GOMOBILE/android-ndk-r10e/arm/bin/arm-linux-androideabi-g++ CGO_ENABLED=1 go build -p=2 -pkgdir=$GOMOBILE/pkg_android_arm -tags="" -v -x -buildmode=c-shared -o=$WORK/android/src/main/jniLibs/armeabi-v7a/libgojni.so $WORK/androidlib/main.go
"go version"?
go version devel +d862753 Wed Sep 9 05:29:20 2015 +0000 linux/amd64
Well that's disappointing. Can you put the output of running that command in a gist? Can you add -a to the command line and try again, just to be sure?
Sure, here is the output and readelf again:
https://gist.github.com/gen2brain/2f35a965f55066954e7d
https://gist.github.com/anonymous/1d4c7f1e089bbd05fd9b
The only thing I can think is that the standard library (runtime etc) needs to be rebuilt -- it does't seem that gomobile passes -a on to the go tool. Maybe I need to set gomobile up and try to figure out what's going on myself, but you could try removing $GOROOT/pkg/android_arm* and trying again...
This time with $GOROOT/pkg/android_arm/* deleted, link to readelf gist is at the end:
Well I've installed gomobile and can reproduce this and don't at all understand what is going on. Will dig further.
Ah it's down to gomobile passing pkgdir to the go tool I think.
Yeah it's gomobile's fault (#12581) if I build the so directly with the go tool it does not have TEXTREL set.
Great work on this @mwhudson!
Will this also fix text relocations on Android PIE executables that the system linker warns about (see e.g. #10807 (comment)), or should that be a separate issue since this issue is about shared libraries?
@fornwall I think it's related, I don't know enough about the build processes to know if it's the same -- basically you now need to pass -shared to go tool compile and go tool asm when compiling all the code. I don't know how best to do that! I guess someone (me?) should implement -buildmode=pie support in the go tool...
any updates here? I built an android app using go, and while it runs, I get an ugly warning every time.
Workaround is to compile libgojni.so manually and replace it in aar.
# gomobile bind -v -x -work -o /tmp/bukanir.aar -target android bukanir 2>&1 >/dev/null | grep "^WORK="
WORK=/tmp/gomobile-work-686967262
Now, point CC and CXX to arm toolchain you made with make-standalone-toolchain.sh from ndk. Toolchain that comes with gomobile init is maybe stripped too much, in my case it didn't tried to link library at all.
# export PATH=/opt/android-toolchain-arm/bin:${PATH}
# GOOS=android GOARCH=arm GOARM=7 CGO_ENABLED=1 go build -v -x -buildmode=c-shared -o=/tmp/libgojni.so /tmp/gomobile-work-686967262/androidlib/main.go
And then unzip aar, replace libgojni.so and zip back. No more warning about text relocations.
I think now that https://go-review.googlesource.com/#/c/15803/ has landed this is fixed? @crawshaw?
The issue in gomobile generated libraries is fixed, but binaries still need to be built for Android as PIC, which could be https://go-review.googlesource.com/#/c/12559 or it could be -buildmode=pie in cmd/go and using it as the default build mode on Android. I'll take a look at that after #12896.