[PATCH] D151755: [DebugInfo] Add error checking around data extraction in DWARFAbbreviationDeclaration::extract
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 30 23:59:46 PDT 2023
jhenderson added a comment.
There are five new places that the function you've changed can bail out with an error, but you've only got one test. I think it makes sense to test the other new places too?
================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp:47
+ uint32_t Data) {
+ std::vector<uint32_t> BadData((Size / 4) + 1, Data);
+ OS.write(reinterpret_cast<char *>(BadData.data()), Size);
----------------
Might be worth adding an `assert` that `Size` is a multiple of 4?
================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp:125-126
+ DWARFAbbreviationDeclarationSet AbbrevSet;
+ const bool DataWasExtracted = AbbrevSet.extract(Data, &Offset);
+ EXPECT_FALSE(DataWasExtracted);
+}
----------------
Nit: it seems unnecessary to have this over two lines?
Is it also worth checking that `Offset` is some specific value?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151755/new/
https://reviews.llvm.org/D151755
More information about the llvm-commits
mailing list