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

Add an Android ABI #379

Closed
wants to merge 1 commit into from

Conversation

palmer-dabbelt
Copy link
Contributor

This came up in a glibc patch review. Someone from Android should probably take a look, as it's very much not my thing so I'm just guessing here...

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
@jrtc27
Copy link
Collaborator

jrtc27 commented May 16, 2023

OS-specific documentation doesn’t belong here, it belongs with the OS. Yes, this also should apply to Linux, but historically that was here so we didn’t remove it. We’re not going to have Linux, Android, FreeBSD, OpenBSD, Haiku and so on sections here, that would be ludicrous.


The Android ABIs do not have a global pointer register, along with the
GP-related relocations and relaxations. `x3` is instead used for the software
shadow call stack, with an ABI name of `sscs`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x3 is the non-ABI name, so that'd be valid regardless of which ABI behavior is being ascribed to the register (ie, it's just the register number). I don't care a ton about names, but IIUC the Android ABI is treating x3 as something very different that the global pointer, so using the same name is probably going to just trip people up.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://android-review.googlesource.com/c/platform/bionic/+/2592806 changes pthread_create.cpp to say "x3" instead of "gp" for consistency.

@MaskRay
Copy link
Collaborator

MaskRay commented May 16, 2023

I agree that OS-specific (Android) ABI doesn't belong here.

This thread is motivated by this thread: https://sourceware.org/pipermail/libc-alpha/2023-May/148045.html

As I said,

Relaxation schemes, including global pointer relaxation, are optional. Making an optional feature opt-in does not constitute an ABI break.

If you don't agree, you need to give more evidence.

@palmer-dabbelt
Copy link
Contributor Author

OK, I'll close it.

@enh-google
Copy link

I agree that OS-specific (Android) ABI doesn't belong here.

yeah, Android's differences from other OSes for other architectures haven't been documented anywhere other than in Android's documentation afaik, so not having anything in the risc-v documentation isn't a regression.

if anything, it would probably make more sense to say in psabi that gp isn't much use expect for tiny embedded systems, and that OSes may choose to reuse it? (that's more of a POSIX-style "we just describing what's actually happening in the real world" view of things.)

@palmer-dabbelt
Copy link
Contributor Author

I agree that OS-specific (Android) ABI doesn't belong here.

yeah, Android's differences from other OSes for other architectures haven't been documented anywhere other than in Android's documentation afaik, so not having anything in the risc-v documentation isn't a regression.

I agree it's not a regression, but we've also just got a source of undefined behavior floating around here that I was trying to close. If you guys are OK relying on the unspecified behavior then it's actually better for me, this way when it breaks it's not my problem.

if anything, it would probably make more sense to say in psabi that gp isn't much use expect for tiny embedded systems, and that OSes may choose to reuse it? (that's more of a POSIX-style "we just describing what's actually happening in the real world" view of things.)

There's similar language in there about platform-specific behavior, but it doesn't really fix the problem: GP is defined for the only ABI we're looking at in toolchain land, saying there might be other ABIs doesn't let us make sure we build correct systems.

@palmer-dabbelt palmer-dabbelt deleted the android branch May 16, 2023 23:28
@enh-google
Copy link

I agree it's not a regression, but we've also just got a source of undefined behavior floating around here that I was trying to close. If you guys are OK relying on the unspecified behavior then it's actually better for me, this way when it breaks it's not my problem.

