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

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 8 inline comments as done.
sepavloff added inline comments.


================
Comment at: llvm/test/tools/llvm-symbolizer/output-style-json-code.test:28
 ## Invalid first argument before any valid one.
-# NO-INLINES:{"Error":{"Message":"unable to parse arguments: some text"},"ModuleName":"{{.*}}/Inputs/addr.exe"}
+# NO-INLINES:{"ModuleName":"{{.*}}/Inputs/addr.exe","Symbol":[{"Column":0,"Discriminator":0,"FileName":"","FunctionName":"","Line":0,"StartAddress":"","StartFileName":"","StartLine":0}]}
 ## Resolve valid address.
----------------
jhenderson wrote:
> These seem like a regression to me in the JSON output, as it's no longer recording the error?
It is not an error anymore, it is "no source information". If llvm-symbolizer is called with an address without such information, for example:
```
    llvm-symbolizer -e llvm-symbolizer --output-style=JSON 0x3
```
the output is similar:
```
[{"Address":"0x3","ModuleName":"llvm-symbolizer","Symbol":[{"Column":0,"Discriminator":0,"FileName":"","FunctionName":"","Line":0,"StartAddress":"","StartFileName":"","StartLine":0}]}]
```
It is consistent with the new behavior for other output styles, and is natural if symbol lookup is implemented.


================
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();
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157210