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

[Chris Jackson via Phabricator via llvm-commits]

chrisjackson added a comment.

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

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


--add-symbol has its own visibility flags, true, but it is not a requirement to specify those flags. The normal output is STV_DEFAULT, whether or not the "default" flag is specified. Thus, by default newly added symbols are exported when using ELF visibility rules. We are in the process of transitioning to the ELF visibility scheme away from a different private implementation. The old implementation required the use of __declspec(dllexport) to export. This meant that exporting was an opt-in process on the part of the exporter, whereas the normal ELF process is an opt-out process on the part of the exporter. Since exporting was always opt-in, GNU objcopy's behaviour (and up to this point of transition llvm-objcopy's too) resulted in the new symbols not being exported.

Note that the producers of an object may not always be the same as the consumers of that object, so the behaviour really needs to be on the producer's end. Otherwise an updated build of the object will require a change in the consumer's source code - or their linked output will contain exported symbols without them expecting it.

I've taken a brief look at the Kiel/μVision docs and I see that the behaviour they have is somewhat similar to what we require, namely that symbols are STV_HIDDEN by default, so it looks like we aren't the only people for whom this behaviour would be useful.


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

https://reviews.llvm.org/D65893