[PATCH] D53787: [Sema] Provide -fvisibility-global-new-delete-hidden option

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

In https://reviews.llvm.org/D53787#1283051, @rsmith wrote:

> In https://reviews.llvm.org/D53787#1282995, @mcgrathr wrote:
>
> > In https://reviews.llvm.org/D53787#1282975, @rsmith wrote:
> >
> > > Other symbols must have exactly one definition (modulo the permission for duplicate identical definitions for some cases), but these ones have a default definition that is designed to be overridable by a different definition appearing anywhere in the program.
> >
> >
> > I don't understand this claim.  These are symbols like any others at link time.  A single definition must be supplied as for any other function.
>
>
> The program has two choices: either it provides a definition, and that gets used everywhere, or it does not, and the default version (provided by the toolchain) gets used everywhere.


That's true of all global symbols normally provided by libraries.

> This is what the language model requires, and it's so important that the entire program agrees on what the default heap is that we intentionally make this work even when using `-fvisibility=hidden`.

So you're concerned with people who use `-fvisibility=hidden` on the TUs that define some functions, and then use dynamic linking with the expectation that those definitions will be seen across shared library boundaries.
I don't see how people specifying visibility different than what they actually want is different for these particular functions than for any others.  But I'm biased towards users who actually understand what they asked the compiler to do.

>>> Other symbols are generally provided in one library and consumed by users of that library, whereas these symbols are typically provided by the main binary and consumed by the libraries that it uses. And so on.
>> 
>> I don't understand this claim.  These symbols are normally defined in libc++.so and nowhere else.
> 
> That's not accurate. As noted above, programs can provide their own definitions, which are required to replace the version that would otherwise be implicitly provided.

So they are global symbols defined in libraries.  That's all what you've said actually means to me.

I think it's clear that there is nothing different about these symbols than any others from the perspective of how linking works and how that relates to the language specification.  The *only* actual difference is that the language specification says that you can override these standard library symbols and that you may not override other standard library symbols--a distinction that does not actually exist materially in linking, nor matter to a programmer following the language specification and not using the compiler in ways outside the language specification (such as controlling symbol visibility).

All of your explanations for how they are different have to do with saving people from themselves when they asked the compiler to do something non-default with all symbols but actually meant to treat these particular symbols differently.
AFAICT the rationale goes something like: linking is subtle and hard to understand; many people use -fvisibility=hidden and do not fully understand what it means; a likely error is failing to understand the need to define a function with explicit-default visibility when it should be visible across shared library boundaries; conforming C++ code may define its own `operator new`/`operator delete` functions to have those used by the standard C++ library code (which might be in a shared library) in preference to its own definitions, but may not define other standard functions and have the same expectation; ergo, people with a poor understanding of linking who have followed some advice they didn't fully understand to use `-fvisibility=hidden` and also might define their own `operator new`/`operator delete` functions are best served by the compiler doing what they meant rather than what they said.

I'm a person who understands linking thoroughly, so I'm not the target audience for the concerns you raise.  That's fine.  Since I do understand the issues, I have no trouble understanding when I need a special compiler option to get the link-time behavior that's correct for my needs.


Repository:
  rC Clang

https://reviews.llvm.org/D53787