[llvm-debuginfo-analyzer] Fix a couple of unhandled DWARF situations leading to a crash #137221
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-binary-utilities @llvm/pr-subscribers-debuginfo Author: Javier Lopez-Gomez (jalopezg-git) ChangesThis pull request fixes a couple of unhandled situations in DWARF input leading to a crash. Specifically,
FYI, @CarlosAlbertoEnciso. I believe this patchset is ready; feel free to start review. Full diff: https://github.com/llvm/llvm-project/pull/137221.diff 4 Files Affected:
diff --git a/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp b/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
index 8bbaf93db0caa..f187b1a57bd45 100644
--- a/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp
@@ -330,13 +330,16 @@ void LVScope::addMissingElements(LVScope *Reference) {
Symbol->setIsOptimized();
Symbol->setReference(Reference);
- // The symbol can be a constant, parameter or variable.
+ // The symbol can be a constant, parameter, variable or unspecified
+ // parameters (i.e. `...`).
if (Reference->getIsConstant())
Symbol->setIsConstant();
else if (Reference->getIsParameter())
Symbol->setIsParameter();
else if (Reference->getIsVariable())
Symbol->setIsVariable();
+ else if (Reference->getIsUnspecified())
+ Symbol->setIsUnspecified();
else
llvm_unreachable("Invalid symbol kind.");
}
diff --git a/llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp b/llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp
index ad14baa0c9269..b12208c53b8e3 100644
--- a/llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp
+++ b/llvm/lib/DebugInfo/LogicalView/Readers/LVBinaryReader.cpp
@@ -433,6 +433,13 @@ Error LVBinaryReader::createInstructions(LVScope *Scope,
ArrayRef<uint8_t> Bytes = arrayRefFromStringRef(*SectionContentsOrErr);
uint64_t Offset = Address - SectionAddress;
+ if (Offset > Bytes.size()) {
+ LLVM_DEBUG({
+ dbgs() << "offset (" << hexValue(Offset) << ") is beyond section size ("
+ << hexValue(Bytes.size()) << "); malformed input?\n";
+ });
+ return Error::success();
+ }
uint8_t const *Begin = Bytes.data() + Offset;
uint8_t const *End = Bytes.data() + Offset + Size;
diff --git a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
index c062c15481da9..7262da634d18d 100644
--- a/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
+++ b/llvm/unittests/DebugInfo/LogicalView/DWARFReaderTest.cpp
@@ -30,6 +30,7 @@ extern const char *TestMainArgv0;
namespace {
const char *DwarfClang = "test-dwarf-clang.o";
+const char *DwarfClangUnspecParams = "test-dwarf-clang-unspec-params.elf";
const char *DwarfGcc = "test-dwarf-gcc.o";
// Helper function to get the first compile unit.
@@ -37,7 +38,7 @@ LVScopeCompileUnit *getFirstCompileUnit(LVScopeRoot *Root) {
EXPECT_NE(Root, nullptr);
const LVScopes *CompileUnits = Root->getScopes();
EXPECT_NE(CompileUnits, nullptr);
- EXPECT_EQ(CompileUnits->size(), 1u);
+ EXPECT_GT(CompileUnits->size(), 0u);
LVScopes::const_iterator Iter = CompileUnits->begin();
EXPECT_NE(Iter, nullptr);
@@ -124,6 +125,32 @@ void checkElementProperties(LVReader *Reader) {
ASSERT_EQ(Lines->size(), 0x12u);
}
+// Check proper handling of DW_AT_unspecified_parameters in
+// LVScope::addMissingElements().
+void checkUnspecifiedParameters(LVReader *Reader) {
+ LVScopeRoot *Root = Reader->getScopesRoot();
+ LVScopeCompileUnit *CompileUnit = getFirstCompileUnit(Root);
+
+ EXPECT_EQ(Root->getFileFormatName(), "elf64-x86-64");
+ EXPECT_EQ(Root->getName(), DwarfClangUnspecParams);
+
+ const LVPublicNames &PublicNames = CompileUnit->getPublicNames();
+ ASSERT_EQ(PublicNames.size(), 1u);
+
+ LVPublicNames::const_iterator IterNames = PublicNames.cbegin();
+ LVScope *Function = (*IterNames).first;
+ EXPECT_EQ(Function->getName(), "foo_printf");
+ const LVElements *Elements = Function->getChildren();
+ ASSERT_NE(Elements, nullptr);
+ EXPECT_EQ(std::any_of(
+ Elements->begin(), Elements->end(),
+ [](const LVElement *elt) {
+ return elt->getIsSymbol() &&
+ static_cast<const LVSymbol *>(elt)->getIsUnspecified();
+ }),
+ true);
+}
+
// Check the logical elements selection.
void checkElementSelection(LVReader *Reader) {
LVScopeRoot *Root = Reader->getScopesRoot();
@@ -253,6 +280,7 @@ void elementProperties(SmallString<128> &InputsDir) {
ReaderOptions.setAttributePublics();
ReaderOptions.setAttributeRange();
ReaderOptions.setAttributeLocation();
+ ReaderOptions.setAttributeInserted();
ReaderOptions.setPrintAll();
ReaderOptions.resolveDependencies();
@@ -264,6 +292,9 @@ void elementProperties(SmallString<128> &InputsDir) {
std::unique_ptr<LVReader> Reader =
createReader(ReaderHandler, InputsDir, DwarfClang);
checkElementProperties(Reader.get());
+
+ Reader = createReader(ReaderHandler, InputsDir, DwarfClangUnspecParams);
+ checkUnspecifiedParameters(Reader.get());
}
// Logical elements selection.
diff --git a/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-unspec-params.elf b/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-unspec-params.elf
new file mode 100755
index 0000000000000..67c6e71fbf7b9
Binary files /dev/null and b/llvm/unittests/DebugInfo/LogicalView/Inputs/test-dwarf-clang-unspec-params.elf differ
|
jalopezg-git
force-pushed
the
jalopezg-logicalview-fixes
branch
from
8b5f4dc to
bd673db
Compare
…f unspecified parameters
jalopezg-git
force-pushed
the
jalopezg-logicalview-fixes
branch
from
bd673db to
4bc3d31
Compare
|
Using the |
| if (Reference->getIsConstant()) | ||
| Symbol->setIsConstant(); | ||
| else if (Reference->getIsParameter()) | ||
| Symbol->setIsParameter(); | ||
| else if (Reference->getIsVariable()) | ||
| Symbol->setIsVariable(); | ||
| else if (Reference->getIsUnspecified()) | ||
| Symbol->setIsUnspecified(); |
| << hexValue(Bytes.size()) << "); malformed input?\n"; | ||
| }); | ||
| return Error::success(); // Continue; returning error aborts reader. | ||
| } |
There was a problem hiding this comment.
I think we should handle this condition as an error and abort the reader. May be something like:
if (Offset > Bytes.size()) {
LLVM_DEBUG({.....});
return createStringError(errc::bad_address, .......);
}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request fixes a couple of unhandled situations in DWARF input leading to a crash. Specifically,
...translates toDW_TAG_unspecified_parameters), which is then followed by a definition,llvm_unreachable()is hit inLVScope::addMissingElements().This is only visible in Debug builds (see stack trace below), but still.
test-dwarf-clang-unspec-params.elftriggers this condition.LVBinaryReader::createInstructions()does not check whetherOffsetlies within theBytesArrayRef. A specially crafted DWARF input can lead to this condition.FYI, @CarlosAlbertoEnciso. I believe this patchset is ready; feel free to start review.