This is an archive of the discontinued LLVM Phabricator instance.

[MC] Emit ELF files with ELFOSABI_GNU if specified in triple
Needs RevisionPublic

Authored by MoritzS on Jan 12 2021, 1:44 AM.

Details

Summary

This makes a slight difference when using GNU binutils to inspect ELF
files. For example when defining a symbol of type STT_GNU_IFUNC, readelf
shows this with ELFOSABI_GNU:

3: 0000000000000001     0 IFUNC   GLOBAL DEFAULT    2 my_ifunc

But this with ELFOSABI_NONE:

3: 0000000000000001     0 <OS specific>: 10 GLOBAL DEFAULT    2 my_ifunc

Diff Detail

Event Timeline

MoritzS created this revision.Jan 12 2021, 1:44 AM
MoritzS requested review of this revision.Jan 12 2021, 1:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2021, 1:44 AM
MoritzS added inline comments.Jan 12 2021, 2:07 AM
llvm/include/llvm/MC/MCELFObjectWriter.h
74

Should I add the clang-format changes here? This adds quite a lot of changes in lines that were not changed.

MoritzS updated this revision to Diff 316030.Jan 12 2021, 2:10 AM

Fixed clang-format error

MoritzS marked an inline comment as not done.Jan 12 2021, 2:11 AM
MaskRay requested changes to this revision.EditedJan 12 2021, 9:51 AM

This is known. GNU as by default emits ELFOSABI_NONE on Linux - if it does not use any GNU extensioin. If the assembly uses STT_GNU_IFUNC/STB_GNU_UNIQUE, GNU as will switch to ELFOSABI_GNU.

