[llvm-branch-commits] [clang] 1644103 - [CodeGen] -fno-delete-null-pointer-checks: change dereferenceable to dereferenceable_or_null

[Fangrui Song via llvm-branch-commits]

Author: Fangrui Song
Date: 2020-11-30T12:44:35-08:00
New Revision: 164410324d8bf3b5a99e39f7dfe3c6d6972dab30

URL: https://github.com/llvm/llvm-project/commit/164410324d8bf3b5a99e39f7dfe3c6d6972dab30
DIFF: https://github.com/llvm/llvm-project/commit/164410324d8bf3b5a99e39f7dfe3c6d6972dab30.diff

LOG: [CodeGen] -fno-delete-null-pointer-checks: change dereferenceable to dereferenceable_or_null

After D17993, with -fno-delete-null-pointer-checks we add the dereferenceable attribute to the `this` pointer.

We have observed that one internal target which worked before fails even with -fno-delete-null-pointer-checks.
Switching to dereferenceable_or_null fixes the problem.

dereferenceable currently does not always respect NullPointerIsValid and may
imply nonnull and lead to aggressive optimization. The optimization may be
related to `CallBase::isReturnNonNull`, `Argument::hasNonNullAttr`, or
`Value::getPointerDereferenceableBytes`. See D66664 and D66618 for some discussions.

Reviewed By: bkramer, rsmith

Differential Revision: https://reviews.llvm.org/D92297

Added: 
    

Modified: 
    clang/lib/CodeGen/CGCall.cpp
    clang/test/CodeGenCXX/this-nonnull.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 78740018d416..2b9bfb6a6c88 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2168,13 +2168,21 @@ void CodeGenModule::ConstructAttributeList(
     if (!CodeGenOpts.NullPointerIsValid &&
         getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) {
       Attrs.addAttribute(llvm::Attribute::NonNull);
+      Attrs.addDereferenceableAttr(
+          getMinimumObjectSize(
+              FI.arg_begin()->type.castAs<PointerType>()->getPointeeType())
+              .getQuantity());
+    } else {
+      // FIXME dereferenceable should be correct here, regardless of
+      // NullPointerIsValid. However, dereferenceable currently does not always
+      // respect NullPointerIsValid and may imply nonnull and break the program.
+      // See https://reviews.llvm.org/D66618 for discussions.
+      Attrs.addDereferenceableOrNullAttr(
+          getMinimumObjectSize(
+              FI.arg_begin()->type.castAs<PointerType>()->getPointeeType())
+              .getQuantity());
     }
 
-    Attrs.addDereferenceableAttr(
-        getMinimumObjectSize(
-            FI.arg_begin()->type.castAs<PointerType>()->getPointeeType())
-            .getQuantity());
-
     ArgAttrs[IRArgs.first] = llvm::AttributeSet::get(getLLVMContext(), Attrs);
   }
 

diff  --git a/clang/test/CodeGenCXX/this-nonnull.cpp b/clang/test/CodeGenCXX/this-nonnull.cpp
index 9b074e2bdda1..63873388b070 100644
--- a/clang/test/CodeGenCXX/this-nonnull.cpp
+++ b/clang/test/CodeGenCXX/this-nonnull.cpp
@@ -12,8 +12,9 @@ void TestReturnsVoid(Struct &s) {
   s.ReturnsVoid();
 
   // CHECK-YES: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull dereferenceable(12) %0)
-  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable(12) %0)
+  /// FIXME Use dereferenceable after dereferenceable respects NullPointerIsValid.
+  // CHECK-NO: call void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable_or_null(12) %0)
 }
 
 // CHECK-YES: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* nonnull dereferenceable(12))
-// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable(12))
+// CHECK-NO: declare void @_ZN6Struct11ReturnsVoidEv(%struct.Struct* dereferenceable_or_null(12))