[LowerTypeTests][clang] Implement and allow -fsanitize=cfi-icall for RISCV

[twd2]

Summary

This patch implements jump tables for RISC-V so that CFI-icall can be enabled for it.

[twd2]

Ping?

The build failure is due to the name of the newly introduced kRISCVJumpTableEntrySize. Though I think we can put these magic numbers (4 and 8) just into LowerTypeTestsModule::getJumpTableEntrySize..

[asb]

Is it possible to write a test case for this?

[luismarques]

In D106888#2954425, @asb wrote:

Is it possible to write a test case for this?

Good question. I had checked that the AArch64 implementation had included a test, but I think that was only for new target-specific stuff, which doesn't apply here. So I was assuming the answer was no, but it would be good to get confirmation. I was also confused by the comment about "the build failure", since this applied cleanly and built OK for me when I looked at it.

[twd2]

In D106888#2954571, @luismarques wrote:

In D106888#2954425, @asb wrote:

Is it possible to write a test case for this?

Good question. I had checked that the AArch64 implementation had included a test, but I think that was only for new target-specific stuff, which doesn't apply here. So I was assuming the answer was no, but it would be good to get confirmation. I was also confused by the comment about "the build failure", since this applied cleanly and built OK for me when I looked at it.

Hi, you can check here: https://buildkite.com/llvm-project/premerge-checks/builds/49755#814fd222-2e5a-4400-824d-d1a1f1293c01
The clang-tidy failed due to the invalid case style for variable 'kRISCVJumpTableEntrySize'.

[luismarques]

In D106888#2954788, @twd2 wrote:

Hi, you can check here: https://buildkite.com/llvm-project/premerge-checks/builds/49755#814fd222-2e5a-4400-824d-d1a1f1293c01
The clang-tidy failed due to the invalid case style for variable 'kRISCVJumpTableEntrySize'.

We can ignore the lint check failure. The new constant follows the same style as the existing ones.

[MaskRay]

Needs a test in llvm/test/Transforms/LowerTypeTests

[twd2]

Add tests as suggested.
Use tail xxx@plt instruction instead of manually auipc + jr, which eliminates issues when linking dynamically-linked executables.

[twd2]

Ping?

[MaskRay]

LGTM.

[MaskRay]

I'll wait a bit before pushing to check whether that further opinions.

[twd2]

In D106888#3513001, @MaskRay wrote:

I'll wait a bit before pushing to check whether that further opinions.

Thanks! I don't have commit access, so could you please commit on my behalf when appropriate? Wende Tan <[e-mail]>

clang/lib/Driver/ToolChain.cpp

SanitizerMask Res =
       (SanitizerKind::CFI & ~SanitizerKind::CFIICall) |
       SanitizerKind::CFICastStrict | SanitizerKind::FloatDivideByZero |
       SanitizerKind::UnsignedIntegerOverflow |
       SanitizerKind::UnsignedShiftBase | SanitizerKind::ImplicitConversion |
       SanitizerKind::Nullability | SanitizerKind::LocalBounds;
   if (getTriple().getArch() == llvm::Triple::x86 ||
       getTriple().getArch() == llvm::Triple::x86_64 ||
       getTriple().getArch() == llvm::Triple::arm || getTriple().isWasm() ||
-      getTriple().isAArch64())
+      getTriple().isAArch64() || getTriple().isRISCV())
     Res |= SanitizerKind::CFIICall;
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
       getTriple().isAArch64(64) || getTriple().isRISCV())
     Res |= SanitizerKind::ShadowCallStack;
   if (getTriple().isAArch64(64))
     Res |= SanitizerKind::MemTag;
   return Res;
 }

clang/test/Driver/fsanitize.c

 // RUN: %clang -target x86_64-linux-gnu -fvisibility=hidden -fsanitize=cfi-unrelated-cast -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-UCAST
 // RUN: %clang -target x86_64-linux-gnu -flto -fvisibility=hidden -fsanitize=cfi-nvcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NVCALL
 // RUN: %clang -target x86_64-linux-gnu -flto -fvisibility=hidden -fsanitize=cfi-vcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-VCALL
 // RUN: %clang -target arm-linux-gnu -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target aarch64-linux-gnu -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target arm-linux-android -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target aarch64-linux-android -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target aarch64_be -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
