[Attributor] Check nonnull attribute violation in AAUndefinedBehavior

[okura]

Summary

This patch makes it possible to handle nonnull attribute violation at callsites in AAUndefinedBehavior.
If null pointer is passed to callee at a callsite and the corresponding argument of callee has nonnull attribute, the behavior of the callee is undefined.
In this patch, violations of argument nonnull attributes is only handled.
But violations of returned nonnull attributes can be handled and I will implement that in a follow-up patch.

[baziotis]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2024

Please add a third argument specifying the instructions to check, i.e. {Instruction::CallSite}. Also, a fourth argument true as a value, like the other too above.

llvm/test/Transforms/Attributor/value-simplify.ll

409

Hmm.. We probably need to fix the undefined behavior in this testcase.

[jdoerfert]

This is cool :)

I thought about this again and I think it will expose the problematic case I mentioned on the call. The Attributor derives attributes, then replaces the value with undef because it is dead, then the thing might be folded to UB/unreachable. Let's directly require the noundef attribute as well. We need to split the AAUB behavior accordingly soon but I'm fine with checking it explicitly here first. That will require tests with and without noundef.

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

1988

1993

You don't need a callee, just use the call base.

1995

I somehow feel checking the attribute first is better.

2002

Move this to the beginning to describe what is happening. Maybe elaborate a bit.

2015

Provide a comment before or at the beginning of this lambda explaining it a bit.

2024

No need to specify call site (which is not an opcode) because this is a specialized version of checkForAllInstructions. The CheckBBLivenessOnly can be provided I guess.

[baziotis]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

2024

Oh my bad. The names are big and I missed completely that it is not checkForAllInstructions.

[okura]

• fix comments
• check noundef attribute explicitly
• modify and add tests with noundef attribute

[okura]

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

1993

Do you mean that I don't have to be aware of Callee->arg_size() anywhere?
In the case of callbacks, CB.getNumArgOperands() > Callee->arg_size() holds and I got segfault when getArg is called.
Can I get the callee argument position directly from CallBase?

[jdoerfert]

LGTM. Some minor nits below. I guess this doesn't affect other tests because of the noundef restriction. We should really derive that one as well.

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

1993

Oh, OK. For any variadic function the call can have more operands than the callee arguments. I guess what you really want are the abstract call sites you can get from the call base. Let's keep it like this for now.

2007

I guess the easiest is to check if it is a pointer early on. That should avoid running the code below in case it can never trigger.

2021

Not having a value and having the value undef are also all violations if you will. The former means it is dead anyway, and undef can degenerate to null whenever you want it to.

llvm/test/Transforms/Attributor/undefined_behavior.ll

598

Next up, we can derive noundef for this case (amongst others), given that we actually access the pointer for sure. An undef value would cause UB so %a cannot be undef. Basically the same way we should already derive nonnull for this case. Later ;)

[okura]

• fix nits
• add comment

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

auto InspectBrInstForUB = [&](Instruction &I) {
       Optional<Value *> SimplifiedCond =
           stopOnUndefOrAssumed(A, BrInst->getCondition(), BrInst);
       if (!SimplifiedCond.hasValue())
         return true;
       AssumedNoUBInsts.insert(&I);
       return true;
     };
 
