[PATCH] D63394: [yaml2obj][ELF] Suport STT_LOOS, STT_HIOS, STT_LOPROC and STT_HIPROC.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 11:14:11 PDT 2019


grimar added a comment.

In D63394#1546401 <https://reviews.llvm.org/D63394#1546401>, @jhenderson wrote:

> In D63394#1546292 <https://reviews.llvm.org/D63394#1546292>, @ychen wrote:
>
> > I'm totally fine with not commit this. But it seems just a small convenience from a tooling perspective (no need for the semantics of the symbol type) when you need the value but not in an OS or processor specific way and STT_LOOS looks better than "10"?
>
>
> I may have been a bit hasty rejecting this. My concern is that I have already seen instances in LLVM where people possibly not well-versed in ELF try to use these as actual types in testing and in tools consuming these values, which can lead to confusion or worse, bogus output (e.g. llvm-readelf claiming that a symbol's type is STT_LOOS). @grimar, do you have any thoughts?


Yes, I think I agree. This is delimiters and not values intended to be used. We might want to improve error reporing (i.e. introduce a error to explain this in not just unknown values, but unsupported values).
Or at least add a comment explaining that we don't `enumCase` them intentionally to `ScalarEnumerationTraits<ELFYAML::ELF_STT>::enumeration` probably to avoid futher confusion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63394





More information about the llvm-commits mailing list