The patch improves the few cases but regresses other cases, so I am requesting changes. It is also unclear whether respecting the GNU behavior is strictly necessary (see gold https://sourceware.org/bugzilla/show_bug.cgi?id=17735). STT_GNU_IFUNC is GNU extension and it is adopted by some other OSes. I am not sure the readelf -S output is a justifying reason for this change.

This revision now requires changes to proceed.Jan 12 2021, 9:51 AM

Looking at the original motivation for the patch I'm not convinced that llvm-readelf etc shouldn't just fall back on displaying the "normal" set of GNU extensions even for ELFOSABI_NONE (i.e. those that FreeBSD and GNU have in common). Things like that are really just part of de-facto generic ELF these days. What does binutils do?

Having said that I do think it makes sense to push the getOS call into getOSABI by passing the full triple as it simplifies all the callers (and that also decouples the NFC refactor from your proposed behavioural change).

MaskRay added a comment.EditedJan 12 2021, 10:05 AM

Looking at the original motivation for the patch I'm not convinced that llvm-readelf etc shouldn't just fall back on displaying the "normal" set of GNU extensions even for ELFOSABI_NONE (i.e. those that FreeBSD and GNU have in common). Things like that are really just part of de-facto generic ELF these days. What does binutils do?

For SHF_GNU_RETAIN (new, not in MC), SHT_GNU_IFUNC (useful), SHF_GNU_MBIND (looks like legacy), STB_GNU_UNIQUE, has_gnu_osabi bit is set and emit ELFOSABI_GNU instead of ELFOSABI_NONE for Linux triples. I think FreeBSD uses by ELFOSABI_FREEBSD default and can use such extensions as well. Other triples cannot use these *_GNU_*.

This is known. GNU as by default emits ELFOSABI_NONE on Linux - if it does not use any GNU extensioin. If the assembly uses STT_GNU_IFUNC/STB_GNU_UNIQUE, GNU as will switch to ELFOSABI_GNU.

The patch improves the few cases but regresses other cases, so I am requesting changes. It is also unclear whether respecting the GNU behavior is strictly necessary (see gold https://sourceware.org/bugzilla/show_bug.cgi?id=17735). STT_GNU_IFUNC is GNU extension and it is adopted by some other OSes. I am not sure the readelf -S output is a justifying reason for this change.

I agree with you that it may not be strictly necessary to set ELFOSABI_GNU just because a GNU extension like IFUNC is used. However, if you do use such an extension and explicitly use the x86_64-unknown-linux-gnu triple for example (which is how I found this), I think it makes sense to use ELFOSABI_GNU.

The failing tests use *-gnu triples and still expect ELFOSABI_NONE, so that's why they fail.

I can write more code that checks if any GNU extension are actually used. Only if they are and you are also using a *-gnu triple, it will then use ELFOSABI_GNU. Would you agree to that kind of behavior?

Looking at the original motivation for the patch I'm not convinced that llvm-readelf etc shouldn't just fall back on displaying the "normal" set of GNU extensions even for ELFOSABI_NONE (i.e. those that FreeBSD and GNU have in common). Things like that are really just part of de-facto generic ELF these days. What does binutils do?

The snippets from readelf from the patch are actually from GNU binutils readelf, not from llvm-readelf. So binutils does in fact only display the GNU extensions with ELFOSABI_GNU.

bcain added a subscriber: bcain.Feb 2 2021, 7:04 AM

This is known. GNU as by default emits ELFOSABI_NONE on Linux - if it does not use any GNU extensioin. If the assembly uses STT_GNU_IFUNC/STB_GNU_UNIQUE, GNU as will switch to ELFOSABI_GNU.

The patch improves the few cases but regresses other cases, so I am requesting changes. It is also unclear whether respecting the GNU behavior is strictly necessary (see gold https://sourceware.org/bugzilla/show_bug.cgi?id=17735). STT_GNU_IFUNC is GNU extension and it is adopted by some other OSes. I am not sure the readelf -S output is a justifying reason for this change.

I agree with you that it may not be strictly necessary to set ELFOSABI_GNU just because a GNU extension like IFUNC is used. However, if you do use such an extension and explicitly use the x86_64-unknown-linux-gnu triple for example (which is how I found this), I think it makes sense to use ELFOSABI_GNU.

The failing tests use *-gnu triples and still expect ELFOSABI_NONE, so that's why they fail.

I can write more code that checks if any GNU extension are actually used. Only if they are and you are also using a *-gnu triple, it will then use ELFOSABI_GNU. Would you agree to that kind of behavior?

Yes. We may need to do so for at least SHF_GNU_RETAIN.
https://sourceware.org/pipermail/binutils/2021-March/115581.html

STT_GNU_IFUNC/STB_GNU_UNIQUE probably need the similar behavior but they are not urgent.

static uint8_t getOSABI(const Triple &T) { may need to be static uint8_t getOSABI(bool UsedGnuAbi) {

This is known. GNU as by default emits ELFOSABI_NONE on Linux - if it does not use any GNU extensioin. If the assembly uses STT_GNU_IFUNC/STB_GNU_UNIQUE, GNU as will switch to ELFOSABI_GNU.

The patch improves the few cases but regresses other cases, so I am requesting changes. It is also unclear whether respecting the GNU behavior is strictly necessary (see gold https://sourceware.org/bugzilla/show_bug.cgi?id=17735). STT_GNU_IFUNC is GNU extension and it is adopted by some other OSes. I am not sure the readelf -S output is a justifying reason for this change.

I agree with you that it may not be strictly necessary to set ELFOSABI_GNU just because a GNU extension like IFUNC is used. However, if you do use such an extension and explicitly use the x86_64-unknown-linux-gnu triple for example (which is how I found this), I think it makes sense to use ELFOSABI_GNU.

The failing tests use *-gnu triples and still expect ELFOSABI_NONE, so that's why they fail.

I can write more code that checks if any GNU extension are actually used. Only if they are and you are also using a *-gnu triple, it will then use ELFOSABI_GNU. Would you agree to that kind of behavior?

Yes. We may need to do so for at least SHF_GNU_RETAIN.
https://sourceware.org/pipermail/binutils/2021-March/115581.html

STT_GNU_IFUNC/STB_GNU_UNIQUE probably need the similar behavior but they are not urgent.

static uint8_t getOSABI(const Triple &T) { may need to be static uint8_t getOSABI(bool UsedGnuAbi) {

Sent D97976