+    auto InspectCallSiteForUB = [&](Instruction &I) {
+      // Skip instructions that are already saved.
+      if (AssumedNoUBInsts.count(&I) || KnownUBInsts.count(&I))
+        return true;
+      // Check nonnull argument attribute violation for each callsite.
+      CallBase *CB = cast<CallBase>(&I);
+      const Function *Callee = CB->getCalledFunction();
+      if (!Callee)
+        return true;
+      unsigned ArgNum =
+          std::min(CB->getNumArgOperands(), (unsigned)Callee->arg_size());
+      for (unsigned idx = 0; idx < ArgNum; idx++) {
+        Value *ArgVal = CB->getArgOperand(idx);
+        const auto &ValueSimplifyAA =
+            A.getAAFor<AAValueSimplify>(*this, IRPosition::value(*ArgVal));
+        Optional<Value *> SimplifiedVal =
+            ValueSimplifyAA.getAssumedSimplifiedValue(A);
+
+        // If current argument is known to be simplified to null pointer,
+        // this callsite is considered UB.
+        if (!ValueSimplifyAA.isKnown() || !SimplifiedVal.hasValue())
+          continue;
+        if (!isa<ConstantPointerNull>(*SimplifiedVal.getValue()))
+          continue;
+        auto &NonNullAA = A.getAAFor<AANonNull>(
+            *this, IRPosition::argument(*Callee->getArg(idx)));
+        if (NonNullAA.isKnownNonNull()) {
+          KnownUBInsts.insert(&I);
+          return true;
+        }
+      }
+      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 (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.
+
+define void @arg_nonnull(i32* nonnull %a) {
+; IS__TUNIT____: Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
+; IS__TUNIT____-LABEL: define {{[^@]+}}@arg_nonnull
+; 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
+;
+; IS__CGSCC____: Function Attrs: argmemonly nofree norecurse nosync nounwind willreturn writeonly
+; IS__CGSCC____-LABEL: define {{[^@]+}}@arg_nonnull
+; IS__CGSCC____-SAME: (i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[A:%.*]])
+; IS__CGSCC____-NEXT:    store i32 0, i32* [[A]], align 4
+; IS__CGSCC____-NEXT:    ret void
+;
+  store i32 0, i32* %a
+  ret void
+}
+
+define void @arg_nonnull2(i32* nonnull %a, i32* nonnull %b, i32* %c) {
+; IS__TUNIT____: Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
+; IS__TUNIT____-LABEL: define {{[^@]+}}@arg_nonnull2
+; IS__TUNIT____-SAME: (i32* nocapture nofree nonnull writeonly [[A:%.*]], i32* nocapture nofree nonnull writeonly [[B:%.*]], i32* nofree writeonly [[C:%.*]])
+; IS__TUNIT____-NEXT:    [[D:%.*]] = icmp eq i32* [[C]], null
+; IS__TUNIT____-NEXT:    br i1 [[D]], label [[T:%.*]], label [[F:%.*]]
+; IS__TUNIT____:       t:
+; IS__TUNIT____-NEXT:    store i32 0, i32* [[A]], align 4
+; IS__TUNIT____-NEXT:    br label [[RET:%.*]]
+; IS__TUNIT____:       f:
+; IS__TUNIT____-NEXT:    store i32 1, i32* [[B]], align 4
+; IS__TUNIT____-NEXT:    br label [[RET]]
+; IS__TUNIT____:       ret:
+; IS__TUNIT____-NEXT:    ret void
+;
+; IS__CGSCC____: Function Attrs: argmemonly nofree norecurse nosync nounwind willreturn writeonly
+; IS__CGSCC____-LABEL: define {{[^@]+}}@arg_nonnull2
+; IS__CGSCC____-SAME: (i32* nocapture nofree nonnull writeonly [[A:%.*]], i32* nocapture nofree nonnull writeonly [[B:%.*]], i32* nofree writeonly [[C:%.*]])
+; IS__CGSCC____-NEXT:    [[D:%.*]] = icmp eq i32* [[C]], null
+; IS__CGSCC____-NEXT:    br i1 [[D]], label [[T:%.*]], label [[F:%.*]]
+; IS__CGSCC____:       t:
+; IS__CGSCC____-NEXT:    store i32 0, i32* [[A]], align 4
+; IS__CGSCC____-NEXT:    br label [[RET:%.*]]
+; IS__CGSCC____:       f:
+; IS__CGSCC____-NEXT:    store i32 1, i32* [[B]], align 4
+; IS__CGSCC____-NEXT:    br label [[RET]]
+; IS__CGSCC____:       ret:
+; IS__CGSCC____-NEXT:    ret void
+;
+  %d = icmp eq i32* %c, null
+  br i1 %d, label %t, label %f
+t:
+  store i32 0, i32* %a
+  br label %ret
+f:
+  store i32 1, i32* %b
+  br label %ret
+ret:
+  ret void
+}
+
+; Pass null directly to argument with nonnull attribute
+define void @arg_nonnull_violation1() {
+; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
+; IS__TUNIT____-LABEL: define {{[^@]+}}@arg_nonnull_violation1()
+; IS__TUNIT____-NEXT:    unreachable
+;
+; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
+; IS__CGSCC____-LABEL: define {{[^@]+}}@arg_nonnull_violation1()
+; IS__CGSCC____-NEXT:    unreachable
+;
+  call void @arg_nonnull(i32* null)
+  ret void
+}
+
+; A case that depends on value simplification
+define void @arg_nonnull_violation2(i1 %c) {
+; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
+; IS__TUNIT____-LABEL: define {{[^@]+}}@arg_nonnull_violation2
+; IS__TUNIT____-SAME: (i1 [[C:%.*]])
+; IS__TUNIT____-NEXT:    unreachable
+;
+; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
+; IS__CGSCC____-LABEL: define {{[^@]+}}@arg_nonnull_violation2
+; IS__CGSCC____-SAME: (i1 [[C:%.*]])
+; IS__CGSCC____-NEXT:    unreachable
+;
+  %null = getelementptr i32, i32* null, i32 0
+  %mustnull = select i1 %c, i32* null, i32* %null
+  call void @arg_nonnull(i32* %mustnull)
+  ret void
+}
+
+; Cases for single and multiple violation at a callsite
+define void @arg_nonnull_violation3(i1 %c) {
+; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
+; IS__TUNIT____-LABEL: define {{[^@]+}}@arg_nonnull_violation3
+; IS__TUNIT____-SAME: (i1 [[C:%.*]])
+; IS__TUNIT____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__TUNIT____-NEXT:    br i1 [[C]], label [[T:%.*]], label [[F:%.*]]
+; IS__TUNIT____:       t:
+; IS__TUNIT____-NEXT:    call void @arg_nonnull2(i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* noalias nocapture nofree writeonly align 536870912 null)
+; IS__TUNIT____-NEXT:    unreachable
+; IS__TUNIT____:       f:
+; IS__TUNIT____-NEXT:    call void @arg_nonnull2(i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]])
+; IS__TUNIT____-NEXT:    unreachable
+; IS__TUNIT____:       ret:
+; IS__TUNIT____-NEXT:    ret void
+;
+; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
+; IS__CGSCC____-LABEL: define {{[^@]+}}@arg_nonnull_violation3
+; IS__CGSCC____-SAME: (i1 [[C:%.*]])
+; IS__CGSCC____-NEXT:    [[PTR:%.*]] = alloca i32, align 4
+; IS__CGSCC____-NEXT:    br i1 [[C]], label [[T:%.*]], label [[F:%.*]]
+; IS__CGSCC____:       t:
+; IS__CGSCC____-NEXT:    call void @arg_nonnull2(i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* noalias nocapture nofree writeonly align 536870912 null)
+; IS__CGSCC____-NEXT:    unreachable
+; IS__CGSCC____:       f:
+; IS__CGSCC____-NEXT:    call void @arg_nonnull2(i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* nocapture nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]], i32* nofree nonnull writeonly align 4 dereferenceable(4) [[PTR]])
+; IS__CGSCC____-NEXT:    unreachable
+; IS__CGSCC____:       ret:
+; IS__CGSCC____-NEXT:    ret void
+;
+  %ptr = alloca i32
+  br i1 %c, label %t, label %f
+t:
+  call void @arg_nonnull2(i32* %ptr, i32* %ptr, i32* null)
+  call void @arg_nonnull2(i32* null, i32* %ptr, i32* %ptr)
+  br label %ret
+f:
+  call void @arg_nonnull2(i32* %ptr, i32* %ptr, i32* %ptr)
+  call void @arg_nonnull2(i32* null, i32* null, i32* %ptr)
+  br label %ret
+ret:
+  ret void
+}