+// RUN: %clang -target riscv32 -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
+// RUN: %clang -target riscv64 -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall
 // CHECK-CFI-NOMFCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall
 // CHECK-CFI-DCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast
 // CHECK-CFI-UCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-unrelated-cast
 // CHECK-CFI-NVCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-nvcall
 // CHECK-CFI-VCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-vcall
 
 // RUN: %clang -target x86_64-linux-gnu -fvisibility=hidden -flto -fsanitize=cfi-derived-cast -fno-lto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOLTO

llvm/lib/Transforms/IPO/LowerTypeTests.cpp

void LowerTypeTestsModule::verifyTypeMDNode(GlobalObject *GO, MDNode *Type) {
   auto OffsetInt = dyn_cast<ConstantInt>(OffsetConstMD->getValue());
   if (!OffsetInt)
     report_fatal_error("Type offset must be an integer constant");
 }
 
 static const unsigned kX86JumpTableEntrySize = 8;
 static const unsigned kARMJumpTableEntrySize = 4;
 static const unsigned kARMBTIJumpTableEntrySize = 8;
+static const unsigned kRISCVJumpTableEntrySize = 8;
 
 unsigned LowerTypeTestsModule::getJumpTableEntrySize() {
   switch (Arch) {
     case Triple::x86:
     case Triple::x86_64:
       return kX86JumpTableEntrySize;
     case Triple::arm:
     case Triple::thumb:
       return kARMJumpTableEntrySize;
     case Triple::aarch64:
       if (const auto *BTE = mdconst::extract_or_null<ConstantInt>(
             M.getModuleFlag("branch-target-enforcement")))
         if (BTE->getZExtValue())
           return kARMBTIJumpTableEntrySize;
       return kARMJumpTableEntrySize;
+    case Triple::riscv32:
+    case Triple::riscv64:
+      return kRISCVJumpTableEntrySize;
     default:
       report_fatal_error("Unsupported architecture for jump tables");
   }
 }
 
 // Create a jump table entry for the target. This consists of an instruction
 // sequence containing a relative branch to Dest. Appends inline asm text,
 // constraints and arguments to AsmOS, ConstraintOS and AsmArgs.
