This is an archive of the discontinued LLVM Phabricator instance.

Differential D127148

[ELF] Support 'G' in .eh_frame
ClosedPublic

Authored by fmayer on Jun 6 2022, 1:40 PM.

Details

Reviewers
MaskRay
eugenis
Commits
rGf6b1bfb7d5cc: [ELF] Support 'G' in .eh_frame
rG40f34fe4a87d: [ELF] Support 'G' in .eh_frame

Diff Detail

Event Timeline

fmayer created this revision.Jun 6 2022, 1:40 PM
Herald added a reviewer: MaskRay. · View Herald TranscriptJun 6 2022, 1:40 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: hiraditya, arichardson, emaste. · View Herald Transcript
fmayer requested review of this revision.Jun 6 2022, 1:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 1:40 PM
Herald added subscribers: llvm-commits, StephenFan. · View Herald Transcript
fmayer added a parent revision: D127007: [MC] Add 'G' to augmentation string for MTE instrumented functions.Jun 6 2022, 1:41 PM
fmayer added a reviewer: eugenis.
Harbormaster completed remote builds in B168140: Diff 434594.Jun 6 2022, 2:04 PM
eugenis accepted this revision.Jun 6 2022, 5:31 PM

LGTM

This revision is now accepted and ready to land.Jun 6 2022, 5:31 PM
fmayer updated this revision to Diff 434654.Jun 6 2022, 5:42 PM

test more

MaskRay added a comment.EditedJun 6 2022, 5:49 PM

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp code can be tested with llvm-objdump/llvm-dwarfdump but should have a test in test/DebugInfo.

lld doesn't use llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp code. It's inappropriate to rely on lld/test/ELF to provide the test coverage.

You should move llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp into another patch with a test/DebugInfo patch.

fmayer updated this revision to Diff 434663.Jun 6 2022, 6:03 PM

remove DWARF parser

fmayer updated this revision to Diff 434664.Jun 6 2022, 6:04 PM

remove dwarf test

fmayer retitled this revision from [ELF] Support 'G' in lld and dwarf parser to [ELF] Support 'G' in lld.Jun 6 2022, 6:04 PM
In D127148#3562065, @MaskRay wrote:

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp code can be tested with llvm-objdump/llvm-dwarfdump but should have a test in test/DebugInfo.

lld doesn't use llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp code. It's inappropriate to rely on lld/test/ELF to provide the test coverage.

You should move llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp into another patch with a test/DebugInfo patch.

Done, moved DWARF change to D127171.

Harbormaster completed remote builds in B168199: Diff 434664.Jun 6 2022, 6:17 PM
MaskRay accepted this revision.Jun 6 2022, 7:31 PM
MaskRay added inline comments.
lld/test/ELF/stack-tagging-cfi.s
2
4

-s is not needed

21

Delete NoUnwind

fmayer marked 3 inline comments as done.Jun 7 2022, 1:18 PM
fmayer updated this revision to Diff 434927.Jun 7 2022, 1:21 PM

address comments

Harbormaster completed remote builds in B168387: Diff 434927.Jun 7 2022, 1:22 PM
fmayer updated this revision to Diff 434930.Jun 7 2022, 1:26 PM

rebase

fmayer updated this revision to Diff 434931.Jun 7 2022, 1:26 PM

address comment

Harbormaster completed remote builds in B168390: Diff 434931.Jun 7 2022, 1:41 PM
fmayer updated this revision to Diff 434943.Jun 7 2022, 1:58 PM

remove comments

MaskRay accepted this revision.Jun 7 2022, 2:29 PM
MaskRay retitled this revision from [ELF] Support 'G' in lld to [ELF] Support 'G' in .eh_frame.
Harbormaster completed remote builds in B168399: Diff 434943.Jun 7 2022, 2:31 PM
Closed by commit rG40f34fe4a87d: [ELF] Support 'G' in .eh_frame (authored by fmayer). · Explain WhyJun 8 2022, 1:40 PM
This revision was automatically updated to reflect the committed changes.
fmayer added a commit: rG40f34fe4a87d: [ELF] Support 'G' in .eh_frame.
fmayer added a reverting change: rG6fb4fe728590: Revert "[ELF] Support 'G' in .eh_frame".Jun 8 2022, 1:53 PM
fmayer reopened this revision.Jun 8 2022, 1:53 PM

Tests broke on windows.

This revision is now accepted and ready to land.Jun 8 2022, 1:53 PM
fmayer updated this revision to Diff 435332.Jun 8 2022, 2:25 PM

add requires aarch64

This revision was landed with ongoing or failed builds.Jun 8 2022, 2:29 PM
Closed by commit rGf6b1bfb7d5cc: [ELF] Support 'G' in .eh_frame (authored by fmayer). · Explain Why
This revision was automatically updated to reflect the committed changes.
fmayer added a commit: rGf6b1bfb7d5cc: [ELF] Support 'G' in .eh_frame.
Harbormaster completed remote builds in B168680: Diff 435332.Jun 8 2022, 2:51 PM

