Skip to content

[llvm-debuginfo-analyzer] Common handling of unsigned attribute values. #116027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 28, 2024
Merged

[llvm-debuginfo-analyzer] Common handling of unsigned attribute values. #116027

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9fb77c9
[llvm-debuginfo-analyzer] Common handling of unsigned attribute values.
CarlosAlbertoEnciso Nov 13, 2024
cdf1783
[llvm-debuginfo-analyzer] Common handling of unsigned attribute values.
CarlosAlbertoEnciso Nov 25, 2024
6e8913e
[llvm-debuginfo-analyzer] Common handling of unsigned attribute values.
CarlosAlbertoEnciso Nov 25, 2024
1f771ae
[llvm-debuginfo-analyzer] Common handling of unsigned attribute values.
CarlosAlbertoEnciso Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions llvm/lib/DebugInfo/LogicalView/Readers/LVDWARFReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,15 +254,18 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,
// We are processing .debug_info section, implicit_const attribute
// values are not really stored here, but in .debug_abbrev section.
auto GetAsUnsignedConstant = [&]() -> int64_t {
return AttrSpec.isImplicitConst() ? AttrSpec.getImplicitConstValue()
: *FormValue.getAsUnsignedConstant();
if (AttrSpec.isImplicitConst())
return AttrSpec.getImplicitConstValue();
if (std::optional<uint64_t> Val = FormValue.getAsUnsignedConstant())
return *Val;
return 0;
};

auto GetFlag = [](const DWARFFormValue &FormValue) -> bool {
return FormValue.isFormClass(DWARFFormValue::FC_Flag);
};

auto GetBoundValue = [](const DWARFFormValue &FormValue) -> int64_t {
auto GetBoundValue = [&AttrSpec](const DWARFFormValue &FormValue) -> int64_t {
switch (FormValue.getForm()) {
case dwarf::DW_FORM_ref_addr:
case dwarf::DW_FORM_ref1:
Expand All @@ -283,6 +286,8 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,
return *FormValue.getAsUnsignedConstant();
case dwarf::DW_FORM_sdata:
return *FormValue.getAsSignedConstant();
case dwarf::DW_FORM_implicit_const:
return AttrSpec.getImplicitConstValue();
default:
return 0;
}
Expand All @@ -295,13 +300,13 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,

switch (AttrSpec.Attr) {
case dwarf::DW_AT_accessibility:
CurrentElement->setAccessibilityCode(*FormValue.getAsUnsignedConstant());
CurrentElement->setAccessibilityCode(GetAsUnsignedConstant());
break;
case dwarf::DW_AT_artificial:
CurrentElement->setIsArtificial();
break;
case dwarf::DW_AT_bit_size:
CurrentElement->setBitSize(*FormValue.getAsUnsignedConstant());
CurrentElement->setBitSize(GetAsUnsignedConstant());
break;
case dwarf::DW_AT_call_file:
CurrentElement->setCallFilenameIndex(IncrementFileIndex
Expand Down Expand Up @@ -333,13 +338,12 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,
Stream << hexString(Value, 2);
CurrentElement->setValue(Stream.str());
} else
CurrentElement->setValue(
hexString(*FormValue.getAsUnsignedConstant(), 2));
CurrentElement->setValue(hexString(GetAsUnsignedConstant(), 2));
} else
CurrentElement->setValue(dwarf::toStringRef(FormValue));
break;
case dwarf::DW_AT_count:
CurrentElement->setCount(*FormValue.getAsUnsignedConstant());
CurrentElement->setCount(GetAsUnsignedConstant());
break;
case dwarf::DW_AT_decl_line:
CurrentElement->setLineNumber(GetAsUnsignedConstant());
Expand All @@ -358,10 +362,10 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,
CurrentElement->setIsExternal();
break;
case dwarf::DW_AT_GNU_discriminator:
CurrentElement->setDiscriminator(*FormValue.getAsUnsignedConstant());
CurrentElement->setDiscriminator(GetAsUnsignedConstant());
break;
case dwarf::DW_AT_inline:
CurrentElement->setInlineCode(*FormValue.getAsUnsignedConstant());
CurrentElement->setInlineCode(GetAsUnsignedConstant());
break;
case dwarf::DW_AT_lower_bound:
CurrentElement->setLowerBound(GetBoundValue(FormValue));
Expand All @@ -381,7 +385,7 @@ void LVDWARFReader::processOneAttribute(const DWARFDie &Die,
CurrentElement->setUpperBound(GetBoundValue(FormValue));
break;
case dwarf::DW_AT_virtuality:
CurrentElement->setVirtualityCode(*FormValue.getAsUnsignedConstant());
CurrentElement->setVirtualityCode(GetAsUnsignedConstant());
break;

case dwarf::DW_AT_abstract_origin:
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/tools/llvm-debuginfo-analyzer/DWARF/05-dwarf-incorrect-lexical-scope-variable.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@
; ONE-EMPTY:
; ONE-NEXT: [001] {CompileUnit} 'pr-43860.cpp'
; ONE-NEXT: [002] {Producer} 'clang version 15.0.0 {{.*}}'
; ONE-NEXT: [002] 2 {Function} extern not_inlined 'InlineFunction' -> 'int'
; ONE-NEXT: [002] 2 {Function} extern inlined 'InlineFunction' -> 'int'
; ONE-NEXT: [003] {Block}
; ONE-NEXT: [004] 5 {Variable} 'Var_2' -> 'int'
; ONE-NEXT: [003] 2 {Parameter} 'Param' -> 'int'
; ONE-NEXT: [003] 3 {Variable} 'Var_1' -> 'int'
; ONE-NEXT: [002] 11 {Function} extern not_inlined 'test' -> 'int'
; ONE-NEXT: [003] 12 {Variable} 'A' -> 'int'
; ONE-NEXT: [003] 13 {InlinedFunction} not_inlined 'InlineFunction' -> 'int'
; ONE-NEXT: [003] 13 {InlinedFunction} inlined 'InlineFunction' -> 'int'
; ONE-NEXT: [004] {Block}
; ONE-NEXT: [005] {Variable} 'Var_2' -> 'int'
; ONE-NEXT: [004] {Parameter} 'Param' -> 'int'
Expand Down
5 changes: 5 additions & 0 deletions llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
set(LLVM_LINK_COMPONENTS
${LLVM_TARGETS_TO_BUILD}
Copy link
Contributor

@nico nico Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this make the next 3 lines superfluous?

Copy link
Member Author

@CarlosAlbertoEnciso CarlosAlbertoEnciso Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nico That is a good point. With your suggested change, all pre-commit test pass.
Opened #118052

AllTargetsDescs
AllTargetsDisassemblers
AllTargetsInfos
AsmPrinter
DebugInfoLogicalView
MC
MCDisassembler
TargetParser
)

add_llvm_unittest_with_input_files(DebugInfoLogicalViewTests
../DWARF/DwarfGenerator.cpp
../DWARF/DwarfUtils.cpp
Copy link
Collaborator

@pogo59 pogo59 Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay for now, but really these components should be moved into their own library so they can be shared between unittests.

Copy link
Member Author

@CarlosAlbertoEnciso CarlosAlbertoEnciso Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point. I created a PR to record that request. #117864

CodeViewReaderTest.cpp
CommandLineOptionsTest.cpp
CompareElementsTest.cpp
DWARFGeneratedTest.cpp
DWARFReaderTest.cpp
SelectElementsTest.cpp
LocationRangesTest.cpp
Expand Down
Loading
Loading