[PATCH] D53787: [Sema] Use proper visibility for global new and delete declarations

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D53787#1282930, @mcgrathr wrote:

> In https://reviews.llvm.org/D53787#1279899, @rsmith wrote:
>
> > Replacing the global new and delete is supposed to be a whole-program operation (you only get one global allocator). Otherwise you couldn't allocate memory in one DSO and deallocate it in another. (And nor could inline functions etc.)
> >
> > If you really want to do this weird thing, and you understand what you're getting yourself into, I don't see a problem with having a dedicated flag for it, but don't break all existing users of -fvisibility=.
>
>
> I don't really understand how these functions are different from other functions.  The language standards don't have anything to say about ELF visibility.  What you say about "whole-program operation" is true of any global symbol.


These symbols really are special. Other symbols are introduced explicitly by a declaration, whereas these are declared implicitly by the compiler. 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. 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.

> It is especially bizarre to me that explicit attributes on the definition sites are silently ignored for these functions and no others.

Do you have a testcase? That's not the behavior I'm seeing. What I see is that we get a hard error for an explicit attribute on the definition site, because the prior compiler-generated declaration has default visibility. Eg:

  $ echo '__attribute__((visibility("hidden"))) void *operator new(__SIZE_TYPE__) { return (void*)42; }' | clang -x c++ -
  <stdin>:1:16: error: visibility does not match previous declaration
  __attribute__((visibility("hidden"))) void *operator new(__SIZE_TYPE__) { return (void*)42; }
                 ^
  note: previous attribute is here

(That terrible note at the end is trying to point at the implicit compiler-generated declaration.)


Repository:
  rC Clang

https://reviews.llvm.org/D53787