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

Thu Jan 31 09:41:44 PST 2019

mattd added a comment.

In D57105#1378441 <https://reviews.llvm.org/D57105#1378441>, @jhenderson wrote:

> 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.

Thanks for the second pair of eyes on this patch.  I did take a look at the codebase from the monorepo to identify uses of getSymbolName.  I did not see any that would be disturbed by this change; however, I'll take another peek today.

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

https://reviews.llvm.org/D57105