[Attributor] Check violation of returned position nonnull and noundef attribute in AAUndefinedBehavior

[okura]

Summary

This patch is a follow up of D84733.
If a function has noundef attribute in returned position, instructions that return undef or poison value cause UB.

[jdoerfert]

Cool :)

There is a typo in your subject line [Attributor].

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2052

I don't think we need to simplify the value here. If it is not simplified, it should happen as part of AAReturned. I guess we can just not do it for now?

2057

Reading this makes me question if it should ever happen. I hope AAReturnedValues will not look at dead returns so they should not be part of the RetInsts set and if that set is empty this should never be called.

2074–2079

2085

Note: When the other patch lands we should check if we want to ask the AANoUndef here.

[okura]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2057

I overlooked the fact that a dead return instruction is ignored in checkForAllReturnedValuesAndReturnInsts. I'll fix this comment.

[okura]

• fix comment
• change the callback so that it does not simplify the value.

[okura]

• clarify that dead returned values are ignored in checkForReturnedValuesAndReturnedInsts in a comment.

[jdoerfert]

LGTM

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

auto InspectCallSiteForUB = [&](Instruction &I) {
             isa<UndefValue>(*SimplifiedVal.getValue())) {
           KnownUBInsts.insert(&I);
           return true;
         }
       }
       return true;
     };
 
+    auto InspectReturnInstForUB =
+        [&](Value &V, const SmallSetVector<ReturnInst *, 4> RetInsts) {
+          // Check if a return instruction always cause UB or not
+          // Note: It is guaranteed that the returned position of the anchor
+          //       scope has noundef attribute when this is called.
+
+          // When the returned position has noundef attriubte, UB occur in the
+          // following cases.
+          //   (1) Returned value is known to be undef.
+          //   (2) The value is known to be a null pointer and the returned
+          //       position has nonnull attribute (because the returned value is
+          //       poison).
+          // Note: This callback is not called for a dead returned value because
+          //       such values are ignored in
+          //       checkForAllReturnedValuesAndReturnedInsts.
+          bool FoundUB = false;
+          if (isa<UndefValue>(V)) {
+            FoundUB = true;
+          } else {
+            auto &NonNullAA = A.getAAFor<AANonNull>(
+                *this, IRPosition::returned(*getAnchorScope()));
+            if (NonNullAA.isKnownNonNull() && isa<ConstantPointerNull>(V))
+              FoundUB = true;
+          }
+
+          if (FoundUB)
+            for (ReturnInst *RI : RetInsts)
+              KnownUBInsts.insert(RI);
+          return true;
+        };
+
     A.checkForAllInstructions(InspectMemAccessInstForUB, *this,
                               {Instruction::Load, Instruction::Store,
                                Instruction::AtomicCmpXchg,
                                Instruction::AtomicRMW},
                               /* CheckBBLivenessOnly */ true);
     A.checkForAllInstructions(InspectBrInstForUB, *this, {Instruction::Br},
                               /* CheckBBLivenessOnly */ true);
     A.checkForAllCallLikeInstructions(InspectCallSiteForUB, *this);
+
+    // If the returned position of the anchor scope has noundef attriubte, check
+    // all returned instructions.
+    // TODO: If AANoUndef is implemented, ask it here.
+    if (IRPosition::returned(*getAnchorScope()).hasAttr({Attribute::NoUndef}))
+      A.checkForAllReturnedValuesAndReturnInsts(InspectReturnInstForUB, *this);
+
     if (NoUBPrevSize != AssumedNoUBInsts.size() ||
         UBPrevSize != KnownUBInsts.size())
       return ChangeStatus::CHANGED;
     return ChangeStatus::UNCHANGED;
   }
 
   bool isKnownToCauseUB(Instruction *I) const override {
     return KnownUBInsts.count(I);

llvm/test/Transforms/Attributor/undefined_behavior.ll

 ; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@foo()
 ; IS__CGSCC____-NEXT:    ret i32 1
 ;
   %X = call i32 @callee(i1 false, i32* null)
   ret i32 %X
 }
 
-; Tests for nonnull attribute violation.
+; Tests for nonnull noundef attribute violation.
+;
+; Tests for argument position
 
 define void @arg_nonnull_1(i32* nonnull %a) {
 ; IS__TUNIT____: Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@arg_nonnull_1
 ; IS__TUNIT____-SAME: (i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[A:%.*]])
 ; IS__TUNIT____-NEXT:    store i32 0, i32* [[A]], align 4
 ; IS__TUNIT____-NEXT:    ret void
 ;
f:
   call void @arg_nonnull_12_noundef_2(i32* null, i32* %ptr, i32* %ptr)
   call void @arg_nonnull_12_noundef_2(i32* null, i32* %ptr, i32* null)
   call void @arg_nonnull_12_noundef_2(i32* null, i32* null, i32* %ptr)
   call void @arg_nonnull_12_noundef_2(i32* null, i32* null, i32* null)
   br label %ret
 ret:
   ret void
 }
