[Lldb-commits] [PATCH] D63591: DWARFDebugLoc: Make parsing and error reporting more robust

Paul Robinson via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 20 05:56:02 PDT 2019

probinson added a comment.

Looks pretty good, and thanks especially for the error-case tests!
I'll give other folks a chance to chime in if they want to.

Comment at: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:101
+    if (!Data.isValidOffsetForDataOfSize(*Offset, 2 * Data.getAddressSize()))
+      return createError("location list overflows the debug_loc section");
This identical createError call occurs many times, maybe add a createLocListOverflowError() helper?

Comment at: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:115
-    if (!Data.isValidOffsetForDataOfSize(*Offset, 2)) {
-      WithColor::error() << "location list overflows the debug_loc section.\n";
-      return None;
-    }
+    if (!Data.isValidOffsetForDataOfSize(*Offset, 2))
+      return createError("location list overflows the debug_loc section");
You could do `SavedOffset = *Offset;` here, and then add a `SavedOffset == *Offset` check to the next one.  There's no harm to calling a `get*` function with an invalid offset.

Comment at: lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:218
-  return LL;
Maybe put an llvm_unreachable here.

Comment at: test/DebugInfo/X86/dwarfdump-debug-loc-error-cases.s:1
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux --defsym CASE1=0 -o %t.o
+# RUN: llvm-dwarfdump -debug-loc %t.o 2>&1 | FileCheck %s --check-prefix=CONSUME
I was not aware of `--defsym` that looks incredibly useful!

In a test that generates multiple .o files I prefer to give each one a unique name, e.g. `%t0.o` and `%t1.o` etc.  It can make it easier to debug a broken test.




More information about the lldb-commits mailing list