[PATCH] D135984: Add a "--load-address <addr>" option to llvm-gsymutil.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I don't know anything about GSYM or llvm-gsymutil really, but this sounds a lot like llvm-symbolizer's `--adjust-vma` option, but rather than being an offset, it sets the address outright. I guess it doesn't really make sense to follow llvm-symbolizer's option name and behaviour instead of this?

As an aside, I note that llvm-gsymutil doesn't appear to have any CommandGuide documentation in llvm/docs/CommandGuide. Perhaps worth adding at some point...



================
Comment at: llvm/include/llvm/DebugInfo/GSYM/GsymReader.h:57
+  // Users can change the base address of a GSYM file manually when
+  // symbolicating which allows clients to do lookups using addresses from a
+  // process that loaded the object file at a different base address.
----------------
Is the term not "symbolizing" rather than "symbolicating" or is that something else?


================
Comment at: llvm/include/llvm/DebugInfo/GSYM/GsymReader.h:232
+  /// All addresses in a GSYM file are based off of the base address from the
+  /// GSYM header. When symbolicating addresses, the object file is often loaded
+  /// at a load address that doesn't match the base address in the header. If we
----------------
Ditto.


================
Comment at: llvm/test/tools/llvm-gsymutil/X86/elf-dwarf.yaml:10
 # RUN: echo -e "0x400391 %/t.gsym\n0x4004cd %/t.gsym" | llvm-gsymutil --addresses-from-stdin  2>&1 | FileCheck %s --check-prefix=ADDRI --dump-input=always
+# RUN: echo -e "0x800391 %/t.gsym\n0x8004cd %/t.gsym" | llvm-gsymutil --load-address=0x800000 --addresses-from-stdin  2>&1 | FileCheck %s --check-prefix=ADDRI_BASE --dump-input=always
 # RUN: llvm-gsymutil --address=0x400391 --address=0x4004cd --verbose %t.gsym 2>&1 | FileCheck %s --check-prefix=ADDRV --dump-input=always
----------------
Nit.

Also, these lines are getting very long. Perhaps it would be worth splitting them up e.g.
```
# RUN: echo -e ... | \
# RUN:   llvm-gsymutil ... |
# RUN:   FileCheck ...
```


================
Comment at: llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp:535
 
+    // Set the load address if it has been
+    if (LoadAddress > 0)
----------------
Has been what? Looks like the comment is incomplete to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135984