Page MenuHomePhabricator

[ELF] Support defining __start/__stop symbols as hidden
Needs ReviewPublic

Authored by phosek on Dec 13 2018, 3:40 PM.

Details

Reviewers
ruiu
espindola
Summary

The normal use for these symbols is always local to the module, and we
want to be able to have multiple modules using the same section names.

Consider a static library that is meant to be statically linked into
shared objects as a private implementation detail. Say this library
provides macros/templates to its users that emit metadata into a named
section, and then library code that iterates over the section. Each
instance of this library inside a given shared object must iterate over
its own list, not the list of whichever user of the static library
happens to be first in dynamic linking symbol resolution order.

We cannot define these symbols as hidden by default because some users
rely on using dlsym to look them up, see
https://sourceware.org/bugzilla/show_bug.cgi?id=21964

However, on new platforms that don't have any legacy code that depends
on the current behavior, it makes sense to define them as hidden by
default. This change introduces the -z hidden-start-stop-symbols option
that provides that ability.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

phosek created this revision.Dec 13 2018, 3:40 PM

I'm open to other suggestions for the option name, other thing we considered is -start-stop-symbols-visibility=....

grimar added a subscriber: grimar.Dec 14 2018, 12:24 AM

-z hide-start-stop maybe? (to shorten the name a bit)

lld/ELF/Writer.cpp
1849

What is B stands for? I would expect to see V (Visibility).

ruiu added a comment.Dec 14 2018, 1:47 PM

Adding a new option is a big deal. Could you elaborate on why you want to make these symbols hidden ones?

However, it's still useful to define them as hidden in many contexts.

Can you raise one example in the description?

pcc added a subscriber: pcc.Dec 14 2018, 9:10 PM

I don't think you need to introduce an option for this. You can give the __start_ and __stop_ symbols hidden visibility in any object file, and the symbols in the final output file will be hidden because the linker will choose the most restrictive visibility that it sees for the visibility of the symbols in the final output file.

Adding a new option is a big deal. Could you elaborate on why you want to make these symbols hidden ones?

The real question is concrete examples of when you'd ever not want them hidden? The normal use for these symbols is always local to the module, and you want to be able to have multiple modules using the same section names.

Consider a static library that is meant to be statically linked into shared objects as a private implementation detail. Say this library provides macros/templates to its users that emit metadata into a named section, and then library code that iterates over the section. Each instance of this library inside a given shared object must iterate over its own list, not the list of whichever user of the static library happens to be first in dynamic linking symbol resolution order.

It seems like the only reason these symbols are exported is because when support for generating these symbols has been first implemented in ld.bfd, it didn't even have a concept of visibility. Later, binutils tried to make these symbols hidden, but that broke clients that depend on the old behavior (according to Hyrum's law) which is documented in https://sourceware.org/bugzilla/show_bug.cgi?id=21964

In D55682#1332033, @pcc wrote:

I don't think you need to introduce an option for this. You can give the __start_ and __stop_ symbols hidden visibility in any object file, and the symbols in the final output file will be hidden because the linker will choose the most restrictive visibility that it sees for the visibility of the symbols in the final output file.

Yes that works, but it has to be done explicitly for each such symbol. We cannot change these symbols to be hidden by default for existing platforms as described above, but we would like to make them hidden by default in Fuchsia since we don't have any legacy code (yet) that would depend on the existing behavior. Introducing new option allows us to set it in the driver like we already do e.g. for -z rodynamic which is a similar case.

phosek marked an inline comment as done.Dec 19 2018, 11:59 AM
phosek added inline comments.
lld/ELF/Writer.cpp
1849

Binding (that's what addOptionalRegular's argument is called as well), would you prefer V?

phosek edited the summary of this revision. (Show Details)Dec 19 2018, 12:03 PM

However, it's still useful to define them as hidden in many contexts.

Can you raise one example in the description?

I have updated the description to better explain the motivation and provide an example.

In D55682#1332033, @pcc wrote:

I don't think you need to introduce an option for this. You can give the __start_ and __stop_ symbols hidden visibility in any object file, and the symbols in the final output file will be hidden because the linker will choose the most restrictive visibility that it sees for the visibility of the symbols in the final output file.

Yes that works, but it has to be done explicitly for each such symbol. We cannot change these symbols to be hidden by default for existing platforms as described above, but we would like to make them hidden by default in Fuchsia since we don't have any legacy code (yet) that would depend on the existing behavior. Introducing new option allows us to set it in the driver like we already do e.g. for -z rodynamic which is a similar case.

Does that mean there is a visibility mismatch between source code and linker-defined symbol? e.g.

//extern char __start_f __attribute__((visibility("hidden")));
extern char __start_f; // Fuchsia wants to write code like this, but its visibility is set to hidden due to -z hidden-start-stop-symbols

__attribute__((section("f"))) char *foo() {
  return &__start_f;
}

Please correct me but if my understanding is correct, I think the explicit visibility attribute in source code would be clearer.

ruiu added a comment.Dec 19 2018, 1:40 PM

Thank you for the explanation. Your explanation makes sense to me. Being said that, I don't think I'd add a new option to the linker for Fuchsia, as I guess that the situation in which you want to iterate over start/end symbols is not too frequent. Also if you iterate over a known list of start/end symbols, you can define the symbols as hidden ones using attribute, so there's already an easy workaround that works on many ELF linkers.