[PATCH] D59875: [yaml2obj][obj2yaml] - Teach yaml2obj/obj2yaml tools about STB_GNU_UNIQUE symbols.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added a comment.

In D59875#1445681 <https://reviews.llvm.org/D59875#1445681>, @jhenderson wrote:

> LGTM.
>
> The way symbols are handled in yaml2obj looks like it's desperately in need of an overhaul. It looks very fragile and easy to miss one thing when modifying it like you did here. That's a separate issue though.


I also feel I do not like the way how yaml2obj describes/handles symbols now. At least currently for each binding, we might need to extend the syntax and it does not seem to me as a reasonable/nice approach in general.
I think we should change/improve something here, let me take a look closer, I'll try to suggest something here after the additional investigation.



================
Comment at: tools/obj2yaml/elf2yaml.cpp:268
     default:
       llvm_unreachable("Unknown ELF symbol binding");
     }
----------------
jhenderson wrote:
> Sounds to me like llvm_unreachable here is wrong, because it is reachable by any value not recognised. It should just be a normal error saying what the unknown binding value is. That can be a separate patch though if you want.
Yeah, I noticed that but was not sure because I would not expect to see any another kind of bindings here (I just never heard about other ones I think). But now, after your comment, I think you're right, that perhaps better be an error just in case. I'll prepare a patch.


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

https://reviews.llvm.org/D59875