[llvm] 81429ab - [Attributor][FIX] Do not create UB by introducing a `noundef undef`

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 9 11:02:54 PST 2021


Author: Johannes Doerfert
Date: 2021-02-09T13:02:38-06:00
New Revision: 81429abd83362f4c3eecbed41ebc917ddf26b13f

URL: https://github.com/llvm/llvm-project/commit/81429abd83362f4c3eecbed41ebc917ddf26b13f
DIFF: https://github.com/llvm/llvm-project/commit/81429abd83362f4c3eecbed41ebc917ddf26b13f.diff

LOG: [Attributor][FIX] Do not create UB by introducing a `noundef undef`

This was reported as PR49104. The reproducer uses varargs but the issue
is the same, we know an argument is dead but can't change the signature
for some reason. The PR49104 situation was: We are in an CG-SCC
traversal and we remove all the uses of an argument and proof it thereby
dead. However, if we do not remove the argument, via signature rewrite,
we need to ensure that the `undef` we introduce at the call site doesn't
clash with a `noundef` attribute.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/ArgumentPromotion/musttail.ll
    llvm/test/Transforms/Attributor/align.ll
    llvm/test/Transforms/Attributor/noundef.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 03ad45135001..ba2423ab6b5e 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Analysis/MemorySSAUpdater.h"
 #include "llvm/Analysis/MustExecute.h"
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/IR/Attributes.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/NoFolder.h"
@@ -1252,6 +1253,17 @@ ChangeStatus Attributor::cleanupIR() {
           isInstructionTriviallyDead(I))
         DeadInsts.push_back(I);
     }
