[PATCH] D57105: [ELF] Return the section name when calling getSymbolName on a section symbol.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 31 02:10:48 PST 2019


jhenderson added a comment.

Okay, the change as-is LGTM, assuming that the following has been done:

> I think before putting this in as is, it's probably worth reviewing all the call sites and seeing if there is anywhere where returning the section name would be harmful. I suspect it's probably mostly fine, given the lack of test failures.

A few things to look out for:

1. Some code paths may already do the extra work needed to look up the section name, if the symbol doesn't have a name. These should be tidied up to not do this any more (it can be a later change).
2. Some code paths may always print the name returned, but this may be undesirable for certain situations (I can't think of any off the top of my head however). This wouldn't be picked up by tests necessarily, due to the way FileCheck tests tend to be written.
3. Some code may be using the name in some kind of map, if non-empty. This could cause issues e.g. with relocations in LLD, when attempting to map references to symbols via names. In practice, it shouldn't be an issue, but we should double check.


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

https://reviews.llvm.org/D57105





More information about the llvm-commits mailing list