[asan] Keep Itanium mangled names in global metadata

MaskRay · MaskRay · commit 00be3578e084 · 2022-11-19T01:06:26.000Z
The runtime calls `MaybeDemangleGlobalName` for error reporting and `__cxxabiv1::__cxa_demangle` is called if available, so demanging Itanium mangled names in global metadata is unnecessary and wastes data size. Add `MaybeDemangleGlobalName` in ODR violation detection to support demangled names in a suppressions file. `MaybeDemangleGlobalName` may call `DemangleCXXABI` and leak memory. Use an internal allocation to prevent lsan leak (in case there is no fatal asan error). The debug feature `report_globals=2` prints information for all instrumented global variables. `MaybeDemangleGlobalName` would be slow, so don't do that. The output looks like `Added Global[0x56448f092d60]: beg=0x56448fa66d60 size=4/32 name=_ZL13test_global_2` and I think the mangled name is fine. Other mangled schemes e.g. Windows (see win-string-literal.ll) remain the current behavior. Reviewed By: hctim Differential Revision: https://reviews.llvm.org/D138095
diff --git a/compiler-rt/lib/asan/asan_globals.cpp b/compiler-rt/lib/asan/asan_globals.cpp
@@ -148,9 +148,9 @@ static void CheckODRViolationViaIndicator(const Global *g) {
   for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
     if (g->odr_indicator == l->g->odr_indicator &&
         (flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
-        !IsODRViolationSuppressed(g->name))
-      ReportODRViolation(g, FindRegistrationSite(g),
-                         l->g, FindRegistrationSite(l->g));
+        !IsODRViolationSuppressed(MaybeDemangleGlobalName(g->name)))
+      ReportODRViolation(g, FindRegistrationSite(g), l->g,
+                         FindRegistrationSite(l->g));
   }
 }
 
@@ -164,7 +164,7 @@ static void CheckODRViolationViaPoisoning(const Global *g) {
     for (ListOfGlobals *l = list_of_all_globals; l; l = l->next) {
       if (g->beg == l->g->beg &&
           (flags()->detect_odr_violation >= 2 || g->size != l->g->size) &&
-          !IsODRViolationSuppressed(g->name))
+          !IsODRViolationSuppressed(MaybeDemangleGlobalName(g->name)))
         ReportODRViolation(g, FindRegistrationSite(g),
                            l->g, FindRegistrationSite(l->g));
     }
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
@@ -49,12 +49,17 @@ const char *DemangleCXXABI(const char *name) {
   // FIXME: __cxa_demangle aggressively insists on allocating memory.
   // There's not much we can do about that, short of providing our
   // own demangler (libc++abi's implementation could be adapted so that
-  // it does not allocate). For now, we just call it anyway, and we leak
-  // the returned value.
-  if (&__cxxabiv1::__cxa_demangle)
-    if (const char *demangled_name =
-          __cxxabiv1::__cxa_demangle(name, 0, 0, 0))
-      return demangled_name;
+  // it does not allocate). For now, we just call it anyway, and use
+  // InternalAlloc to prevent lsan error.
+  if (&__cxxabiv1::__cxa_demangle) {
+    if (char *demangled_name = __cxxabiv1::__cxa_demangle(name, 0, 0, 0)) {
+      size_t size = internal_strlen(demangled_name) + 1;
+      char *buf = (char *)InternalAlloc(size);
+      internal_memcpy(buf, demangled_name, size);
+      free(demangled_name);
+      return buf;
+    }
+  }
 
   return name;
 }
diff --git a/compiler-rt/test/asan/TestCases/Linux/odr_indicators.cpp b/compiler-rt/test/asan/TestCases/Linux/odr_indicators.cpp
@@ -11,11 +11,11 @@ int test_global_1;
 // INDICATOR1-DAG: Added Global{{.*}} name=test_global_1{{.*}} odr_indicator={{0x0*[^0]+.*$}}
 
 static int test_global_2;
-// CHECK-DAG: Added Global{{.*}} name=test_global_2{{.*}} odr_indicator={{0xf+$}}
+// CHECK-DAG: Added Global{{.*}} name=_ZL13test_global_2 {{.*}} odr_indicator={{0xf+$}}
 
 namespace {
 static int test_global_3;
-// CHECK-DAG: Added Global{{.*}} name={{.*}}::test_global_3{{.*}} odr_indicator={{0xf+$}}
+// CHECK-DAG: Added Global{{.*}} name=_ZN12_GLOBAL__N_113test_global_3E {{.*}} odr_indicator={{0xf+$}}
 } // namespace
 
 int main() {
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2264,9 +2264,11 @@ bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M,
     if (G->hasSanitizerMetadata())
       MD = G->getSanitizerMetadata();
 
-    // TODO: Symbol names in the descriptor can be demangled by the runtime
-    // library. This could save ~0.4% of VM size for a private large binary.
-    std::string NameForGlobal = llvm::demangle(G->getName().str());
+    // ASan runtime demangles Itanium mangled names, so keep the original name
+    // to prevent unneeded size increase of the string table.
+    std::string NameForGlobal = G->getName().str();
+    if (!StringRef(NameForGlobal).startswith("_Z"))
+      NameForGlobal = llvm::demangle(NameForGlobal);
     GlobalVariable *Name =
         createPrivateGlobalForString(M, NameForGlobal,
                                      /*AllowMerging*/ true, kAsanGenPrefix);
diff --git a/llvm/test/Instrumentation/AddressSanitizer/local_alias.ll b/llvm/test/Instrumentation/AddressSanitizer/local_alias.ll
@@ -13,17 +13,20 @@ target triple = "x86_64-unknown-linux-gnu"
 @a = dso_local global [2 x i32] zeroinitializer, align 4
 @b = private global [2 x i32] zeroinitializer, align 4
 @c = internal global [2 x i32] zeroinitializer, align 4
-@d = unnamed_addr global [2 x i32] zeroinitializer, align 4
+@_ZL1d = unnamed_addr global [2 x i32] zeroinitializer, align 4
 
 ; Check that we generate internal alias and odr indicator symbols for global to be protected.
 ; CHECK-NOINDICATOR-NOT: __odr_asan_gen_a
 ; CHECK-NOALIAS-NOT: private alias
+; CHECK-INDICATOR: @___asan_gen_.1 = private unnamed_addr constant [2 x i8] c"a\00", align 1
 ; CHECK-INDICATOR: @__odr_asan_gen_a = global i8 0, align 1
+; CHECK-INDICATOR: @___asan_gen_.4 = private unnamed_addr constant [6 x i8] c"_ZL1d\00", align 1
+; CHECK-INDICATOR: @__odr_asan_gen__ZL1d = global i8 0, align 1
 ; CHECK-ALIAS: @0 = private alias { [2 x i32], [24 x i8] }, ptr @a
 
 ; CHECK-ALIAS: @1 = private alias { [2 x i32], [24 x i8] }, ptr @b
 ; CHECK-ALIAS: @2 = private alias { [2 x i32], [24 x i8] }, ptr @c
-; CHECK-ALIAS: @3 = private alias { [2 x i32], [24 x i8] }, ptr @d
+; CHECK-ALIAS: @3 = private alias { [2 x i32], [24 x i8] }, ptr @_ZL1d
 
 ; Function Attrs: nounwind sanitize_address uwtable
 define i32 @foo(i32 %M) #0 {
