[PATCH] D132691: [clang] Do not instrument the rtti_proxies under hwasan

[Leonard Chan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcdb30f7a2635: [clang] Do not instrument the rtti_proxies 
under hwasan (authored by leonardchan).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132691/new/

https://reviews.llvm.org/D132691

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp


Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -6,6 +6,7 @@
 /// hwasan-instrumented.
 // CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 
x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to 
i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr 
@_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 
ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr 
getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) 
to i64)) to i32)] }, no_sanitize_hwaddress, align 4
 // CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
+// CHECK-DAG: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, 
no_sanitize_hwaddress, comdat
 
 class A {
 public:
@@ -21,6 +22,7 @@
 /// If the vtable happens to be hidden, then the alias is not needed. In this
 /// case, the original vtable struct itself should be unsanitized.
 // CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] 
[i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 
ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, 
i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr 
dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr 
inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, 
no_sanitize_hwaddress, align 4
+// CHECK-DAG: @_ZTI1B.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1B, 
no_sanitize_hwaddress, comdat
 
 class __attribute__((visibility("hidden"))) B {
 public:
Index: clang/lib/CodeGen/CGVTables.h
===
--- clang/lib/CodeGen/CGVTables.h
+++ clang/lib/CodeGen/CGVTables.h
@@ -155,8 +155,8 @@
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
llvm::StringRef AliasNameRef);
 
-  /// Specify a vtable should not be instrumented with hwasan.
-  void RemoveHwasanMetadata(llvm::GlobalValue *VTable);
+  /// Specify a global should not be instrumented with hwasan.
+  void RemoveHwasanMetadata(llvm::GlobalValue *GV) const;
 };
 
 } // end namespace CodeGen
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -664,6 +664,12 @@
 proxy->setVisibility(llvm::GlobalValue::HiddenVisibility);
 proxy->setComdat(module.getOrInsertComdat(rttiProxyName));
   }
+  // Do not instrument the rtti proxies with hwasan to avoid a duplicate
+  // symbol error. Aliases generated by hwasan will retain the same namebut
+  // the addresses they are set to may have different tags from different
+  // compilation units. We don't run into this without hwasan because the
+  // proxies are in comdat groups, but those aren't propagated to the 
alias.
+  RemoveHwasanMetadata(proxy);
 }
 target = proxy;
   }
@@ -938,13 +944,13 @@
 // relocations. A future alternative for this would be finding which usages of
 // the vtable can continue to use the untagged hwasan value without any loss of
 // value in hwasan.
-void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *VTable) {
+void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *GV) const {
   if (CGM.getLangOpts().Sanitize.has(SanitizerKind::HWAddress)) {
 llvm::GlobalValue::SanitizerMetadata Meta;
-if (VTable->hasSanitizerMetadata())
-  Meta = VTable->getSanitizerMetadata();
+if (GV->hasSanitizerMetadata())
+  Meta = GV->getSanitizerMetadata();
 Meta.NoHWAddress = true;
-VTable->setSanitizerMetadata(Meta);
+GV->setSanitizerMetadata(Meta);
   }
 }
 


Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -6,6 +6,7 @@
 /// hwasan-instrumented.
 // CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 x 

[PATCH] D132691: [clang] Do not instrument the rtti_proxies under hwasan

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: mcgrathr, phosek.
leonardchan added a project: clang.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
leonardchan requested review of this revision.

We run into a duplicate symbol error when instrumenting the rtti_proxies 
generated as part of the relative vtables ABI with hwasan:

  ld.lld: error: duplicate symbol: typeinfo for icu_71::UObject (.rtti_proxy)
  >>> defined at brkiter.cpp
  >>>
arm64-hwasan-shared/obj/third_party/icu/source/common/libicuuc.brkiter.cpp.o:(typeinfo
 for icu_71::UObject (.rtti_proxy))
  >>> defined at locavailable.cpp
  >>>
arm64-hwasan-shared/obj/third_party/icu/source/common/libicuuc.locavailable.cpp.o:(.data.rel.ro..L_ZTIN6icu_717UObjectE.rtti_proxy.hwasan+0xE00)

The issue here is that the hwasan alias carries over the visibility and linkage 
of the original proxy, so we have duplicate external symbols that participate 
in linking. Similar to D132425  we can just 
disable hwasan for the proxies for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132691

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CGVTables.h
  clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp


Index: clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
===
--- clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
+++ clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp
@@ -6,6 +6,7 @@
 /// hwasan-instrumented.
 // CHECK-DAG: @_ZTV1A.local = private unnamed_addr constant { [3 x i32] } { [3 
x i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1A.rtti_proxy to 
i64), i64 ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr 
@_ZTV1A.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 
ptrtoint (ptr dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr 
getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1A.local, i32 0, i32 0, i32 2) 
to i64)) to i32)] }, no_sanitize_hwaddress, align 4
 // CHECK-DAG: @_ZTV1A = unnamed_addr alias { [3 x i32] }, ptr @_ZTV1A.local
+// CHECK-DAG: @_ZTI1A.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1A, 
no_sanitize_hwaddress, comdat
 
 class A {
 public:
@@ -21,6 +22,7 @@
 /// If the vtable happens to be hidden, then the alias is not needed. In this
 /// case, the original vtable struct itself should be unsanitized.
 // CHECK-DAG: @_ZTV1B = hidden unnamed_addr constant { [3 x i32] } { [3 x i32] 
[i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), i64 
ptrtoint (ptr getelementptr inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, 
i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr 
dso_local_equivalent @_ZN1B3fooEv to i64), i64 ptrtoint (ptr getelementptr 
inbounds ({ [3 x i32] }, ptr @_ZTV1B, i32 0, i32 0, i32 2) to i64)) to i32)] }, 
no_sanitize_hwaddress, align 4
+// CHECK-DAG: @_ZTI1B.rtti_proxy = hidden unnamed_addr constant ptr @_ZTI1B, 
no_sanitize_hwaddress, comdat
 
 class __attribute__((visibility("hidden"))) B {
 public:
Index: clang/lib/CodeGen/CGVTables.h
===
--- clang/lib/CodeGen/CGVTables.h
+++ clang/lib/CodeGen/CGVTables.h
@@ -155,8 +155,8 @@
   void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable,
llvm::StringRef AliasNameRef);
 
-  /// Specify a vtable should not be instrumented with hwasan.
-  void RemoveHwasanMetadata(llvm::GlobalValue *VTable);
+  /// Specify a global should not be instrumented with hwasan.
+  void RemoveHwasanMetadata(llvm::GlobalValue *GV) const;
 };
 
 } // end namespace CodeGen
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -664,6 +664,12 @@
 proxy->setVisibility(llvm::GlobalValue::HiddenVisibility);
 proxy->setComdat(module.getOrInsertComdat(rttiProxyName));
   }
+  // Do not instrument the rtti proxies with hwasan to avoid a duplicate
+  // symbol error. Aliases generated by hwasan will retain the same namebut
+  // the addresses they are set to may have different tags from different
+  // compilation units. We don't run into this without hwasan because the
+  // proxies are in comdat groups, but those aren't propagated to the 
alias.
+  RemoveHwasanMetadata(proxy);
 }
 target = proxy;
   }
@@ -938,13 +944,13 @@
 // relocations. A future alternative for this would be finding which usages of
 // the vtable can continue to use the untagged hwasan value without any loss of
 // value in hwasan.
-void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *VTable) {
+void CodeGenVTables::RemoveHwasanMetadata(llvm::GlobalValue *GV) const {
   if