+
+; Tests for returned position
+
+define nonnull i32* @returned_nonnnull(i32 %c) {
+; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
+; IS__TUNIT____-LABEL: define {{[^@]+}}@returned_nonnnull
+; IS__TUNIT____-SAME: (i32 [[C:%.*]])
+; IS__TUNIT____-NEXT:    switch i32 [[C]], label [[ONDEFAULT:%.*]] [
+; IS__TUNIT____-NEXT:    i32 0, label [[ONZERO:%.*]]
+; IS__TUNIT____-NEXT:    i32 1, label [[ONONE:%.*]]
+; IS__TUNIT____-NEXT:    ]
+; IS__TUNIT____:       onzero:
+; IS__TUNIT____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__TUNIT____-NEXT:    ret i32* [[PTR]]
+; IS__TUNIT____:       onone:
+; IS__TUNIT____-NEXT:    ret i32* null
+; IS__TUNIT____:       ondefault:
+; IS__TUNIT____-NEXT:    ret i32* undef
+;
+; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
+; IS__CGSCC____-LABEL: define {{[^@]+}}@returned_nonnnull
+; IS__CGSCC____-SAME: (i32 [[C:%.*]])
+; IS__CGSCC____-NEXT:    switch i32 [[C]], label [[ONDEFAULT:%.*]] [
+; IS__CGSCC____-NEXT:    i32 0, label [[ONZERO:%.*]]
+; IS__CGSCC____-NEXT:    i32 1, label [[ONONE:%.*]]
+; IS__CGSCC____-NEXT:    ]
+; IS__CGSCC____:       onzero:
+; IS__CGSCC____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__CGSCC____-NEXT:    ret i32* [[PTR]]
+; IS__CGSCC____:       onone:
+; IS__CGSCC____-NEXT:    ret i32* null
+; IS__CGSCC____:       ondefault:
+; IS__CGSCC____-NEXT:    ret i32* undef
+;
+  switch i32 %c, label %ondefault [ i32 0, label %onzero
+  i32 1, label %onone ]
+onzero:
+  %ptr = alloca i32
+  ret i32* %ptr
+onone:
+  ret i32* null
+ondefault:
+  ret i32* undef
+}
+
+define noundef i32* @returned_noundef(i32 %c) {
+; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
+; IS__TUNIT____-LABEL: define {{[^@]+}}@returned_noundef
+; IS__TUNIT____-SAME: (i32 [[C:%.*]])
+; IS__TUNIT____-NEXT:    switch i32 [[C]], label [[ONDEFAULT:%.*]] [
+; IS__TUNIT____-NEXT:    i32 0, label [[ONZERO:%.*]]
+; IS__TUNIT____-NEXT:    i32 1, label [[ONONE:%.*]]
+; IS__TUNIT____-NEXT:    ]
+; IS__TUNIT____:       onzero:
+; IS__TUNIT____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__TUNIT____-NEXT:    ret i32* [[PTR]]
+; IS__TUNIT____:       onone:
+; IS__TUNIT____-NEXT:    ret i32* null
+; IS__TUNIT____:       ondefault:
+; IS__TUNIT____-NEXT:    unreachable
+;
+; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
+; IS__CGSCC____-LABEL: define {{[^@]+}}@returned_noundef
+; IS__CGSCC____-SAME: (i32 [[C:%.*]])
+; IS__CGSCC____-NEXT:    switch i32 [[C]], label [[ONDEFAULT:%.*]] [
+; IS__CGSCC____-NEXT:    i32 0, label [[ONZERO:%.*]]
+; IS__CGSCC____-NEXT:    i32 1, label [[ONONE:%.*]]
+; IS__CGSCC____-NEXT:    ]
+; IS__CGSCC____:       onzero:
+; IS__CGSCC____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__CGSCC____-NEXT:    ret i32* [[PTR]]
+; IS__CGSCC____:       onone:
+; IS__CGSCC____-NEXT:    ret i32* null
+; IS__CGSCC____:       ondefault:
+; IS__CGSCC____-NEXT:    unreachable
+;
+  switch i32 %c, label %ondefault [ i32 0, label %onzero
+  i32 1, label %onone ]
+onzero:
+  %ptr = alloca i32
+  ret i32* %ptr
+onone:
+  ret i32* null
+ondefault:
+  ret i32* undef
+}
+
+define nonnull noundef i32* @returned_nonnnull_noundef(i32 %c) {
+; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
+; IS__TUNIT____-LABEL: define {{[^@]+}}@returned_nonnnull_noundef
+; IS__TUNIT____-SAME: (i32 [[C:%.*]])
+; IS__TUNIT____-NEXT:    switch i32 [[C]], label [[ONDEFAULT:%.*]] [
+; IS__TUNIT____-NEXT:    i32 0, label [[ONZERO:%.*]]
+; IS__TUNIT____-NEXT:    i32 1, label [[ONONE:%.*]]
+; IS__TUNIT____-NEXT:    ]
+; IS__TUNIT____:       onzero:
+; IS__TUNIT____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__TUNIT____-NEXT:    ret i32* [[PTR]]
+; IS__TUNIT____:       onone:
+; IS__TUNIT____-NEXT:    unreachable
+; IS__TUNIT____:       ondefault:
+; IS__TUNIT____-NEXT:    unreachable
+;
+; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
+; IS__CGSCC____-LABEL: define {{[^@]+}}@returned_nonnnull_noundef
+; IS__CGSCC____-SAME: (i32 [[C:%.*]])
+; IS__CGSCC____-NEXT:    switch i32 [[C]], label [[ONDEFAULT:%.*]] [
+; IS__CGSCC____-NEXT:    i32 0, label [[ONZERO:%.*]]
+; IS__CGSCC____-NEXT:    i32 1, label [[ONONE:%.*]]
+; IS__CGSCC____-NEXT:    ]
+; IS__CGSCC____:       onzero:
+; IS__CGSCC____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__CGSCC____-NEXT:    ret i32* [[PTR]]
+; IS__CGSCC____:       onone:
+; IS__CGSCC____-NEXT:    unreachable
+; IS__CGSCC____:       ondefault:
+; IS__CGSCC____-NEXT:    unreachable
+;
+  switch i32 %c, label %ondefault [ i32 0, label %onzero
+  i32 1, label %onone ]
+onzero:
+  %ptr = alloca i32
+  ret i32* %ptr
+onone:
+  ret i32* null
+ondefault:
+  ret i32* undef
+}