This is an archive of the discontinued LLVM Phabricator instance.

Differential D127171

[DWARF] Support 'G' in dwarf parser
ClosedPublic

Authored by fmayer on Jun 6 2022, 5:57 PM.

Details

Reviewers
MaskRay
Commits
rGa2695c3fe1b7: [DWARF] Support 'G' in dwarf parser
rG4c71c3386c5c: [DWARF] Support 'G' in dwarf parser

Diff Detail

Unit TestsFailed

TimeTest
60,100 msx64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
30 msx64 debian > LLVM.DebugInfo/AArch64::stack-tagging-cfi.s

Event Timeline

fmayer created this revision.Jun 6 2022, 5:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 5:57 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
fmayer requested review of this revision.Jun 6 2022, 5:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2022, 5:57 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
fmayer mentioned this in D127007: [MC] Add 'G' to augmentation string for MTE instrumented functions.Jun 6 2022, 5:58 PM
fmayer updated this revision to Diff 434660.Jun 6 2022, 5:59 PM

test

fmayer mentioned this in D127148: [ELF] Support 'G' in .eh_frame.Jun 6 2022, 6:06 PM
fmayer added a reviewer: MaskRay.
Herald added a subscriber: StephenFan. · View Herald TranscriptJun 6 2022, 6:07 PM
fmayer added a parent revision: D127007: [MC] Add 'G' to augmentation string for MTE instrumented functions.Jun 6 2022, 6:15 PM
Harbormaster completed remote builds in B168195: Diff 434660.Jun 6 2022, 6:43 PM
MaskRay accepted this revision.Jun 6 2022, 7:30 PM

LGTM.

llvm/test/DebugInfo/AArch64/stack-tagging-cfi.s
1

Use an generic ELF target triple to indicate that this does not need Linux features.

2

llvm-dwarfdump --eh-frame %t.o

19

Remove

This revision is now accepted and ready to land.Jun 6 2022, 7:30 PM
MaskRay added inline comments.Jun 6 2022, 7:30 PM
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
1144

Can reuse break attached to case 'B'

fmayer updated this revision to Diff 434925.Jun 7 2022, 1:18 PM

address comments

fmayer added inline comments.Jun 7 2022, 1:20 PM
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
1144

Is that actually worth saving one line for potential confusion down the line if anyone were ever to add anything to one of those?

fmayer marked 3 inline comments as done.Jun 7 2022, 1:23 PM
fmayer updated this revision to Diff 434928.Jun 7 2022, 1:24 PM

test

fmayer added a subscriber: eugenis.Jun 7 2022, 1:37 PM
MaskRay accepted this revision.Jun 7 2022, 1:45 PM
MaskRay added inline comments.
llvm/test/DebugInfo/AArch64/stack-tagging-cfi.s
19

Delete comments like // -- End function

fmayer updated this revision to Diff 434942.Jun 7 2022, 1:57 PM

address comment

fmayer marked an inline comment as done.Jun 7 2022, 1:58 PM
Harbormaster completed remote builds in B168398: Diff 434942.Jun 7 2022, 3:10 PM
This revision was landed with ongoing or failed builds.Jun 8 2022, 12:39 PM
Closed by commit rG4c71c3386c5c: [DWARF] Support 'G' in dwarf parser (authored by fmayer). · Explain Why
This revision was automatically updated to reflect the committed changes.
fmayer added a commit: rG4c71c3386c5c: [DWARF] Support 'G' in dwarf parser.
fmayer added a reverting change: rG0d54a874ee24: Revert "[DWARF] Support 'G' in dwarf parser".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 435326.Jun 8 2022, 2:21 PM

require aarch64-registered-target

This revision was landed with ongoing or failed builds.Jun 8 2022, 2:28 PM
Closed by commit rGa2695c3fe1b7: [DWARF] Support 'G' in dwarf parser (authored by fmayer). · Explain Why
This revision was automatically updated to reflect the committed changes.
fmayer added a commit: rGa2695c3fe1b7: [DWARF] Support 'G' in dwarf parser.
Harbormaster completed remote builds in B168679: Diff 435326.Jun 8 2022, 3:11 PM

Revision Contents

PathSize
llvm/
lib/
DebugInfo/
DWARF/
4 lines
test/
DebugInfo/
AArch64/
21 lines

Diff 434660

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;
MaskRayUnsubmitted
Not Done
ReplyInline Actions

Can reuse break attached to case 'B'

MaskRay: Can reuse `break` attached to `case 'B'`
fmayerAuthorUnsubmitted
Done
ReplyInline Actions

Is that actually worth saving one line for potential confusion down the line if anyone were ever to add anything to one of those?

fmayer: Is that actually worth saving one line for potential confusion down the line if anyone were…
} }
} }
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

llvm/test/DebugInfo/AArch64/stack-tagging-cfi.s

  • This file was added.
# RUN: llvm-mc -filetype=obj -triple aarch64-generic-linux -mattr=+mte %s -o %t.o
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: llvm-objdump -s --dwarf=frames %t.o | FileCheck %s

Use an generic ELF target triple to indicate that this does not need Linux features.

MaskRay: Use an generic ELF target triple to indicate that this does not need Linux features.
# RUN: llvm-objdump -s --dwarf=frames %t.o | FileCheck %s
MaskRayUnsubmitted
Done
ReplyInline Actions

llvm-dwarfdump --eh-frame %t.o

MaskRay: `llvm-dwarfdump --eh-frame %t.o`
# 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
.globl NoUnwind // -- Begin function NoUnwind
MaskRayUnsubmitted
Done
ReplyInline Actions

Remove

MaskRay: Remove
MaskRayUnsubmitted
Done
ReplyInline Actions

Delete comments like // -- End function

MaskRay: Delete comments like `// -- End function`
.p2align 2
.type NoUnwind,@function