From adc5107b734e877e646b0c557ca611c12fd2ffc9 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Sun, 25 Jul 2021 18:04:50 +0200
Subject: [PATCH] [Attributes] Remove nonnull from UB-implying attributes

From LangRef:

> if the parameter or return pointer is null, poison value is
> returned or passed instead. The nonnull attribute should be
> combined with the noundef attribute to ensure a pointer is not
> null or otherwise the behavior is undefined.

Dropping noundef is sufficient to prevent UB. Including nonnull
in this method just muddies the semantics.
---
 lib/IR/Attributes.cpp                         |  1 -
 test/Transforms/DeadArgElim/NoundefAttrs.ll   | 10 +++++-----
 test/Transforms/InstCombine/unused-nonnull.ll |  4 ++--
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/IR/Attributes.cpp b/lib/IR/Attributes.cpp
index fc05f52ee74..92721c74317 100644
--- a/lib/IR/Attributes.cpp
+++ b/lib/IR/Attributes.cpp
@@ -1340,7 +1340,6 @@ AttributeList::removeParamUndefImplyingAttributes(LLVMContext &C,
                                                   unsigned ArgNo) const {
   AttrBuilder B;
   B.addAttribute(Attribute::NoUndef);
-  B.addAttribute(Attribute::NonNull);
   B.addDereferenceableAttr(1);
   B.addDereferenceableOrNullAttr(1);
   return removeParamAttributes(C, ArgNo, B);
diff --git a/test/Transforms/DeadArgElim/NoundefAttrs.ll b/test/Transforms/DeadArgElim/NoundefAttrs.ll
index 576f972fc87..155974cbf1a 100644
--- a/test/Transforms/DeadArgElim/NoundefAttrs.ll
+++ b/test/Transforms/DeadArgElim/NoundefAttrs.ll
@@ -1,10 +1,10 @@
 ; RUN: opt -deadargelim -S < %s | FileCheck %s
 
-; If caller is changed to pass in undef, noundef and related attributes
-; should be deleted.
+; If caller is changed to pass in undef, noundef, dereferenceable and other
+; attributes that imply immediate undefined behavior should be delete.
+; Other attributes like nonnull, which only imply poison, can be safely kept.
 
-
-; CHECK:   define i64 @bar(i64* %0, i64 %1)
+; CHECK:   define i64 @bar(i64* nonnull %0, i64 %1)
 define i64 @bar(i64* nonnull dereferenceable(8) %0, i64 %1) {
 entry:
   %2 = add i64 %1, 8
@@ -12,7 +12,7 @@ entry:
 }
 
 define i64 @foo(i64* %p, i64 %v) {
-; CHECK:   %retval = call i64 @bar(i64* undef, i64 %v)
+; CHECK:   %retval = call i64 @bar(i64* nonnull undef, i64 %v)
   %retval = call i64 @bar(i64* nonnull dereferenceable(8) %p, i64 %v)
   ret i64 %retval
 }
diff --git a/test/Transforms/InstCombine/unused-nonnull.ll b/test/Transforms/InstCombine/unused-nonnull.ll
index 0f6ef729998..f29485218ab 100644
--- a/test/Transforms/InstCombine/unused-nonnull.ll
+++ b/test/Transforms/InstCombine/unused-nonnull.ll
@@ -9,7 +9,7 @@ target triple = "x86_64-unknown-linux-gnu"
 
 define i32 @main(i32 %argc, i8** %argv) #0 {
 ; CHECK-LABEL: define {{[^@]+}}@main
-; CHECK-SAME: (i32 [[ARGC:%.*]], i8** nocapture readnone [[ARGV:%.*]]) local_unnamed_addr #0
+; CHECK-SAME: (i32 [[ARGC:%.*]], i8** nocapture readnone [[ARGV:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = icmp slt i32 [[ARGC]], 2
 ; CHECK-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[TMP0]], i32 0, i32 [[ARGC]]
@@ -37,7 +37,7 @@ done:
 
 define i32 @compute(i8* noundef nonnull %ptr, i32 %x) #1 {
 ; CHECK-LABEL: define {{[^@]+}}@compute
-; CHECK-SAME: (i8* nocapture readnone [[PTR:%.*]], i32 returned [[X:%.*]]) local_unnamed_addr #1
+; CHECK-SAME: (i8* nocapture nonnull readnone [[PTR:%.*]], i32 returned [[X:%.*]]) local_unnamed_addr #[[ATTR1:[0-9]+]] {
 ; CHECK-NEXT:    ret i32 [[X]]
 ;
   ret i32 %x