labath added inline comments.

================
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");
----------------
probinson wrote:
> 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.
The debug_loc function doesn't use the SavedOffset pattern, because it is 
always reading data in fixed-size chunks. I think it would be better to keep it 
that way, as this is slightly more readable.


================
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
----------------
probinson wrote:
> 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.
I'm writing a bunch of tests in assembly these days, so I've learned a lot of 
interesting tricks there. :)

I'll update the tests to use distinct file names.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63591



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to