[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2022-05-14 Thread Wende Tan via Phabricator via cfe-commits 
twd2 added a comment.

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 


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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2022-05-13 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

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


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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2022-05-13 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2022-05-13 Thread Wende Tan via Phabricator via cfe-commits 
twd2 added a comment.

Ping?


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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2022-05-07 Thread Wende Tan via Phabricator via cfe-commits 
twd2 updated this revision to Diff 427901.
twd2 added a comment.
Herald added subscribers: sunshaoce, StephenFan, arichardson.
Herald added a project: All.

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


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

https://reviews.llvm.org/D106888

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fsanitize.c
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp
  llvm/test/Transforms/LowerTypeTests/function-weak.ll
  llvm/test/Transforms/LowerTypeTests/function.ll

Index: llvm/test/Transforms/LowerTypeTests/function.ll
===
--- llvm/test/Transforms/LowerTypeTests/function.ll
+++ llvm/test/Transforms/LowerTypeTests/function.ll
@@ -5,6 +5,8 @@
 ; 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.
@@ -23,6 +25,7 @@
 ; 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]+]]
@@ -52,6 +55,7 @@
 ; 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
@@ -68,12 +72,16 @@
 ; 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}
Index: llvm/test/Transforms/LowerTypeTests/function-weak.ll
===
--- llvm/test/Transforms/LowerTypeTests/function-weak.ll
+++ llvm/test/Transforms/LowerTypeTests/function-weak.ll
@@ -2,6 +2,8 @@
 ; 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"
@@ -52,6 +54,7 @@
 
 ; 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:
Index: llvm/lib/Transforms/IPO/LowerTypeTests.cpp
===
--- llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1223,6 +1223,7 @@
 static const unsigned kX86JumpTableEntrySize = 8;
 static const unsigned kARMJumpTableEntrySize = 4;
 static

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2022-02-11 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

Needs a test in llvm/test/Transforms/LowerTypeTests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2021-08-19 Thread Luís Marques via Phabricator via cfe-commits 
luismarques added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2021-08-19 Thread Wende Tan via Phabricator via cfe-commits 
twd2 added a comment.

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'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2021-08-19 Thread Luís Marques via Phabricator via cfe-commits 
luismarques added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2021-08-19 Thread Alex Bradbury via Phabricator via cfe-commits 
asb added a comment.

Is it possible to write a test case for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2021-08-05 Thread Wende Tan via Phabricator via cfe-commits 
twd2 added a comment.

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`..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106888

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106888: [RISC-V] Implement jump tables for CFI-icall

2021-07-27 Thread Wende Tan via Phabricator via cfe-commits 
twd2 created this revision.
twd2 added reviewers: MaskRay, asb, eugenis, pcc.
twd2 created this object with edit policy "Administrators".
twd2 added projects: LLVM, Sanitizers, clang.
Herald added subscribers: ormris, vkmr, luismarques, sameer.abuasal, s.egerton, 
Jim, PkmX, rogfer01, shiva0217, kito-cheng, simoncook, hiraditya.
twd2 requested review of this revision.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106888

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fsanitize.c
  llvm/lib/Transforms/IPO/LowerTypeTests.cpp


Index: llvm/lib/Transforms/IPO/LowerTypeTests.cpp
===
--- llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -1211,6 +1211,7 @@
 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) {
@@ -1226,6 +1227,9 @@
 if (BTE->getZExtValue())
   return kARMBTIJumpTableEntrySize;
   return kARMJumpTableEntrySize;
+case Triple::riscv32:
+case Triple::riscv64:
+  return kRISCVJumpTableEntrySize;
 default:
   report_fatal_error("Unsupported architecture for jump tables");
   }
@@ -1253,6 +1257,10 @@
 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");
   }
@@ -1270,7 +1278,8 @@
 void LowerTypeTestsModule::buildBitSetsFromFunctions(
 ArrayRef TypeIds, ArrayRef 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);
@@ -1415,6 +1424,11 @@
 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);
 
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -603,6 +603,8 @@
 // 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
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -1008,7 +1008,7 @@
   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())


Index: llvm/lib/Transforms/IPO/LowerTypeTests.cpp
===
--- llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++