This is an archive of the discontinued LLVM Phabricator instance.

Differential D76151

[MC][X86] Allow SHT_PROGBITS for .eh_frame on x86-64
ClosedPublic

Authored by MaskRay on Mar 13 2020, 12:56 PM.

Details

Reviewers
• espindola
joerg
Commits
rGbf60953faf3a: [MC][X86] Allow SHT_PROGBITS for .eh_frame on x86-64
Summary

GNU as emits SHT_PROGBITS .eh_frame by default for .cfi_* directives.
We follow x86-64 psABI and use SHT_X86_64_UNWIND for .eh_frame

Don't error for SHT_PROGBITS .eh_frame on x86-64.
This keeps compatibility for .section .eh_frame,"a",@progbits in existing assembly files.

See https://groups.google.com/d/msg/x86-64-abi/7sr4E6THl3g/zUU2UPHOAQAJ
for more discussions.

Diff Detail

Event Timeline

MaskRay created this revision.Mar 13 2020, 12:56 PM
Herald added a reviewer: • espindola. · View Herald TranscriptMar 13 2020, 12:56 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: llvm-commits, hiraditya, fedor.sergeev, emaste. · View Herald Transcript
MaskRay edited the summary of this revision. (Show Details)Mar 13 2020, 1:11 PM
Harbormaster completed remote builds in B49177: Diff 250284.Mar 13 2020, 2:34 PM
MaskRay updated this revision to Diff 250597.Mar 16 2020, 10:36 AM
MaskRay edited the summary of this revision. (Show Details)

Repurpose

Harbormaster failed remote builds in B49334: Diff 250597!Mar 16 2020, 11:27 AM
joerg added a subscriber: joerg.Mar 16 2020, 1:31 PM

This looks much better than the in-tree state, thanks.

MaskRay updated this revision to Diff 250673.Mar 16 2020, 6:59 PM
MaskRay retitled this revision from [MC] Don't use SHT_X86_64_UNWIND for .eh_frame on non-Solaris OSes to [MC][X86] Allow SHT_PROGBITS for .eh_frame on x86-64.

Fix test. Fix title.

Harbormaster completed remote builds in B49383: Diff 250673.Mar 16 2020, 8:44 PM
joerg accepted this revision.Mar 28 2020, 3:42 PM

Can you add a variation with .pushsection for @progbits and @nobits? Otherwise LGTM.

This revision is now accepted and ready to land.Mar 28 2020, 3:42 PM
MaskRay added a comment.Mar 28 2020, 5:08 PM
In D76151#1948225, @joerg wrote:

Can you add a variation with .pushsection for @progbits and @nobits? Otherwise LGTM.

I think a .pushsection test will be redundant as it shares code with .section.

I am waiting for the resolution https://groups.google.com/forum/#!topic/x86-64-abi/7sr4E6THl3g , otherwise I cannot say "x86-64 psABI allows SHT_PROGBITS as an alternative section type" :/

joerg added a comment.Mar 28 2020, 5:18 PM

We tend to test push .section and .pushsection in this file, so it would be nice to continue doing that.

grimar added a subscriber: grimar.Mar 30 2020, 4:29 AM
MaskRay updated this revision to Diff 258089.Apr 16 2020, 10:42 AM
MaskRay edited the summary of this revision. (Show Details)

