Skip to content
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/link: remove text relocations from android libraries #9210

Closed
gopherbot opened this issue Dec 5, 2014 · 51 comments

Comments

Projects
None yet
@gopherbot
Copy link

commented Dec 5, 2014

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.

@gopherbot gopherbot added the new label Dec 5, 2014

@crawshaw crawshaw self-assigned this Dec 11, 2014

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2014

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.

@minux

This comment has been minimized.

Copy link
Member

commented Dec 11, 2014

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.

@hyangah hyangah added the repo-mobile label Dec 11, 2014

@gen2brain

This comment has been minimized.

Copy link

commented Dec 11, 2014

Here it is, I also saw this error.
http://sandbox.blokada.in.rs/libgojni.so

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2014

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?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2014

Thanks. There are no text relocations in that list, so I am perplexed by the error message you reported.

@gen2brain

This comment has been minimized.

Copy link

commented Dec 11, 2014

Actually, I didn't opened this issue, but I saw same message as momchil.dev when running in lollipop emulator.

@minux

This comment has been minimized.

Copy link
Member

commented Dec 11, 2014

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?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2014

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.

@jhleath

This comment has been minimized.

Copy link

commented Dec 13, 2014

So, does this issue imply that Android support is currently unusable for Nexus devices?

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Dec 13, 2014

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.

@crawshaw crawshaw removed their assignment Dec 13, 2014

@crawshaw crawshaw added repo-main and removed repo-mobile labels Dec 13, 2014

@crawshaw crawshaw changed the title golang.org/x/mobile/app: lib has text relocations cmd/ld: remove text relocations from android libraries Dec 13, 2014

@jhleath

This comment has been minimized.

Copy link

commented Dec 13, 2014

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.

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Dec 13, 2014

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.

@bradfitz bradfitz removed the new label Dec 18, 2014

@mokiat

This comment has been minimized.

Copy link

commented Dec 19, 2014

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!

@jhleath

This comment has been minimized.

Copy link

commented Dec 19, 2014

I would recommend recreating the Docker image and trying again. The virtual memory issue should have been fixed by #9311.

@mokiat

This comment has been minimized.

Copy link

commented Dec 19, 2014

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.

@jhleath

This comment has been minimized.

Copy link

commented Dec 19, 2014

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.

@mokiat

This comment has been minimized.

Copy link

commented Dec 19, 2014

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!

@mokiat

This comment has been minimized.

Copy link

commented Dec 19, 2014

@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.

@minux

This comment has been minimized.

Copy link
Member

commented Dec 19, 2014

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.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2015

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.

gopherbot pushed a commit that referenced this issue Sep 4, 2015

cmd/internal/ld: put read-only relocated data into .data.rel.ro when …
…making a shared object

Currently Go produces shared libraries that cannot be shared between processes
because they have relocations against the text segment (not text section). This
fixes this by moving some data to sections with magic names recognized by the
static linker.

Fixes #10914
Updates #9210

Change-Id: I7178daadc0ae87953d5a084aa3d580f4e3b46d47
Reviewed-on: https://go-review.googlesource.com/10300
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot

This comment has been minimized.

Copy link
Author

commented Sep 6, 2015

CL https://golang.org/cl/14306 mentions this issue.

gopherbot pushed a commit that referenced this issue Sep 8, 2015

cmd/internal/ld: put read-only relocated data into .data.rel.ro when …
…making a shared object

Currently Go produces shared libraries that cannot be shared between processes
because they have relocations against the text segment (not text section). This
fixes this by moving some data to sections with magic names recognized by the
static linker.

The change in genasmsym to add STYPELINK to the switch should fix things on
darwin/arm64.

Fixes #10914
Updates #9210

Change-Id: Iab4a6678dd04cec6114e683caac5cf31b1063309
Reviewed-on: https://go-review.googlesource.com/14306
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 9, 2015

Hi, I think this should be fixed in tip now, but am not in a position to check myself. Can someone do that?

@gen2brain

This comment has been minimized.

Copy link

commented Sep 9, 2015

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

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

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.

@gen2brain

This comment has been minimized.

Copy link

commented Sep 10, 2015

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

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

"go version"?

@gen2brain

This comment has been minimized.

Copy link

commented Sep 10, 2015

go version devel +d862753 Wed Sep 9 05:29:20 2015 +0000 linux/amd64

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

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?

@gen2brain

This comment has been minimized.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

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...

@gen2brain

This comment has been minimized.

Copy link

commented Sep 10, 2015

This time with $GOROOT/pkg/android_arm/* deleted, link to readelf gist is at the end:

https://gist.github.com/gen2brain/b25bf3aad576e4f00b34

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

Well I've installed gomobile and can reproduce this and don't at all understand what is going on. Will dig further.

@minux

This comment has been minimized.

Copy link
Member

commented Sep 10, 2015

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

Ah it's down to gomobile passing pkgdir to the go tool I think.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2015

Yeah it's gomobile's fault (#12581) if I build the so directly with the go tool it does not have TEXTREL set.

@fornwall

This comment has been minimized.

Copy link

commented Sep 11, 2015

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?

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2015

@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...

@acornejo

This comment has been minimized.

Copy link

commented Oct 13, 2015

any updates here? I built an android app using go, and while it runs, I get an ugly warning every time.

@gen2brain

This comment has been minimized.

Copy link

commented Oct 13, 2015

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.

@mwhudson

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2015

I think now that https://go-review.googlesource.com/#/c/15803/ has landed this is fixed? @crawshaw?

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2015

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.

@crawshaw

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2015

Closing this as we have #10807 for the remaining -buildmode=pie work, and everyone running into this is using the gomobile tool, which is now fixed.

@crawshaw crawshaw closed this Oct 16, 2015

mwhudson added a commit to mwhudson/go that referenced this issue Jan 4, 2016

cmd/internal/ld: put read-only relocated data into .data.rel.ro when …
…making a shared object

Currently Go produces shared libraries that cannot be shared between processes
because they have relocations against the text segment (not text section). This
fixes this by moving some data to sections with magic names recognized by the
static linker.

The change in genasmsym to add STYPELINK to the switch should fix things on
darwin/arm64.

Fixes golang#10914
Updates golang#9210

Change-Id: Iab4a6678dd04cec6114e683caac5cf31b1063309

@golang golang locked and limited conversation to collaborators Oct 17, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.