[PATCH] D82017: [DebugInfo/DWARF] - Do not hang when CFI are truncated.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:138-139
+        // prohibited in call frame instructions, see sec. 6.4.2 in DWARFv5.
+        Instructions.back().Expression =
+            DWARFExpression(Extractor, Data.getAddressSize());
+        break;
----------------
What happens if the expression itself is truncated? I'd expect this to somehow report an error that can be detected here. Same goes below.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp:178-182
+  // Unable to read a truncated DW_CFA_advance_loc4 instruction.
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_advance_loc4}),
+      FailedWithMessage(
+          "unexpected end of data at offset 0x1 while reading [0x1, 0x5)"));
----------------
Accidental duplicate test case?


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp:185-188
+  EXPECT_THAT_ERROR(
+      ParseCFI(TestCIE, {dwarf::DW_CFA_restore_extended}),
+      FailedWithMessage("unable to decode LEB128 at offset 0x00000001: "
+                        "malformed uleb128, extends past end"));
----------------
Given that this and the next several cases are identical, barring the input value, (as indeed are many) have you considered changing to using a set of parameterised test cases? No idea how much extra complexity (if any) it would add, without actually trying it, so it might not be worth it, but in principle you could have a test body for each of the different possible error messages, and a list of parameters for each test body listing the opcodes that trigger that case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82017/new/

https://reviews.llvm.org/D82017