Mention GNU as but drop mention of x86-64 psABI change (which hasn't happened yet and may not happen)

Harbormaster failed remote builds in B53585: Diff 258089!Apr 16 2020, 11:09 AM
Closed by commit rGbf60953faf3a: [MC][X86] Allow SHT_PROGBITS for .eh_frame on x86-64 (authored by MaskRay). · Explain WhyApr 16 2020, 11:09 AM
This revision was automatically updated to reflect the committed changes.
MaskRay mentioned this in D82852: [ELF][test] Add some additional .eh_frame/.eh_frame_hdr testing.Jul 1 2020, 4:55 PM
atanasyan mentioned this in D107707: [MC][ELF] Do not error on parsing .debug_* section directive for MIPS.Aug 7 2021, 10:30 PM
atanasyan mentioned this in rG990e8025b5fc: [MC][ELF] Do not error on parsing .debug_* section directive for MIPS.Aug 8 2021, 10:55 PM

Revision Contents

PathSize
llvm/
lib/
MC/
MCParser/
6 lines
test/
MC/
ELF/
8 lines

Diff 258098

llvm/lib/MC/MCParser/ELFAsmParser.cpp

Show First 20 Lines • Show All 631 Lines • ▼ Show 20 Lines if (const MCSectionELF *Section =
GroupName = Group->getName(); GroupName = Group->getName();
Flags |= ELF::SHF_GROUP; Flags |= ELF::SHF_GROUP;
} }
} }
MCSectionELF *Section = getContext().getELFSection( MCSectionELF *Section = getContext().getELFSection(
SectionName, Type, Flags, Size, GroupName, UniqueID, LinkedToSym); SectionName, Type, Flags, Size, GroupName, UniqueID, LinkedToSym);
getStreamer().SwitchSection(Section, Subsection); getStreamer().SwitchSection(Section, Subsection);
if (Section->getType() != Type) // x86-64 psABI names SHT_X86_64_UNWIND as the canonical type for .eh_frame,
// but GNU as emits SHT_PROGBITS .eh_frame for .cfi_* directives. Don't error
// for SHT_PROGBITS .eh_frame
if (Section->getType() != Type &&
!(SectionName == ".eh_frame" && Type == ELF::SHT_PROGBITS))
Error(loc, "changed section type for " + SectionName + ", expected: 0x" + Error(loc, "changed section type for " + SectionName + ", expected: 0x" +
utohexstr(Section->getType())); utohexstr(Section->getType()));
if (Section->getFlags() != Flags) if (Section->getFlags() != Flags)
Error(loc, "changed section flags for " + SectionName + ", expected: 0x" + Error(loc, "changed section flags for " + SectionName + ", expected: 0x" +
utohexstr(Section->getFlags())); utohexstr(Section->getFlags()));
if (Section->getEntrySize() != Size) if (Section->getEntrySize() != Size)
Error(loc, "changed section entsize for " + SectionName + Error(loc, "changed section entsize for " + SectionName +
", expected: " + Twine(Section->getEntrySize())); ", expected: " + Twine(Section->getEntrySize()));
▲ Show 20 Lines • Show All 260 Lines • Show Last 20 Lines

llvm/test/MC/ELF/section-type-changed.s

# RUN: not llvm-mc -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error: # RUN: not llvm-mc -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
.section .foo,"a",@progbits .section .foo,"a",@progbits
# CHECK: {{.*}}.s:[[# @LINE+1]]:1: error: changed section type for .foo, expected: 0x1 # CHECK: {{.*}}.s:[[# @LINE+1]]:1: error: changed section type for .foo, expected: 0x1
.section .foo,"a",@init_array .section .foo,"a",@init_array
# CHECK: {{.*}}.s:[[# @LINE+1]]:1: error: changed section type for .foo, expected: 0x1 # CHECK: {{.*}}.s:[[# @LINE+1]]:1: error: changed section type for .foo, expected: 0x1
.pushsection .foo,"a",@nobits .pushsection .foo,"a",@nobits
.pushsection .foo,"a",@progbits .pushsection .foo,"a",@progbits
## GNU as emits SHT_PROGBITS .eh_frame for .cfi_* directives. Don't error.
.section .eh_frame,"a",@progbits
.section .eh_frame,"a",@unwind
.pushsection .eh_frame,"a",@progbits
# CHECK: {{.*}}.s:[[# @LINE+1]]:1: error: changed section type for .eh_frame, expected: 0x70000001
.section .eh_frame,"a",@nobits