+    if (isa<UndefValue>(NewV) && isa<CallBase>(U->getUser())) {
+      auto *CB = cast<CallBase>(U->getUser());
+      if (CB->isArgOperand(U)) {
+        unsigned Idx = CB->getArgOperandNo(U);
+        CB->removeParamAttr(Idx, Attribute::NoUndef);
+        Function *Fn = CB->getCalledFunction();
+        assert(Fn && "Expected callee when call argument is replaced!");
+        if (Fn->arg_size() > Idx)
+          Fn->removeParamAttr(Idx, Attribute::NoUndef);
+      }
+    }
     if (isa<Constant>(NewV) && isa<BranchInst>(U->getUser())) {
       Instruction *UserI = cast<Instruction>(U->getUser());
       if (isa<UndefValue>(NewV)) {

diff  --git a/llvm/test/Transforms/Attributor/ArgumentPromotion/musttail.ll b/llvm/test/Transforms/Attributor/ArgumentPromotion/musttail.ll
index 34adb3ebc232..9374fa75816e 100644
--- a/llvm/test/Transforms/Attributor/ArgumentPromotion/musttail.ll
+++ b/llvm/test/Transforms/Attributor/ArgumentPromotion/musttail.ll
@@ -167,7 +167,7 @@ define i32 @caller2b(%T* %g) {
 ; IS__CGSCC____: Function Attrs: argmemonly nofree norecurse nosync nounwind willreturn
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@caller2b
 ; IS__CGSCC____-SAME: (%T* nocapture nofree readonly [[G:%.*]]) [[ATTR3]] {
-; IS__CGSCC____-NEXT:    [[V:%.*]] = call i32 @test2b(%T* nocapture nofree readonly [[G]], i32 noundef undef) [[ATTR6:#.*]]
+; IS__CGSCC____-NEXT:    [[V:%.*]] = call i32 @test2b(%T* nocapture nofree readonly [[G]], i32 undef) [[ATTR6:#.*]]
 ; IS__CGSCC____-NEXT:    ret i32 0
 ;
   %v = call i32 @test2b(%T* %g, i32 0)

diff  --git a/llvm/test/Transforms/Attributor/align.ll b/llvm/test/Transforms/Attributor/align.ll
index c210763ce3dd..dd5e8815bbd3 100644
--- a/llvm/test/Transforms/Attributor/align.ll
+++ b/llvm/test/Transforms/Attributor/align.ll
@@ -157,7 +157,7 @@ define i32* @test6_2() #0 {
 define internal i8* @f1(i8* readnone %0) local_unnamed_addr #0 {
 ; IS__TUNIT____: Function Attrs: nofree noinline nosync nounwind readnone uwtable willreturn
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@f1
-; IS__TUNIT____-SAME: (i8* noalias nofree noundef nonnull readnone returned align 8 dereferenceable(1) "no-capture-maybe-returned" [[TMP0:%.*]]) local_unnamed_addr [[ATTR0]] {
+; IS__TUNIT____-SAME: (i8* noalias nofree nonnull readnone returned align 8 dereferenceable(1) "no-capture-maybe-returned" [[TMP0:%.*]]) local_unnamed_addr [[ATTR0]] {
 ; IS__TUNIT____-NEXT:    br label [[TMP3:%.*]]
 ; IS__TUNIT____:       2:
 ; IS__TUNIT____-NEXT:    unreachable
@@ -166,7 +166,7 @@ define internal i8* @f1(i8* readnone %0) local_unnamed_addr #0 {
 ;
 ; IS__CGSCC_OPM: Function Attrs: nofree noinline nosync nounwind readnone uwtable willreturn
 ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@f1
-; IS__CGSCC_OPM-SAME: (i8* noalias nofree noundef nonnull readnone returned align 8 dereferenceable(1) "no-capture-maybe-returned" [[TMP0:%.*]]) local_unnamed_addr [[ATTR2:#.*]] {
+; IS__CGSCC_OPM-SAME: (i8* noalias nofree nonnull readnone returned align 8 dereferenceable(1) "no-capture-maybe-returned" [[TMP0:%.*]]) local_unnamed_addr [[ATTR2:#.*]] {
 ; IS__CGSCC_OPM-NEXT:    br label [[TMP3:%.*]]
 ; IS__CGSCC_OPM:       2:
 ; IS__CGSCC_OPM-NEXT:    unreachable
@@ -251,7 +251,7 @@ define align 4 i8* @test7() #0 {
 define internal i8* @f1b(i8* readnone %0) local_unnamed_addr #0 {
 ; IS__CGSCC_OPM: Function Attrs: nofree noinline nosync nounwind readnone uwtable willreturn
 ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@f1b
-; IS__CGSCC_OPM-SAME: (i8* noalias nofree noundef nonnull readnone returned align 8 dereferenceable(1) "no-capture-maybe-returned" [[TMP0:%.*]]) local_unnamed_addr [[ATTR2]] {
+; IS__CGSCC_OPM-SAME: (i8* noalias nofree nonnull readnone returned align 8 dereferenceable(1) "no-capture-maybe-returned" [[TMP0:%.*]]) local_unnamed_addr [[ATTR2]] {
 ; IS__CGSCC_OPM-NEXT:    br label [[TMP3:%.*]]
 ; IS__CGSCC_OPM:       2:
 ; IS__CGSCC_OPM-NEXT:    unreachable

diff  --git a/llvm/test/Transforms/Attributor/noundef.ll b/llvm/test/Transforms/Attributor/noundef.ll
index 211338eefa0b..75241f5d7a91 100644
--- a/llvm/test/Transforms/Attributor/noundef.ll
+++ b/llvm/test/Transforms/Attributor/noundef.ll
@@ -62,6 +62,65 @@ define void @callback_caller() {
   ret void
 }
 
+; Drop the noundef if when we replace the call argument with `undef`. We use a
+; varargs function as we cannot (yet) rewrite their signature. If we ever can,
+; try to come up with a 
diff erent scheme to verify the `noundef` is dropped if
+; signature rewriting is not happening.
+define internal void @callee_with_dead_noundef_arg(i1 noundef %create, ...) {
+; CHECK-LABEL: define {{[^@]+}}@callee_with_dead_noundef_arg
+; CHECK-SAME: (i1 [[CREATE:%.*]], ...) {
+; CHECK-NEXT:    call void @unknown()
+; CHECK-NEXT:    ret void
+;
+  call void @unknown()
+  ret void
+}
+
+define void @caller_with_unused_arg(i1 %c) {
+; CHECK-LABEL: define {{[^@]+}}@caller_with_unused_arg
+; CHECK-SAME: (i1 [[C:%.*]]) {
+; CHECK-NEXT:    call void (i1, ...) @callee_with_dead_noundef_arg(i1 undef)
+; CHECK-NEXT:    ret void
+;
+  call void (i1, ...) @callee_with_dead_noundef_arg(i1 %c)
+  ret void
+}
+
+define internal void @callee_with_dead_arg(i1 %create, ...) {
+; CHECK-LABEL: define {{[^@]+}}@callee_with_dead_arg
+; CHECK-SAME: (i1 [[CREATE:%.*]], ...) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[IF_THEN3:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    unreachable
+; CHECK:       if.then3:
+; CHECK-NEXT:    call void @unknown()
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %create, label %if.then3, label %if.then
+
+if.then:                                          ; preds = %entry
+  ret void
+
+if.then3:                                         ; preds = %entry
+  call void @unknown()
+  ret void
+}
+
+; Drop the noundef if when we replace the call argument with `undef`. We use a
+; varargs function as we cannot (yet) rewrite their signature. If we ever can,
+; try to come up with a 
diff erent scheme to verify the `noundef` is dropped if
+; signature rewriting is not happening.
+define void @caller_with_noundef_arg() {
+; CHECK-LABEL: define {{[^@]+}}@caller_with_noundef_arg() {
+; CHECK-NEXT:    call void (i1, ...) @callee_with_dead_arg(i1 undef)
+; CHECK-NEXT:    ret void
+;
+  call void (i1, ...) @callee_with_dead_arg(i1 noundef true)
+  ret void
+}
+
 declare !callback !0 void @callback_broker(void (i8*)*, i8*)
 !1 = !{i64 0, i64 1, i1 false}
 !0 = !{!1}


        


More information about the llvm-commits mailing list