[PATCH] D75485: Support DW_FORM_strx* in llvm-dwp.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/tools/llvm-dwp/X86/invalid_cu_header_unittype.s:5
+
+# CHECK: error: Cannot parse compile unit header: unexpected end of data at offset 0x6 while reading [0x6, 0x7)
+  .section	.debug_info.dwo,"e", at progbits
----------------
The other messages are specific about what they can't parse - the unit version or the unit length. Hmm, I see because this uses the generic "check the error after parsing a bunch of fields".

Maybe it'd be worth checking that the length is potentially correct? That the length is long enough to include the header, and that the section is long enough to fit the length that's been parsed?


================
Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:125
+    return make_error<DWPError>("Cannot parse compile unit version: " +
+                                llvm::toString(std::move(Err)) + "\n");
+  if (Header.Version >= 5) {
----------------
I think the norm is probably not to include a newline in the error string - but wherever it's printed can choose where the newline should go.
Also, I think the norm is to not capitalize error messages (think about compiler error messages "error: unexpected thing") - so probably best to change "Cannot" to "cannot" in all these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75485