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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 308454.
MaskRay marked an inline comment as done.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Fix a comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92297/new/

https://reviews.llvm.org/D92297

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


Index: clang/test/CodeGenCXX/this-nonnull.cpp
===================================================================
--- clang/test/CodeGenCXX/this-nonnull.cpp
+++ clang/test/CodeGenCXX/this-nonnull.cpp
@@ -12,8 +12,9 @@
   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))
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2168,13 +2168,21 @@
     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);
   }
 



Index: clang/test/CodeGenCXX/this-nonnull.cpp
===================================================================
--- clang/test/CodeGenCXX/this-nonnull.cpp
+++ clang/test/CodeGenCXX/this-nonnull.cpp
@@ -12,8 +12,9 @@
   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))
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2168,13 +2168,21 @@
     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);
   }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits