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

ARM build creates broken PIC with TEXTREL #119

Closed
ghost opened this issue May 10, 2015 · 14 comments
Closed

ARM build creates broken PIC with TEXTREL #119

ghost opened this issue May 10, 2015 · 14 comments

Comments

@ghost
Copy link

ghost commented May 10, 2015

Just tested on armhf with build script from https://launchpad.net/~random-stuff/+archive/ubuntu/ppa

The build contains TEXTREL (aka - which makes the .so a broken/not PIC shared object)

CFLAGS="-g3 -O0" make -C projects/unix/ NEON=1 USE_GLES=1 OPTFLAGS=""  all -j4
readelf -d projects/unix/libmupen64plus.so.2.0.0|grep -i TEXTREL
 0x00000016 (TEXTREL)                    0x0
scanelf -qT projects/unix/libmupen64plus.so.2.0.0
  libmupen64plus.so.2.0.0: (memory/data?) [0xD6600] in (optimized out: previous .B8) [0xD65CC]
  libmupen64plus.so.2.0.0: (memory/data?) [0xD6604] in (optimized out: previous .jiptr) [0xD6600]
  libmupen64plus.so.2.0.0: (memory/data?) [0xD6608] in (optimized out: previous .jdptr) [0xD6604]
  libmupen64plus.so.2.0.0: (memory/data?) [0xD660C] in (optimized out: previous .tlbptr) [0xD6608]
  libmupen64plus.so.2.0.0: (memory/data?) [0xD696C] in (optimized out: previous new_dyna_start) [0xD6938]
  libmupen64plus.so.2.0.0: (memory/data?) [0xD6970] in (optimized out: previous .dlptr) [0xD696C]
  projects/unix/libmupen64plus.so.2.0.0

A valid PIC build is created when compiling without NEW_DYNAREC:

CFLAGS="-g3 -O0" make -C projects/unix/ NEON=1 USE_GLES=1 OPTFLAGS="" DYNAREC="" -j4 all
readelf -d projects/unix/libmupen64plus.so.2.0.0|grep -i TEXTREL
scanelf -qT projects/unix/libmupen64plus.so.2.0.0

All broken instruction/definitions which require a relocation can be found in src/r4300/new_dynarec/arm/linkage_arm.S

@littleguy77
Copy link
Member

So is this the source of this warning in the Android logcat?

W/linker(24206): libmupen64plus-core.so has text relocations. This is wasting memory and prevents security hardening. Please fix.

@ghost
Copy link
Author

ghost commented May 10, 2015

Yes, this is the source of this warning and many other problems

@Gillou68310
Copy link
Member

@conchurnavid Why trying to build the core as a PIC shared library? IMO load-time relocation should be faster (at runtime) than PIC in this case.

@ghost
Copy link
Author

ghost commented May 13, 2015

For example because the new dynarec arm code automatically enables PIC?

Hardening? ...

@Gillou68310
Copy link
Member

PIC is only enabled if using the -fPIC option at compile time, otherwise a non-PIC shared library is created. On android we're using non-PIC libraries.

@ghost
Copy link
Author

ghost commented May 13, 2015

Android is even requiring -fPIE -pie for newer API levels. Also the standard toolchain setup of the NDK sets -fPIC or -fpic since ages for almost (all?) targets. Check toolchains/arm-linux-androideabi-*/setup.mk

@Gillou68310
Copy link
Member

Yep you're right I totally missed this one, thanks for the info ;-)

@Gillou68310
Copy link
Member

Lol the core library don't even link without PIC anyway, I now understand the issue here. Better late than never ;-)

@Gillou68310
Copy link
Member

@conchurnavid in case you missed it @ecsv created a patch to fix the TEXTREL issue at
http://anonscm.debian.org/cgit/collab-maint/mupen64plus-core.git/tree/debian/patches/arm_pic_notextrel.patch?h=armhf_test&id=cd15ffe60ce3363d3ff78374f082e8f4b42407f0

Tested and working on android ;-)

@ghost
Copy link
Author

ghost commented May 26, 2015

Thanks. I couldn't find a pull request. Created my own at #120 with the patch from Debian

@Gillou68310
Copy link
Member

Thanks for creating the PR ;-)

@ghost
Copy link
Author

ghost commented Jun 13, 2015

PR was merged but then the problem was reintroduced by 87b9b57

@Gillou68310
Copy link
Member

@richard42, @Narann this issue can be closed

@Narann
Copy link
Member

Narann commented Aug 25, 2016

Thanks!

@Narann Narann closed this as completed Aug 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants