[PATCH] D72313: [llvm-symbolizer]Fix printing of malformed address values not passed via stdin

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Thanks for the patch! Could you add an additional test case for passing the values via a response file, rather than directly on the command-line, i.e. `llvm-symbolizer <some args> @response.txt` where response.txt contains "some text", inctwo and "some text2" etc.

Also, I think it would be good to expand `invalid-input-address.test` to cover this. In fact, you might want to put both your new case and something equivalent to the existing one in sym.test in the same place. That way, we're not lumping multiple different kinds of checks into the same test file.



================
Comment at: llvm/test/tools/llvm-symbolizer/sym.test:33
 
 RUN: llvm-addr2line -obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefix=A2L %s
 RUN: llvm-addr2line -a -obj=%p/Inputs/addr.exe < %p/Inputs/addr.inp | FileCheck -check-prefixes=A2L,A2L_A %s
----------------
You should probably add the same test case as you have above for llvm-addr2line (the GNU-compatible version of llvm-symbolizer).


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:332
     while (fgets(InputString, sizeof(InputString), stdin)) {
-      symbolizeInput(InputString, Symbolizer, Printer);
+      // strip newline characters
+      std::string StrippedInputString(InputString);
----------------
LLVM style for comments is a leading capital letter and trailing full stop:
`// Strip newline characters.`


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:334
+      std::string StrippedInputString(InputString);
+      StrippedInputString.erase(std::remove(StrippedInputString.begin(), StrippedInputString.end(), '\n'), StrippedInputString.end());
+      symbolizeInput(StrippedInputString, Symbolizer, Printer);
----------------
Don't forget that on some systems (i.e. Windows), new lines end in '\r\n'. You need to make sure both are stripped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72313