[libcxx] Don't use -fvisibility-global-new-delete-hidden when not defining them

[mcgrathr]

Summary

When builing the hermetic static library, the compiler switch
-fvisibility-global-new-delete-hidden is necessary to get the new and
delete operator definitions made correctly. However, when those
definitions are not included in the library, then this switch does harm.
With lld (though not all linkers) setting STV_HIDDEN on SHN_UNDEF
symbols makes it an error to leave them undefined or defined via dynamic
linking that should generate PLTs for -shared linking (lld makes this a
hard error even without -z defs). Though leaving the symbols undefined
would usually work in practice if the linker were to allow it (and the
user didn't pass -z defs), this actually indicates a real problem that
could bite some target configurations more subtly at runtime. For
example, x86-32 ELF -fpic code generation uses hidden visibility on
declarations in the caller's scope as a signal that the call will never
be resolved to a PLT entry and so doesn't have to meet the special ABI
requirements for PLT calls (setting %ebx). Since these functions might
actually be resolved to PLT entries at link time (we don't know what the
user is linking in when the hermetic library doesn't provide all the
symbols itself), it's not safe for the compiler to treat their
declarations at call sites as having hidden visibility.

[phosek]

LGTM

libcxx/trunk/src/CMakeLists.txt

PROPERTIES
       COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
   )
   cxx_set_common_defines(cxx_static)
 
   if (LIBCXX_HERMETIC_STATIC_LIBRARY)
     append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
+    # If the hermetic library doesn't define the operator new/delete functions
+    # then its code shouldn't declare them with hidden visibility.  They might
+    # actually be provided by a shared library at link time.
+    if (LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS)
       append_flags_if_supported(CXX_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
+    endif()
     target_compile_options(cxx_static PRIVATE ${CXX_STATIC_LIBRARY_FLAGS})
     target_compile_definitions(cxx_static PRIVATE _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
   endif()
 
   list(APPEND LIBCXX_BUILD_TARGETS "cxx_static")
   if (LIBCXX_INSTALL_STATIC_LIBRARY)
     list(APPEND LIBCXX_INSTALL_TARGETS "cxx_static")
   endif()

llvm/trunk/utils/gn/secondary/libcxx/src/BUILD.gn

defines += [
       # specified.
       "_CRT_SECURE_NO_WARNINGS",
 
       # Use the ISO conforming behaviour for conversion
       # in printf, scanf.
       "_CRT_STDIO_ISO_WIDE_SPECIFIERS",
     ]
   }
+  if (!libcxx_enable_new_delete_definitions) {
+    defines += [ "_LIBCPP_DISABLE_NEW_DELETE_DEFINITIONS" ]
+  }
   if (libcxx_enable_exceptions) {
     if (current_os == "win") {
       cflags_cc += [ "/EHsc" ]
     }
   } else {
     if (current_os == "win") {
       cflags_cc += [
         "/EHs-",
         "/EHa-",
       ]
     } else {
       cflags_cc += [ "-fno-exceptions" ]
     }
-    defines += [ "-D_LIBCPP_NO_EXCEPTIONS" ]
+    defines += [ "_LIBCPP_NO_EXCEPTIONS" ]
   }
   if (!libcxx_enable_rtti) {
     if (current_os == "win") {
       cflags_cc += [ "/GR-" ]
     } else {
       cflags_cc += [ "-fno-rtti" ]
     }
-    defines += [ "-D_LIBCPP_NO_RTTI" ]
+    defines += [ "_LIBCPP_NO_RTTI" ]
   }
 }
 
 cxx_sources = [
   "algorithm.cpp",
   "string.cpp",
   "ios.cpp",
   "condition_variable.cpp",
if (libcxx_enable_static) {
   static_library("cxx_static") {
     output_dir = runtimes_dir
     output_name = "c++"
     complete_static_lib = true
     configs -= [ "//llvm/utils/gn/build:thin_archive" ]
     sources = cxx_sources
     if (libcxx_hermetic_static_library) {
       cflags = [ "-fvisibility=hidden" ]
+      if (libcxx_enable_new_delete_definitions) {
         cflags_cc = [ "-fvisibility-global-new-delete-hidden" ]
+      }
       defines = [ "_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS" ]
     }
     deps = [
       "//compiler-rt/lib/builtins",
       "//libcxxabi/src:cxxabi_static",
       "//libunwind/src:unwind_static",
     ]
     configs += [ ":cxx_config" ]