From 93e5cf6b9c08d99cf7cefc0adda346bd7ba56049 Mon Sep 17 00:00:00 2001 From: Leonard Chan Date: Fri, 26 Aug 2022 18:20:34 +0000 Subject: [PATCH] [clang] Do not instrument relative vtables under hwasan Full context in https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=107017. Instrumenting hwasan with globals results in a linker error under the relative vtables abi: ``` ld.lld: error: libunwind.cpp:(.rodata..L_ZTVN9libunwind12UnwindCursorINS_17LocalAddressSpaceENS_15Registers_arm64EEE.hwasan+0x8): relocation R_AARCH64_PLT32 out of range: 6845471433603167792 is not in [-2147483648, 2147483647]; references libunwind::AbstractUnwindCursor::~AbstractUnwindCursor() >>> defined in libunwind/src/CMakeFiles/unwind_shared.dir/libunwind.cpp.obj ``` This is because the tag is included in the vtable address when calculating the offset between the vtable and virtual function. A temporary solution until we can resolve this is to just disable hwasan instrumentation on relative vtables specifically, which can be done in the frontend. Differential Revision: https://reviews.llvm.org/D132425 --- clang/lib/CodeGen/CGVTables.cpp | 25 ++++++++++++++-- clang/lib/CodeGen/CGVTables.h | 3 ++ clang/lib/CodeGen/ItaniumCXXABI.cpp | 7 +++-- .../RelativeVTablesABI/relative-vtables-hwasan.cpp | 34 ++++++++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index cdd40d2..c8b29ae 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -921,12 +921,33 @@ llvm::GlobalVariable *CodeGenVTables::GenerateConstructionVTable( CGM.EmitVTableTypeMetadata(RD, VTable, *VTLayout.get()); - if (UsingRelativeLayout && !VTable->isDSOLocal()) - GenerateRelativeVTableAlias(VTable, OutName); + if (UsingRelativeLayout) { + RemoveHwasanMetadata(VTable); + if (!VTable->isDSOLocal()) + GenerateRelativeVTableAlias(VTable, OutName); + } return VTable; } +// Ensure this vtable is not instrumented by hwasan. That is, a global alias is +// not generated for it. This is mainly used by the relative-vtables ABI where +// vtables instead contain 32-bit offsets between the vtable and function +// pointers. Hwasan is disabled for these vtables for now because the tag in a +// vtable pointer may fail the overflow check when resolving 32-bit PLT +// 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) { + if (CGM.getLangOpts().Sanitize.has(SanitizerKind::HWAddress)) { + llvm::GlobalValue::SanitizerMetadata Meta; + if (VTable->hasSanitizerMetadata()) + Meta = VTable->getSanitizerMetadata(); + Meta.NoHWAddress = true; + VTable->setSanitizerMetadata(Meta); + } +} + // If the VTable is not dso_local, then we will not be able to indicate that // the VTable does not need a relocation and move into rodata. A frequent // time this can occur is for classes that should be made public from a DSO diff --git a/clang/lib/CodeGen/CGVTables.h b/clang/lib/CodeGen/CGVTables.h index bdfc075..c044ecc 100644 --- a/clang/lib/CodeGen/CGVTables.h +++ b/clang/lib/CodeGen/CGVTables.h @@ -154,6 +154,9 @@ public: /// when a vtable may not be dso_local. void GenerateRelativeVTableAlias(llvm::GlobalVariable *VTable, llvm::StringRef AliasNameRef); + + /// Specify a vtable should not be instrumented with hwasan. + void RemoveHwasanMetadata(llvm::GlobalValue *VTable); }; } // end namespace CodeGen diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 2809cbe..cb97af7 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -1769,8 +1769,11 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT, } } - if (VTContext.isRelativeLayout() && !VTable->isDSOLocal()) - CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName()); + if (VTContext.isRelativeLayout()) { + CGVT.RemoveHwasanMetadata(VTable); + if (!VTable->isDSOLocal()) + CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName()); + } } bool ItaniumCXXABI::isVirtualOffsetNeededForVTableField( diff --git a/clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp b/clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp new file mode 100644 index 0000000..b414b8d --- /dev/null +++ b/clang/test/CodeGenCXX/RelativeVTablesABI/relative-vtables-hwasan.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -S -o - -emit-llvm -fsanitize=hwaddress | FileCheck %s + +/// The usual vtable will have default visibility. In this case, the actual +/// vtable is hidden and the alias is made public. With hwasan enabled, we want +/// to ensure the local one self-referenced in the hidden vtable is not +/// 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 + +class A { +public: + virtual void foo(); +}; + +void A::foo() {} + +void A_foo(A *a) { + a->foo(); +} + +/// 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 + +class __attribute__((visibility("hidden"))) B { +public: + virtual void foo(); +}; + +void B::foo() {} + +void B_foo(B *b) { + b->foo(); +} -- 2.7.4