Page MenuHomePhabricator

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

Authored by MoritzS on Tue, Jan 12, 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

Unit TestsFailed

TimeTest
40 msx64 debian > LLVM.MC/AArch64::elf_osabi_flags.s
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-mc -filetype=obj -triple aarch64 /mnt/disks/ssd0/agent/llvm-project/llvm/test/MC/AArch64/elf_osabi_flags.s -o -| /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-readobj -h - | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/llvm/test/MC/AArch64/elf_osabi_flags.s
50 msx64 debian > LLVM.MC/X86::inline-asm-obj.ll
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llc /mnt/disks/ssd0/agent/llvm-project/llvm/test/MC/X86/inline-asm-obj.ll -o - | /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-mc -triple=x86_64-pc-linux -o /mnt/disks/ssd0/agent/llvm-project/build/test/MC/X86/Output/inline-asm-obj.ll.tmp1 -filetype=obj
1,620 msx64 debian > lld.ELF::emulation-mips.s
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-mc -filetype=obj -triple=mips-unknown-linux /mnt/disks/ssd0/agent/llvm-project/lld/test/ELF/emulation-mips.s -o /mnt/disks/ssd0/agent/llvm-project/build/tools/lld/test/ELF/Output/emulation-mips.s.tmpmips
1,230 msx64 debian > lld.ELF::emulation-x86.s
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llvm-mc -filetype=obj -triple=x86_64-unknown-freebsd /mnt/disks/ssd0/agent/llvm-project/lld/test/ELF/emulation-x86.s -o /mnt/disks/ssd0/agent/llvm-project/build/tools/lld/test/ELF/Output/emulation-x86.s.tmpx64
80 msx64 windows > LLVM.MC/AArch64::elf_osabi_flags.s
Script: -- : 'RUN: at line 1'; c:\ws\w16c2-1\llvm-project\premerge-checks\build\bin\llvm-mc.exe -filetype=obj -triple aarch64 C:\ws\w16c2-1\llvm-project\premerge-checks\llvm\test\MC\AArch64\elf_osabi_flags.s -o -| c:\ws\w16c2-1\llvm-project\premerge-checks\build\bin\llvm-readobj.exe -h - | c:\ws\w16c2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16c2-1\llvm-project\premerge-checks\llvm\test\MC\AArch64\elf_osabi_flags.s
View Full Test Results (8 Failed)

Event Timeline

MoritzS created this revision.Tue, Jan 12, 1:44 AM
MoritzS requested review of this revision.Tue, Jan 12, 1:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Jan 12, 1:44 AM
MoritzS added inline comments.Tue, Jan 12, 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.Tue, Jan 12, 2:10 AM

Fixed clang-format error

MoritzS marked an inline comment as not done.Tue, Jan 12, 2:11 AM
MaskRay requested changes to this revision.EditedTue, Jan 12, 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.Tue, Jan 12, 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.EditedTue, Jan 12, 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.