[PATCH] D65893: [llvm-objcopy] Allow the visibility of the start, end and size symbols created by --binary to be specified with --binary-symbol-visibility

Wed Aug 14 19:05:02 PDT 2019

MaskRay added a comment.

In D65893#1629220 <https://reviews.llvm.org/D65893#1629220>, @chrisjackson wrote:

> I agree that the generalized version is useful and I have been looking at a patch for this. However, in this specific case the symbol names are automatically generated based on the path of the binary, so it makes it difficult to specify these specific symbol names on the command line.

The symbol names are generated but that is unavoidable. You also have to specify the name in the source: `extern const char _binary_a_start[];`

> With regards to the expected semantics, we anticipate standard ELF semantics when combining different visibilities. Our toolchain operates using a default visibility of STV_HIDDEN for definitions unless explicitly exported, in which case they are STV_PROTECTED. This means that declarations need not be explicitly marked with a visibility in order to be exported or not. The new switch allows the replication of the compiler's -fvisibility=hidden switch in llvm-objcopy. As an alternative to the name --binary-symbol-visibility, how about --symbol-visibility?  This would affect the visibility of all the new symbols generated by llvm-objcopy, not just those symbols added for binary input.

There are no other options that add new symbols. `--add-symbol` has its own visibility flags, `--symbol-visibility` will not help that feature. `--new-symbol-visibility` might be a better name but people need to be taught it will be overridden by `--add-symbol` visibility flags. I am still not sure the little amount of saved labor (`__attribute__((visibility("protected")))` marks) justifies a new option, given that Keil embedded development tools already require other markers like `__declspec(dllexport)` and `__declspec(dllimport)`. And another question: how did you survive with GNU objcopy? It doesn't allow you to change the visibility of `_binary_xxx_start`...

> In D65893#1620711 <https://reviews.llvm.org/D65893#1620711>, @MaskRay wrote:
>  Quote gABI:

>> Second, if any reference to or definition of a name is a symbol with a non-default visibility attribute, the visibility attribute must be propagated to the resolving symbol in the linked object. If different visibility attributes are specified for distinct references to or definitions of a symbol, the most constraining visibility attribute must be propagated to the resolving symbol in the linked object. The attributes, ordered from least to most constraining, are: STV_PROTECTED, STV_HIDDEN and STV_INTERNAL.

> You can achieve the same with: __attribute__((visibility("hidden"))) extern char _binary_a_start[]; So I'm not sure why another option is needed (I think my another concern is that the option name --binary-symbol-visibility can make people puzzled if they add/delete/change symbols). See D55682 <https://reviews.llvm.org/D55682>.

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

https://reviews.llvm.org/D65893