[PATCH] D157210: [symbolizer] Change reaction on invalid input

Wed Aug 9 23:55:08 PDT 2023

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

Thanks for addressing my concerns. LGTM.

================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:233
+  if (Offset.getAsInteger(IsAddr2Line ? 16 : 0, ModuleOffset))
+    return errorCodeToError(errc::invalid_argument);
+  return Error::success();
----------------
sepavloff wrote:
> jhenderson wrote:
> > Rather than just producing "invalid argument", could we give additional information about what the argument is that is deemed to be invalid, e.g. "1foo is not a valid address" or something like that?
> Actually it is not an error, `llvm-symbolize` processes it as an address which has no related source information. It is consistent with the present behavior of GNU `addr2line` (even of version before 2.39). An error is returned so that the message "??:0" could be printed immediately. To distinguish this case from the errors that require emitting a diagnostic message,  a different error type is used here. Actually particular error code is not used, only different class matters. The code in `symbolizeInput` can react differently in such case.
> 
> Implementation of symbol lookup will make this trick unnecessary, as any string could potentially be a symbol name.
Okay, on the basis that this is only temporary, this is fine. The comment is useful too.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157210