[llvm] 92f2d02 - DebugInfo: Sink string form validation down from verifier to form parsing
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 14 15:42:02 PST 2021
Author: David Blaikie
Date: 2021-12-14T15:41:53-08:00
New Revision: 92f2d02b4a315eb91cd59e90394c0805123a96fa
URL: https://github.com/llvm/llvm-project/commit/92f2d02b4a315eb91cd59e90394c0805123a96fa
DIFF: https://github.com/llvm/llvm-project/commit/92f2d02b4a315eb91cd59e90394c0805123a96fa.diff
LOG: DebugInfo: Sink string form validation down from verifier to form parsing
Avoid duplicating the string decoding - improve the error messages down
in form parsing (& produce an Expected<const char*> instead of
Optional<const char*> to communicate the extra error details)
Added:
Modified:
llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp
llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
llvm/test/DebugInfo/X86/debug-macro-empty-str-offset.s
llvm/test/tools/llvm-dwarfdump/X86/verify_invalid_strp.yaml
Removed:
################################################################################
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
index 858b6d432bbf..130cdb8800a9 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h
@@ -112,7 +112,7 @@ class DWARFFormValue {
Optional<UnitOffset> getAsRelativeReference() const;
Optional<uint64_t> getAsUnsignedConstant() const;
Optional<int64_t> getAsSignedConstant() const;
- Optional<const char *> getAsCString() const;
+ Expected<const char *> getAsCString() const;
Optional<uint64_t> getAsAddress() const;
Optional<object::SectionedAddress> getAsSectionedAddress() const;
Optional<uint64_t> getAsSectionOffset() const;
@@ -173,9 +173,14 @@ namespace dwarf {
/// \returns an optional value that contains a value if the form value
/// was valid and was a string.
inline Optional<const char *> toString(const Optional<DWARFFormValue> &V) {
- if (V)
- return V->getAsCString();
- return None;
+ if (!V)
+ return None;
+ Expected<const char*> E = V->getAsCString();
+ if (!E) {
+ consumeError(E.takeError());
+ return None;
+ }
+ return *E;
}
/// Take an optional DWARFFormValue and try to extract a string value from it.
@@ -185,10 +190,16 @@ inline Optional<const char *> toString(const Optional<DWARFFormValue> &V) {
/// was valid and was a string.
inline StringRef toStringRef(const Optional<DWARFFormValue> &V,
StringRef Default = {}) {
- if (V)
- if (auto S = V->getAsCString())
- return *S;
- return Default;
+ if (!V)
+ return Default;
+ auto S = V->getAsCString();
+ if (!S) {
+ consumeError(S.takeError());
+ return Default;
+ }
+ if (!*S)
+ return Default;
+ return *S;
}
/// Take an optional DWARFFormValue and extract a string value from it.
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
index 1292bfbc0591..b96a4c19758f 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
@@ -331,7 +331,7 @@ class DWARFUnit {
Optional<object::SectionedAddress>
getAddrOffsetSectionItem(uint32_t Index) const;
- Optional<uint64_t> getStringOffsetSectionItem(uint32_t Index) const;
+ Expected<uint64_t> getStringOffsetSectionItem(uint32_t Index) const;
DWARFDataExtractor getDebugInfoExtractor() const;
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp
index 80ffd81b3403..7a81d7ff064b 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp
@@ -194,13 +194,11 @@ Error DWARFDebugMacro::parseImpl(
if (MacroContributionOffset == MacroToUnits.end())
return createStringError(errc::invalid_argument,
"Macro contribution of the unit not found");
- Optional<uint64_t> StrOffset =
+ Expected<uint64_t> StrOffset =
MacroContributionOffset->second->getStringOffsetSectionItem(
Data.getULEB128(&Offset));
if (!StrOffset)
- return createStringError(
- errc::invalid_argument,
- "String offsets contribution of the unit not found");
+ return StrOffset.takeError();
E.MacroStr =
MacroContributionOffset->second->getStringExtractor().getCStr(
&*StrOffset);
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
index cea0f63bbf81..86991a3949dd 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp
@@ -613,50 +613,53 @@ void DWARFFormValue::dump(raw_ostream &OS, DIDumpOptions DumpOpts) const {
}
void DWARFFormValue::dumpString(raw_ostream &OS) const {
- Optional<const char *> DbgStr = getAsCString();
- if (DbgStr.hasValue()) {
+ if (auto DbgStr = dwarf::toString(*this)) {
auto COS = WithColor(OS, HighlightColor::String);
COS.get() << '"';
- COS.get().write_escaped(DbgStr.getValue());
+ COS.get().write_escaped(*DbgStr);
COS.get() << '"';
}
}
-Optional<const char *> DWARFFormValue::getAsCString() const {
+Expected<const char *> DWARFFormValue::getAsCString() const {
if (!isFormClass(FC_String))
- return None;
+ return make_error<StringError>("Invalid form for string attribute",
+ inconvertibleErrorCode());
if (Form == DW_FORM_string)
return Value.cstr;
// FIXME: Add support for DW_FORM_GNU_strp_alt
if (Form == DW_FORM_GNU_strp_alt || C == nullptr)
- return None;
+ return make_error<StringError>("Unsupported form for string attribute",
+ inconvertibleErrorCode());
uint64_t Offset = Value.uval;
- if (Form == DW_FORM_line_strp) {
- // .debug_line_str is tracked in the Context.
- if (const char *Str = C->getLineStringExtractor().getCStr(&Offset))
- return Str;
- return None;
- }
+ Optional<uint32_t> Index;
if (Form == DW_FORM_GNU_str_index || Form == DW_FORM_strx ||
Form == DW_FORM_strx1 || Form == DW_FORM_strx2 || Form == DW_FORM_strx3 ||
Form == DW_FORM_strx4) {
if (!U)
- return None;
- Optional<uint64_t> StrOffset = U->getStringOffsetSectionItem(Offset);
+ return make_error<StringError>("API limitation - string extraction not "
+ "available without a DWARFUnit",
+ inconvertibleErrorCode());
+ Expected<uint64_t> StrOffset = U->getStringOffsetSectionItem(Offset);
+ Index = Offset;
if (!StrOffset)
- return None;
+ return StrOffset.takeError();
Offset = *StrOffset;
}
// Prefer the Unit's string extractor, because for .dwo it will point to
// .debug_str.dwo, while the Context's extractor always uses .debug_str.
- if (U) {
- if (const char *Str = U->getStringExtractor().getCStr(&Offset))
- return Str;
- return None;
- }
- if (const char *Str = C->getStringExtractor().getCStr(&Offset))
+ DataExtractor StrData = Form == DW_FORM_line_strp
+ ? C->getLineStringExtractor()
+ : U ? U->getStringExtractor()
+ : C->getStringExtractor();
+ if (const char *Str = StrData.getCStr(&Offset))
return Str;
- return None;
+ std::string Msg = FormEncodingString(Form).str();
+ if (Index)
+ Msg += (" uses index " + Twine(*Index) + ", but the referenced string").str();
+ Msg += (" offset " + Twine(Offset) + " is beyond .debug_str bounds").str();
+ return make_error<StringError>(Msg,
+ inconvertibleErrorCode());
}
Optional<uint64_t> DWARFFormValue::getAsAddress() const {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
index 82c34f537036..9b7ffed2ca67 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
@@ -214,13 +214,17 @@ DWARFUnit::getAddrOffsetSectionItem(uint32_t Index) const {
return {{Address, Section}};
}
-Optional<uint64_t> DWARFUnit::getStringOffsetSectionItem(uint32_t Index) const {
+Expected<uint64_t> DWARFUnit::getStringOffsetSectionItem(uint32_t Index) const {
if (!StringOffsetsTableContribution)
- return None;
+ return make_error<StringError>(
+ "DW_FORM_strx used without a valid string offsets table",
+ inconvertibleErrorCode());
unsigned ItemSize = getDwarfStringOffsetsByteSize();
uint64_t Offset = getStringOffsetsBase() + Index * ItemSize;
if (StringOffsetSection.Data.size() < Offset + ItemSize)
- return None;
+ return make_error<StringError>("DW_FORM_strx uses index " + Twine(Index) +
+ ", which is too large",
+ inconvertibleErrorCode());
DWARFDataExtractor DA(Context.getDWARFObj(), StringOffsetSection,
isLittleEndian, 0);
return DA.getRelocatedValue(ItemSize, &Offset);
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 7673a721c4ea..1e49798ab426 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -616,7 +616,6 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
DWARFAttribute &AttrValue,
ReferenceMap &LocalReferences,
ReferenceMap &CrossUnitReferences) {
- const DWARFObject &DObj = DCtx.getDWARFObj();
auto DieCU = Die.getDwarfUnit();
unsigned NumErrors = 0;
const auto Form = AttrValue.Value.getForm();
@@ -667,51 +666,15 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die,
}
break;
}
- case DW_FORM_strp: {
- auto SecOffset = AttrValue.Value.getAsSectionOffset();
- assert(SecOffset); // DW_FORM_strp is a section offset.
- if (SecOffset && *SecOffset >= DObj.getStrSection().size()) {
- ++NumErrors;
- error() << "DW_FORM_strp offset beyond .debug_str bounds:\n";
- dump(Die) << '\n';
- }
- break;
- }
+ case DW_FORM_strp:
case DW_FORM_strx:
case DW_FORM_strx1:
case DW_FORM_strx2:
case DW_FORM_strx3:
case DW_FORM_strx4: {
- auto Index = AttrValue.Value.getRawUValue();
- auto DieCU = Die.getDwarfUnit();
- // Check that we have a valid DWARF v5 string offsets table.
- if (!DieCU->getStringOffsetsTableContribution()) {
- ++NumErrors;
- error() << FormEncodingString(Form)
- << " used without a valid string offsets table:\n";
- dump(Die) << '\n';
- break;
- }
- // Check that the index is within the bounds of the section.
- unsigned ItemSize = DieCU->getDwarfStringOffsetsByteSize();
- // Use a 64-bit type to calculate the offset to guard against overflow.
- uint64_t Offset =
- (uint64_t)DieCU->getStringOffsetsBase() + Index * ItemSize;
- if (DObj.getStrOffsetsSection().Data.size() < Offset + ItemSize) {
- ++NumErrors;
- error() << FormEncodingString(Form) << " uses index "
- << format("%" PRIu64, Index) << ", which is too large:\n";
- dump(Die) << '\n';
- break;
- }
- // Check that the string offset is valid.
- uint64_t StringOffset = *DieCU->getStringOffsetSectionItem(Index);
- if (StringOffset >= DObj.getStrSection().size()) {
+ if (Error E = AttrValue.Value.getAsCString().takeError()) {
++NumErrors;
- error() << FormEncodingString(Form) << " uses index "
- << format("%" PRIu64, Index)
- << ", but the referenced string"
- " offset is beyond .debug_str bounds:\n";
+ error() << toString(std::move(E)) << ":\n";
dump(Die) << '\n';
}
break;
diff --git a/llvm/test/DebugInfo/X86/debug-macro-empty-str-offset.s b/llvm/test/DebugInfo/X86/debug-macro-empty-str-offset.s
index 33239526c2ca..a8897a389ccb 100644
--- a/llvm/test/DebugInfo/X86/debug-macro-empty-str-offset.s
+++ b/llvm/test/DebugInfo/X86/debug-macro-empty-str-offset.s
@@ -5,7 +5,7 @@
# RUN: llvm-mc -triple x86_64-unknown-linux -filetype=obj %s -o -| \
# RUN: not llvm-dwarfdump -debug-macro - /dev/null 2>&1 | FileCheck %s
-# CHECK: error: String offsets contribution of the unit not found
+# CHECK: error: DW_FORM_strx used without a valid string offsets table
.section .debug_abbrev,"", at progbits
.byte 1 # Abbreviation Code
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_invalid_strp.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_invalid_strp.yaml
index e630999cd594..81b420f2346d 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_invalid_strp.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_invalid_strp.yaml
@@ -2,7 +2,7 @@
# RUN: not llvm-dwarfdump -debug-info -verify %t.o | FileCheck %s
# CHECK: Verifying non-dwo Units...
-# CHECK-NEXT: error: DW_FORM_strp offset beyond .debug_str bounds:
+# CHECK-NEXT: error: DW_FORM_strp offset 4660 is beyond .debug_str bounds:
--- !ELF
FileHeader:
More information about the llvm-commits
mailing list