[PATCH] D114728: [Coroutine] Remove the prologue data of `-fsanitize=function` for split functions

Yuanfang Chen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 29 11:20:21 PST 2021


ychen created this revision.
ychen added reviewers: pcc, rjmccall.
Herald added subscribers: ChuanqiXu, lxfind, hiraditya.
ychen requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

There are no proper RTTI for these split functions. So just delete the
prologue data.

Fixes PR50345.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114728

Files:
  clang/test/CodeGenCXX/catch-undef-behavior.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-split-00.ll


Index: llvm/test/Transforms/Coroutines/coro-split-00.ll
===================================================================
--- llvm/test/Transforms/Coroutines/coro-split-00.ll
+++ llvm/test/Transforms/Coroutines/coro-split-00.ll
@@ -1,7 +1,7 @@
 ; Tests that coro-split pass splits the coroutine into f, f.resume and f.destroy
 ; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
 
-define i8* @f() "coroutine.presplit"="1" {
+define i8* @f() "coroutine.presplit"="1" prologue <{ i32, i32 }> <{ i32 846595819, i32 1 }> {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %need.alloc = call i1 @llvm.coro.alloc(token %id)
@@ -32,7 +32,7 @@
   ret i8* %hdl
 }
 
-; CHECK-LABEL: @f(
+; CHECK-LABEL: @f() prologue <{ i32, i32 }> <{ i32 846595819, i32 1 }> {
 ; CHECK: call i8* @malloc
 ; CHECK: @llvm.coro.begin(token %id, i8* %phi)
 ; CHECK: store void (%f.Frame*)* @f.resume, void (%f.Frame*)** %resume.addr
@@ -43,7 +43,7 @@
 ; CHECK-NOT: call void @free(
 ; CHECK: ret i8* %hdl
 
-; CHECK-LABEL: @f.resume(
+; CHECK-LABEL: @f.resume({{.*}}) {
 ; CHECK-NOT: call i8* @malloc
 ; CHECK-NOT: call void @print(i32 0)
 ; CHECK: call void @print(i32 1)
@@ -51,13 +51,13 @@
 ; CHECK: call void @free(
 ; CHECK: ret void
 
-; CHECK-LABEL: @f.destroy(
+; CHECK-LABEL: @f.destroy({{.*}}) {
 ; CHECK-NOT: call i8* @malloc
 ; CHECK-NOT: call void @print(
 ; CHECK: call void @free(
 ; CHECK: ret void
 
-; CHECK-LABEL: @f.cleanup(
+; CHECK-LABEL: @f.cleanup({{.*}}) {
 ; CHECK-NOT: call i8* @malloc
 ; CHECK-NOT: call void @print(
 ; CHECK-NOT: call void @free(
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===================================================================
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -896,6 +896,20 @@
   NewF->setUnnamedAddr(savedUnnamedAddr);
   NewF->setDLLStorageClass(savedDLLStorageClass);
 
+  if (Shape.ABI == coro::ABI::Switch && NewF->hasPrologueData()) {
+    if (auto *CS = dyn_cast<ConstantStruct>(NewF->getPrologueData())) {
+      if (auto *CI = dyn_cast<ConstantInt>(CS->getOperand(0))) {
+        // This value should match the value returned by
+        // TargetCodeGenInfo::getUBSanFunctionSignature().
+        unsigned Sig = (0xeb << 0) | // jmp rel8
+                       (0x06 << 8) | //           .+0x08
+                       ('v' << 16) | ('2' << 24);
+        if (CI->getZExtValue() == Sig)
+          NewF->setPrologueData(nullptr);
+      }
+    }
+  }
+
   // Replace the attributes of the new function:
   auto OrigAttrs = NewF->getAttributes();
   auto NewAttrs = AttributeList();
Index: clang/test/CodeGenCXX/catch-undef-behavior.cpp
===================================================================
--- clang/test/CodeGenCXX/catch-undef-behavior.cpp
+++ clang/test/CodeGenCXX/catch-undef-behavior.cpp
@@ -399,7 +399,12 @@
   // CHECK-NEXT: br i1 [[AND]]
 }
 
-//
+// FIXME: If the function signature value is changed, make the same change in
+//        CoroSplit. Making them share a single API to avoid this is better.
+//        However, function prefix/prologue data mechansim do not work well with
+//        coroutine split in general, we may want to redesign that or
+//        deprecate some features (see intended usages in
+//        https://reviews.llvm.org/D6454) in the future.
 // CHECK-LABEL: @_Z22indirect_function_callPFviE({{.*}} prologue <{ i32, i32 }> <{ i32 846595819, i32 trunc (i64 sub (i64 ptrtoint (i8** {{.*}} to i64), i64 ptrtoint (void (void (i32)*)* @_Z22indirect_function_callPFviE to i64)) to i32) }>
 // CHECK-X32: @_Z22indirect_function_callPFviE({{.*}} prologue <{ i32, i32 }> <{ i32 846595819, i32 sub (i32 ptrtoint (i8** [[IndirectRTTI_ZTIFvPFviEE]] to i32), i32 ptrtoint (void (void (i32)*)* @_Z22indirect_function_callPFviE to i32)) }>
 // CHECK-X86: @_Z22indirect_function_callPFviE({{.*}} prologue <{ i32, i32 }> <{ i32 846595819, i32 sub (i32 ptrtoint (i8** [[IndirectRTTI_ZTIFvPFviEE]] to i32), i32 ptrtoint (void (void (i32)*)* @_Z22indirect_function_callPFviE to i32)) }>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114728.390421.patch
Type: text/x-patch
Size: 4106 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211129/47dcb2cf/attachment-0001.bin>


More information about the cfe-commits mailing list