[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 << "1: auipc t0, %pcrel_hi($" << ArgIndex << ")\n"
+          << "jr %pcrel_lo(1b)(t0)\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());