Revision Contents

PathSize
lld/
ELF/
4 lines
test/
ELF/
24 lines
llvm/
lib/
DebugInfo/
DWARF/
4 lines

Diff 434594

lld/ELF/EhFrame.cpp

Show First 20 Lines • Show All 165 Lines • ▼ Show 20 Lines for (char c : aug) {
if (c == 'R') if (c == 'R')
return readByte(); return readByte();
if (c == 'z') if (c == 'z')
skipLeb128(); skipLeb128();
else if (c == 'L') else if (c == 'L')
readByte(); readByte();
else if (c == 'P') else if (c == 'P')
skipAugP(); skipAugP();
else if (c != 'B' && c != 'S') else if (c != 'B' && c != 'S' && c != 'G')
failOn(aug.data(), "unknown .eh_frame augmentation string: " + aug); failOn(aug.data(), "unknown .eh_frame augmentation string: " + aug);
} }
return DW_EH_PE_absptr; return DW_EH_PE_absptr;
} }
bool EhReader::hasLSDA() { bool EhReader::hasLSDA() {
StringRef aug = getAugmentation(); StringRef aug = getAugmentation();
for (char c : aug) { for (char c : aug) {
if (c == 'L') if (c == 'L')
return true; return true;
if (c == 'z') if (c == 'z')
skipLeb128(); skipLeb128();
else if (c == 'P') else if (c == 'P')
skipAugP(); skipAugP();
else if (c == 'R') else if (c == 'R')
readByte(); readByte();
else if (c != 'B' && c != 'S') else if (c != 'B' && c != 'S' && c != 'G')
failOn(aug.data(), "unknown .eh_frame augmentation string: " + aug); failOn(aug.data(), "unknown .eh_frame augmentation string: " + aug);
} }
return false; return false;
} }

lld/test/ELF/stack-tagging-cfi.s

  • This file was added.
# REQUIRES: aarch64
MaskRayUnsubmitted
Done
ReplyInline Actions
- # RUN: llvm-mc -filetype=obj -triple aarch64-generic-linux -mattr=+mte %s -o %t.o
+ # RUN: llvm-mc -filetype=obj -triple aarch64 -mattr=+mte %s -o %t.o
# RUN: ld.lld --eh-frame-hdr %t.o -o %t
MaskRay:
# RUN: llvm-mc -filetype=obj -triple aarch64-generic-linux -mattr=+mte %s -o %t.o
# RUN: ld.lld --eh-frame-hdr %t.o -o %t
MaskRayUnsubmitted
Done
ReplyInline Actions

-s is not needed

MaskRay: `-s` is not needed
# RUN: llvm-objdump -s --dwarf=frames %t | FileCheck %s
# CHECK: Augmentation: "zRG"
.text
.globl WithUnwind // -- Begin function WithUnwind
.p2align 2
.type WithUnwind,@function
WithUnwind: // @WithUnwind
.cfi_startproc
// %bb.0: // %entry
.cfi_mte_tagged_frame
ret
.Lfunc_end0:
.size WithUnwind, .Lfunc_end0-WithUnwind
.cfi_endproc
// -- End function
MaskRayUnsubmitted
Done
ReplyInline Actions

Delete NoUnwind

MaskRay: Delete `NoUnwind`
.globl NoUnwind // -- Begin function NoUnwind
.p2align 2
.type NoUnwind,@function

llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp

Show First 20 Lines • Show All 1,132 Lines • ▼ Show 20 Lines if (Id == getCIEId(IsDWARF64, IsEH)) {
AugmentationLength = Data.getULEB128(&Offset); AugmentationLength = Data.getULEB128(&Offset);
StartAugmentationOffset = Offset; StartAugmentationOffset = Offset;
EndAugmentationOffset = Offset + *AugmentationLength; EndAugmentationOffset = Offset + *AugmentationLength;
break; break;
case 'B': case 'B':
// B-Key is used for signing functions associated with this // B-Key is used for signing functions associated with this
// augmentation string // augmentation string
break; break;
// This stack frame contains MTE tagged data, so needs to be
// untagged on unwind.
case 'G':
break;
} }
} }
if (AugmentationLength.hasValue()) { if (AugmentationLength.hasValue()) {
if (Offset != EndAugmentationOffset) if (Offset != EndAugmentationOffset)
return createStringError(errc::invalid_argument, return createStringError(errc::invalid_argument,
"parsing augmentation data at 0x%" PRIx64 "parsing augmentation data at 0x%" PRIx64
" failed", " failed",
▲ Show 20 Lines • Show All 104 Lines • Show Last 20 Lines