[PATCH] D47092: downgrade strong type info names to weak_odr linkage

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

rsmith added a comment.

In https://reviews.llvm.org/D47092#1105315, @efriedma wrote:

> I'm pretty sure this isn't actually a violation of LLVM's linkage rules as they are described in LangRef... at least, my understanding is that "equivalent" doesn't imply anything about linkage.  (If this should be a violation, we need to clarify the rules.)


This is a fair point. (Though I would note that the LangRef doesn't even say that you can't have two external definitions of the same symbol, so it would clearly benefit from some clarification in this area.)

We could certainly teach ASan to just ignore "odr violations" on type info names, if we think that the IR generated here is in fact correct.

If we go down that path, do we still need changes for the Darwin ARM64 case? (That is, is it OK to emit some `weak_odr` definitions along with an `external` definition for the same `_ZTS` symbol, and not use the 'non-unique type info name' flag? Or should we still be treating that case -- and, as a consequence, //all// external-linkage `_ZTS` symbols for classes -- as non-unique?)



================
Comment at: test/CodeGenCXX/arm64.cpp:48
   void A::foo() {}
-  // CHECK-GLOBALS-DAG: @_ZTSN5test21AE = constant [11 x i8]
+  // CHECK-GLOBALS-DAG: @_ZTSN5test21AE = weak_odr constant [11 x i8]
   // CHECK-GLOBALS-DAG: @_ZTIN5test21AE = constant { {{.*}}, i8* getelementptr inbounds ([11 x i8], [11 x i8]* @_ZTSN5test21AE, i32 0, i32 0) }
----------------
rjmccall wrote:
> The way the Darwin ARM64 ABI handles non-unique RTTI is by setting a flag indicating that clients should do string comparisons/hashes; it does not rely on the type name symbols being uniqued.  So this should stay external and the _ZTI definition should change to set the bit.
OK. I was trying to avoid introducing more cases where we do string comparisons, but if you prefer string comparisons over link-time deduplication in all cases for that target, that seems easy enough to support.

(Out of curiosity, does the link-time deduplication not reliably work on that platform? If so, how do you deal with the other cases that require deduplication? If not, what's the motivation for using string comparisons?)


================
Comment at: test/CodeGenCXX/windows-itanium-type-info.cpp:31
 // CHECK-DAG: @_ZTI7derived = dso_local dllexport constant
-// CHECK-DAG: @_ZTS7derived = dso_local dllexport constant
+// CHECK-DAG: @_ZTS7derived = weak_odr dso_local dllexport constant
 // CHECK-DAG: @_ZTV7derived = dso_local dllexport unnamed_addr constant
----------------
rjmccall wrote:
> Is this actually okay?  I didn't think dllexport supported weak symbols.
Good question, I don't know. Do we have some way to specify "weak_odr within this DSO but strong at the DSO boundary"? I suppose a COMDAT would give that effect.


Repository:
  rC Clang

https://reviews.llvm.org/D47092