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.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63591/new/
https://reviews.llvm.org/D63591
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
