[PATCH] D102224: Add option to llvm-gsymutil to read addresses from stdin.

Greg Clayton via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 11 14:42:48 PDT 2021


clayborg added a comment.

So looks like we just need tests:

- check error when user specifies --addresses-from-stdin and also an address
- check error when user specifies --addresses-from-stdin and also an input file
- check for successful lookups on multiple address + file tuples

And if would be good to specify the input format for the --addresses-from-stdin option in the option description.



================
Comment at: llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp:505
+                     [](char c) { return c == '\r' || c == '\n'; });
+
+      StringRef AddrStr, GSYMPath;
----------------
clayborg wrote:
> What happens if you send the following input into this tool:
> 
> "0x1000 /tmp/a.gsym
> 0x2000 /tmp/b.gsym
> "
> 
> If I read the code above correctly, it will end up with a string:
> 
> "0x1000 /tmp/a.gsym0x2000 /tmp/b.gsym"
> 
> because you are erasing all newline characters and the newline between "0x1000 /tmp/a.gsym" and "0x2000 /tmp/b.gsym" will be removed.
Never mind, we are grabbing one line at a time, so this wouldn't happen... Ignore above comment.


================
Comment at: llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp:509
+          llvm::StringRef{StrippedInputString}.split(' ');
+
+      if (GSYMPath != CurrentGSYMPath) {
----------------
Ignore this, after I reread the description, it seems clear there are other tools using this same format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102224



More information about the llvm-commits mailing list