void LowerTypeTestsModule::createJumpTableEntry(
   } else if (JumpTableArch == Triple::aarch64) {
     if (const auto *BTE = mdconst::extract_or_null<ConstantInt>(
           Dest->getParent()->getModuleFlag("branch-target-enforcement")))
       if (BTE->getZExtValue())
         AsmOS << "bti c\n";
     AsmOS << "b $" << ArgIndex << "\n";
   } else if (JumpTableArch == Triple::thumb) {
     AsmOS << "b.w $" << ArgIndex << "\n";
+  } else if (JumpTableArch == Triple::riscv32 ||
+             JumpTableArch == Triple::riscv64) {
+    AsmOS << "tail $" << ArgIndex << "@plt\n";
   } else {
     report_fatal_error("Unsupported architecture for jump tables");
   }
 
   ConstraintOS << (ArgIndex > 0 ? ",s" : "s");
   AsmArgs.push_back(Dest);
 }
 
 Type *LowerTypeTestsModule::getJumpTableEntryType() {
   return ArrayType::get(Int8Ty, getJumpTableEntrySize());
 }
 
 /// Given a disjoint set of type identifiers and functions, build the bit sets
 /// and lower the llvm.type.test calls, architecture dependently.
 void LowerTypeTestsModule::buildBitSetsFromFunctions(
     ArrayRef<Metadata *> TypeIds, ArrayRef<GlobalTypeMember *> Functions) {
   if (Arch == Triple::x86 || Arch == Triple::x86_64 || Arch == Triple::arm ||
-      Arch == Triple::thumb || Arch == Triple::aarch64)
+      Arch == Triple::thumb || Arch == Triple::aarch64 ||
+      Arch == Triple::riscv32 || Arch == Triple::riscv64)
     buildBitSetsFromFunctionsNative(TypeIds, Functions);
   else if (Arch == Triple::wasm32 || Arch == Triple::wasm64)
     buildBitSetsFromFunctionsWASM(TypeIds, Functions);
   else
     report_fatal_error("Unsupported architecture for jump tables");
 }
 
 void LowerTypeTestsModule::moveInitializerToModuleConstructor(
if (JumpTableArch == Triple::thumb) {
     // Thumb jump table assembly needs Thumb2. The following attribute is added
     // by Clang for -march=armv7.
     F->addFnAttr("target-cpu", "cortex-a8");
   }
   if (JumpTableArch == Triple::aarch64) {
     F->addFnAttr("branch-target-enforcement", "false");
     F->addFnAttr("sign-return-address", "none");
   }
+  if (JumpTableArch == Triple::riscv32 || JumpTableArch == Triple::riscv64) {
+    // Make sure the jump table assembly is not modified by the assembler or
+    // the linker.
+    F->addFnAttr("target-features", "-c,-relax");
+  }
   // Make sure we don't emit .eh_frame for this function.
   F->addFnAttr(Attribute::NoUnwind);
 
   BasicBlock *BB = BasicBlock::Create(M.getContext(), "entry", F);
   IRBuilder<> IRB(BB);
 
   SmallVector<Type *, 16> ArgTypes;
   ArgTypes.reserve(AsmArgs.size());

llvm/test/Transforms/LowerTypeTests/function-weak.ll

 ; RUN: opt -S -lowertypetests -mtriple=i686-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,X86 %s
 ; RUN: opt -S -lowertypetests -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,X86 %s
 ; RUN: opt -S -lowertypetests -mtriple=arm-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,ARM %s
 ; RUN: opt -S -lowertypetests -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,ARM %s
+; RUN: opt -S -lowertypetests -mtriple=riscv32-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,RISCV %s
+; RUN: opt -S -lowertypetests -mtriple=riscv64-unknown-linux-gnu < %s | FileCheck --check-prefixes=CHECK,RISCV %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; CHECK: @x = global void ()* null, align 8
 @x = global void ()* @f, align 8
 
 ; CHECK: @x2 = global void ()* null, align 8

 
 define i1 @foo(i8* %p) {
   %x = call i1 @llvm.type.test(i8* %p, metadata !"typeid1")
   ret i1 %x
 }
 
 ; X86: define private void @[[JT]]() #{{.*}} align 8 {
 ; ARM: define private void @[[JT]]() #{{.*}} align 4 {
+; RISCV: define private void @[[JT]]() #{{.*}} align 8 {
 
 ; CHECK: define internal void @__cfi_global_var_init() section ".text.startup" {
 ; CHECK-NEXT: entry:
 ; CHECK-NEXT: store void ()* select (i1 icmp ne (void ()* @f, void ()* null), void ()* @[[JT]], void ()* null), void ()** @x, align 8
 ; CHECK-NEXT: store void ()* select (i1 icmp ne (void ()* @f, void ()* null), void ()* @[[JT]], void ()* null), void ()** @x2, align 8
 ; CHECK-NEXT: store void ()* select (i1 icmp ne (void ()* @f, void ()* null), void ()* @[[JT]], void ()* null), void ()** @x3, align 8
 ; CHECK-NEXT: store void ()* bitcast (i8* getelementptr (i8, i8* bitcast (void ()* select (i1 icmp ne (void ()* @f, void ()* null), void ()* @[[JT]], void ()* null) to i8*), i64 42) to void ()*), void ()** @x4, align 8
 ; CHECK-NEXT: store { void ()*, void ()*, i32 } { void ()* select (i1 icmp ne (void ()* @f, void ()* null), void ()* @[[JT]], void ()* null), void ()* select (i1 icmp ne (void ()* @f, void ()* null), void ()* @[[JT]], void ()* null), i32 42 }, { void ()*, void ()*, i32 }* @s, align 8
 ; CHECK-NEXT: ret void
 ; CHECK-NEXT: }
 
 !0 = !{i32 0, !"typeid1"}

llvm/test/Transforms/LowerTypeTests/function.ll

 ; RUN: opt -S -lowertypetests -mtriple=i686-unknown-linux-gnu < %s | FileCheck --check-prefixes=X86,X86-LINUX,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck --check-prefixes=X86,X86-LINUX,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=i686-pc-win32 < %s | FileCheck --check-prefixes=X86,X86-WIN32,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=x86_64-pc-win32 < %s | FileCheck --check-prefixes=X86,X86-WIN32,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=arm-unknown-linux-gnu < %s | FileCheck --check-prefixes=ARM,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=thumb-unknown-linux-gnu < %s | FileCheck --check-prefixes=THUMB,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck --check-prefixes=ARM,NATIVE %s
+; RUN: opt -S -lowertypetests -mtriple=riscv32-unknown-linux-gnu < %s | FileCheck --check-prefixes=RISCV,NATIVE %s
+; RUN: opt -S -lowertypetests -mtriple=riscv64-unknown-linux-gnu < %s | FileCheck --check-prefixes=RISCV,NATIVE %s
 ; RUN: opt -S -lowertypetests -mtriple=wasm32-unknown-unknown < %s | FileCheck --check-prefix=WASM32 %s
 
 ; Tests that we correctly handle bitsets containing 2 or more functions.
 
 target datalayout = "e-p:64:64"
 
 
 ; NATIVE: @0 = private unnamed_addr constant [2 x void (...)*] [void (...)* bitcast (void ()* @f to void (...)*), void (...)* bitcast (void ()* @g to void (...)*)], align 16
 @0 = private unnamed_addr constant [2 x void (...)*] [void (...)* bitcast (void ()* @f to void (...)*), void (...)* bitcast (void ()* @g to void (...)*)], align 16
 
 ; NATIVE: private constant [0 x i8] zeroinitializer
 ; WASM32: private constant [0 x i8] zeroinitializer
 
 ; NATIVE: @f = alias void (), void ()* @[[JT:.*]]
 
 ; X86: @g = internal alias void (), bitcast ([8 x i8]* getelementptr inbounds ([2 x [8 x i8]], [2 x [8 x i8]]* bitcast (void ()* @[[JT]] to [2 x [8 x i8]]*), i64 0, i64 1) to void ()*)
 ; ARM: @g = internal alias void (), bitcast ([4 x i8]* getelementptr inbounds ([2 x [4 x i8]], [2 x [4 x i8]]* bitcast (void ()* @[[JT]] to [2 x [4 x i8]]*), i64 0, i64 1) to void ()*)
 ; THUMB: @g = internal alias void (), bitcast ([4 x i8]* getelementptr inbounds ([2 x [4 x i8]], [2 x [4 x i8]]* bitcast (void ()* @[[JT]] to [2 x [4 x i8]]*), i64 0, i64 1) to void ()*)
+; RISCV: @g = internal alias void (), bitcast ([8 x i8]* getelementptr inbounds ([2 x [8 x i8]], [2 x [8 x i8]]* bitcast (void ()* @[[JT]] to [2 x [8 x i8]]*), i64 0, i64 1) to void ()*)
 
 ; NATIVE: define hidden void @f.cfi()
 ; WASM32: define void @f() !type !{{[0-9]+}} !wasm.index ![[I0:[0-9]+]]
 define void @f() !type !0 {
   ret void
 }
 
 ; NATIVE: define internal void @g.cfi()
define i1 @foo(i8* %p) {
   %x = call i1 @llvm.type.test(i8* %p, metadata !"typeid1")
   ret i1 %x
 }
 
 ; X86-LINUX:   define private void @[[JT]]() #[[ATTR:.*]] align 8 {
 ; X86-WIN32:   define private void @[[JT]]() #[[ATTR:.*]] align 8 {
 ; ARM:   define private void @[[JT]]() #[[ATTR:.*]] align 4 {
 ; THUMB: define private void @[[JT]]() #[[ATTR:.*]] align 4 {
+; RISCV: define private void @[[JT]]() #[[ATTR:.*]] align 8 {
 
 ; X86:      jmp ${0:c}@plt
 ; X86-SAME: int3
 ; X86-SAME: int3
 ; X86-SAME: int3
 ; X86-SAME: jmp ${1:c}@plt
 ; X86-SAME: int3
 ; X86-SAME: int3
 ; X86-SAME: int3
 
 ; ARM:      b $0
 ; ARM-SAME: b $1
 
 ; THUMB:      b.w $0
 ; THUMB-SAME: b.w $1
 
+; RISCV:      tail $0@plt
+; RISCV-SAME: tail $1@plt
+
 ; NATIVE-SAME: "s,s"(void ()* @f.cfi, void ()* @g.cfi)
 
 ; X86-LINUX: attributes #[[ATTR]] = { naked nounwind }
 ; X86-WIN32: attributes #[[ATTR]] = { nounwind }
 ; ARM: attributes #[[ATTR]] = { naked nounwind
 ; THUMB: attributes #[[ATTR]] = { naked nounwind "target-cpu"="cortex-a8" "target-features"="+thumb-mode" }
+; RISCV: attributes #[[ATTR]] = { naked nounwind "target-features"="-c,-relax" }
 
 ; WASM32: ![[I0]] = !{i64 1}
 ; WASM32: ![[I1]] = !{i64 2}