llvm/test/Transforms/Attributor/value-simplify.ll

 ;
   %c = call token @llvm.call.preallocated.setup(i32 1)
   %call = call i32* @test_preallocated(i32* preallocated(i32) null) ["preallocated"(token %c)]
   ret i32* %call
 }
 
 define internal void @test_sret(%struct.X* sret %a, %struct.X** %b) {
 ;
-; IS__TUNIT____: Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
-; IS__TUNIT____-LABEL: define {{[^@]+}}@test_sret
-; IS__TUNIT____-SAME: (%struct.X* noalias nofree nonnull sret writeonly align 536870912 dereferenceable(8) [[A:%.*]], %struct.X** nocapture nofree nonnull writeonly align 8 dereferenceable(8) [[B:%.*]])
-; IS__TUNIT____-NEXT:    store %struct.X* [[A]], %struct.X** [[B]], align 8
-; IS__TUNIT____-NEXT:    ret void
-;
 ; IS__CGSCC____: Function Attrs: argmemonly nofree norecurse nosync nounwind willreturn writeonly
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@test_sret
 ; IS__CGSCC____-SAME: (%struct.X* noalias nofree nonnull sret writeonly align 536870912 dereferenceable(8) [[A:%.*]], %struct.X** nocapture nofree nonnull writeonly align 8 dereferenceable(8) [[B:%.*]])
 ; IS__CGSCC____-NEXT:    store %struct.X* [[A]], %struct.X** [[B]], align 8
 ; IS__CGSCC____-NEXT:    ret void
 ;
   store %struct.X* %a, %struct.X** %b
   ret void
 }
 ; FIXME: Alignment and dereferenceability are not propagated to the argument
 define void @complicated_args_sret(%struct.X** %b) {
 ;
 ; IS__TUNIT____: Function Attrs: argmemonly nofree nosync nounwind willreturn writeonly
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@complicated_args_sret
 ; IS__TUNIT____-SAME: (%struct.X** nocapture nofree writeonly [[B:%.*]])
-; IS__TUNIT____-NEXT:    call void @test_sret(%struct.X* noalias nocapture nofree writeonly align 536870912 null, %struct.X** nocapture nofree writeonly align 8 [[B]])
-; IS__TUNIT____-NEXT:    ret void
+; IS__TUNIT____-NEXT:    unreachable
 ;
 ; IS__CGSCC____: Function Attrs: argmemonly nofree norecurse nosync nounwind willreturn writeonly
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@complicated_args_sret
 ; IS__CGSCC____-SAME: (%struct.X** nocapture nofree nonnull writeonly align 8 dereferenceable(8) [[B:%.*]])
-; IS__CGSCC____-NEXT:    call void @test_sret(%struct.X* noalias nocapture nofree nonnull writeonly align 536870912 dereferenceable(8) null, %struct.X** nocapture nofree nonnull writeonly align 8 dereferenceable(8) [[B]])
-; IS__CGSCC____-NEXT:    ret void
+; IS__CGSCC____-NEXT:    unreachable
 ;
   call void @test_sret(%struct.X* null, %struct.X** %b)
   ret void
 }
 
 define internal %struct.X* @test_nest(%struct.X* nest %a) {
 ; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@test_nest