i'm hoping that this doesn't live long anyway. what i really want is Zisslpcfi. the OS proper can easily switch to that (because that's effectively just "stop using x3; there's a real shadow stack stack pointer register now"), and the only question is "was x3 as a shadow stack pointer ever supported for apps?". and that's a decision for closer to when we're actually finalizing the Android ABI. (but if we have it on now, pre-release, that at least lets us turn it on if we want to. if we don't have it on from the beginning, someone will clobber x3, either accidentally or because they see a "free" register and want to use it for themselves. might sound crazy, but we've seen stuff like that with TLS slots historically, for example.) @appujee @DanAlbert

There's similar language in there about platform-specific behavior, but it doesn't really fix the problem: GP is defined for the only ABI we're looking at in toolchain land, saying there might be other ABIs doesn't let us make sure we build correct systems.

yeah, the psabi folks encouraged us in this specific x3 direction (when we said we need a register), and it would be nice to have psabi reflect that (a) gp is useless and (b) might be used for other stuff (even if they steer clear of saying what, or who might be doing that). similar to x18 (or at least how x18 was retconned) on arm64.

@palmer-dabbelt
Copy link
Contributor Author

I agree it's not a regression, but we've also just got a source of undefined behavior floating around here that I was trying to close. If you guys are OK relying on the unspecified behavior then it's actually better for me, this way when it breaks it's not my problem.

i'm hoping that this doesn't live long anyway. what i really want is Zisslpcfi. the OS proper can easily switch to that (because that's effectively just "stop using x3; there's a real shadow stack stack pointer register now"), and the only question is "was x3 as a shadow stack pointer ever supported for apps?". and that's a decision for closer to when we're actually finalizing the Android ABI. (but if we have it on now, pre-release, that at least lets us turn it on if we want to. if we don't have it on from the beginning, someone will clobber x3, either accidentally or because they see a "free" register and want to use it for themselves. might sound crazy, but we've seen stuff like that with TLS slots historically, for example.) @appujee @DanAlbert

I'd argue that re-using x3 as a shadow stack is exactly this problem: you're seeing it's unused when GP-based relaxation is disabled, and thus re-purposing it.

There's similar language in there about platform-specific behavior, but it doesn't really fix the problem: GP is defined for the only ABI we're looking at in toolchain land, saying there might be other ABIs doesn't let us make sure we build correct systems.

yeah, the psabi folks encouraged us in this specific x3 direction (when we said we need a register), and it would be nice to have psabi reflect that (a) gp is useless and (b) might be used for other stuff (even if they steer clear of saying what, or who might be doing that). similar to x18 (or at least how x18 was retconned) on arm64.

Yep, and having a GP-free ABI actually documented would be the first step towards an eventual deprecation of the GP-using ABI: we'd do the whole ELF flag thing to check, and then be able to take advantage of the extra register eventually and without a flag day.

Maybe the flags aren't a problem for Android folks because you don't yet have binaries, but there's a lot of stuff built around RISC-V and trying to just throw all those binaries away would be a huge uphill battle.

That said: I don't really care about this so I'm just going to ignore it ;)

@MaskRay
Copy link
Collaborator

MaskRay commented May 17, 2023

https://sourceware.org/pipermail/libc-alpha/2023-May/148164.html

Like I said above, you (or anyone else) is free to violate the spec and then go make sure the systems you're building based on that unspecified behavior continue to work well enough for the users.

Respectfully, I disagree.

Compilers don't generate instructions using GP relative addressing.
For -no-pie, GNU ld performs global pointer relaxation.

Assembly input files using GP (unallocatable in riscv-cc.adoc) are similar to using TP.
The end user should be responsiful for what they do.
If they break the ABI, the program will break as well, but that is not an issue made by the platform.

Conforming relocatable object files do not use GP. Existing GNU ld linked -no-pie executables do not exhibit incorrect behavior.

If a GNU ld linked -no-pie executable uses a shared object and the shared object is upgraded to use GP in a way that is incompatible with global pointer relaxation,
it would constitute an ABI break. However, a platform can well ensure that such a scenario doesn't happen by not having --relax-gp executables in the first place.

jrguzman-ms pushed a commit to msft-mirror-aosp/platform.bionic that referenced this pull request May 18, 2023
Neither is great, but "gp" seems actively misleading (and setjmp.S
says x3 every time, so we should be consistent if nothing else).

Bug: riscv-non-isa/riscv-elf-psabi-doc#379
Test: treehugger
Change-Id: Ibccda74d4794caa770b82e7ba2e31ce7b645b83f
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

Successfully merging this pull request may close these issues.

None yet

4 participants