Page MenuHomePhabricator

[yaml2obj][ELF] Suport STT_LOOS, STT_HIOS, STT_LOPROC and STT_HIPROC.
Needs RevisionPublic

Authored by ychen on Jun 16 2019, 6:17 PM.

Event Timeline

ychen created this revision.Jun 16 2019, 6:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2019, 6:17 PM
MaskRay accepted this revision.Jun 16 2019, 6:58 PM
This revision is now accepted and ready to land.Jun 16 2019, 6:58 PM
jhenderson requested changes to this revision.Jun 17 2019, 4:32 AM

Sorry, I don't think this should be done, as these values are just range delimiters, not actually values that a symbol's type can be. If a symbol in that range is required, it should either be specified by the actual value required (e.g. just "11") or with a corresponding specific enumeration value, if available (e.g. STT_MYOS_TYPE).

This revision now requires changes to proceed.Jun 17 2019, 4:32 AM
ychen added a comment.Jun 17 2019, 8:46 AM

Sorry, I don't think this should be done, as these values are just range delimiters, not actually values that a symbol's type can be. If a symbol in that range is required, it should either be specified by the actual value required (e.g. just "11") or with a corresponding specific enumeration value, if available (e.g. STT_MYOS_TYPE).

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'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?

ychen added a comment.Jun 17 2019, 9:37 AM

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?

Oh, that makes perfect sense to me.

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?

I agree it doesn't make sense to print as a symbol value, although I wouldn't mind if obj2yaml could somehow print it as a comment, e.g.:

Type: 10  # STT_LOOS
Type: 11  # STT_LOOS+1

Similar to the behavior seen in tools/llvm-readobj/elf-section-types.test of printing LOOS+0xF00

grimar added a comment.EditedJun 17 2019, 11:12 AM

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