[clang] Reapply [Clang] Mark new as inaccessiblememonly if sane (PR #217652)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Nikita Popov (nikic)

<details>
<summary>Changes</summary>

Reapply https://github.com/llvm/llvm-project/pull/201375 without changes after https://github.com/llvm/llvm-project/pull/201375 improved our errno TBAA reasoning.

-----

If `-fassume-sane-operator-new` (the default), assume that `operator new` does not read or write accessible memory.

Currently, this assumption already exists due to special treatment in BasicAA. I'd like to remove this special treatment (see https://github.com/llvm/llvm-project/pull/197180), and instead rely only on the `memory` attribute.

It's worth noting that this is consistent with GCC's interpretation of the flag (where it is also enabled by default):

> [...] With -fassume-sane-operators-new-delete option GCC may assume
that calls to the replaceable global operators from new or delete expressions or from __builtin_operator_new or __builtin_operator_delete calls don’t read or modify any global variables or variables whose address could escape to the operators (global state; except for errno for the new and new[] operators). [...]

---
Full diff: https://github.com/llvm/llvm-project/pull/217652.diff


3 Files Affected:

- (modified) clang/lib/CodeGen/CGCall.cpp (+9-3) 
- (modified) clang/test/CodeGenCXX/new_hot_cold.cpp (+4-4) 
- (modified) clang/test/CodeGenCXX/operator-new.cpp (+4-2) 


``````````diff
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 872229cd30219..8e47734f97560 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2792,11 +2792,17 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
       AddAttributesFromFunctionProtoType(
           getContext(), FuncAttrs, Fn->getType()->getAs<FunctionProtoType>());
       if (AttrOnCallSite && Fn->isReplaceableGlobalAllocationFunction()) {
-        // A sane operator new returns a non-aliasing pointer.
-        auto Kind = Fn->getDeclName().getCXXOverloadedOperator();
+        // A sane operator new returns a non-aliasing pointer and does not
+        // read or write accessible memory.
         if (getCodeGenOpts().AssumeSaneOperatorNew &&
-            (Kind == OO_New || Kind == OO_Array_New))
+            Fn->getDeclName().isAnyOperatorNew()) {
           RetAttrs.addAttribute(llvm::Attribute::NoAlias);
+          // FIXME: inaccessiblemem could cause issues if LTO makes the
+          // previously inaccessible memory accessible after linking.
+          FuncAttrs.addMemoryAttr(
+              llvm::MemoryEffects::inaccessibleOrErrnoMemOnly(
+                  llvm::ModRefInfo::ModRef, llvm::ModRefInfo::Mod));
+        }
       }
       const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Fn);
       const bool IsVirtualCall = MD && MD->isVirtual();
diff --git a/clang/test/CodeGenCXX/new_hot_cold.cpp b/clang/test/CodeGenCXX/new_hot_cold.cpp
index 014e815201485..439166ddea566 100644
--- a/clang/test/CodeGenCXX/new_hot_cold.cpp
+++ b/clang/test/CodeGenCXX/new_hot_cold.cpp
@@ -124,7 +124,7 @@ void hot_cold_new_align_nothrow_array() {
 
 // CHECK-DAG: attributes [[ATTR_NOBUILTIN]] = { nobuiltin allocsize(0) {{.*}} }
 // CHECK-DAG: attributes [[ATTR_NOBUILTIN_NOTHROW]] = { nobuiltin nounwind allocsize(0) {{.*}} }
-// CHECK-DAG: attributes [[ATTR_NO_BUILTIN_CALL]] = { allocsize(0) }
-// CHECK-DAG: attributes [[ATTR_BUILTIN_CALL]] = { builtin allocsize(0) }
-// CHECK-DAG: attributes [[ATTR_NO_BUILTIN_NOTHROW_CALL]] = { nounwind allocsize(0) }
-// CHECK-DAG: attributes [[ATTR_BUILTIN_NOTHROW_CALL]] = { builtin nounwind allocsize(0) }
+// CHECK-DAG: attributes [[ATTR_NO_BUILTIN_CALL]] = { allocsize(0) memory(inaccessiblemem: readwrite, errnomem: write) }
+// CHECK-DAG: attributes [[ATTR_BUILTIN_CALL]] = { builtin allocsize(0) memory(inaccessiblemem: readwrite, errnomem: write) }
+// CHECK-DAG: attributes [[ATTR_NO_BUILTIN_NOTHROW_CALL]] = { nounwind allocsize(0) memory(inaccessiblemem: readwrite, errnomem: write) }
+// CHECK-DAG: attributes [[ATTR_BUILTIN_NOTHROW_CALL]] = { builtin nounwind allocsize(0) memory(inaccessiblemem: readwrite, errnomem: write) }
diff --git a/clang/test/CodeGenCXX/operator-new.cpp b/clang/test/CodeGenCXX/operator-new.cpp
index 1d7887146705e..e741d85fe4cd2 100644
--- a/clang/test/CodeGenCXX/operator-new.cpp
+++ b/clang/test/CodeGenCXX/operator-new.cpp
@@ -22,8 +22,10 @@ void *f2(long N) {
   // ALL-NEXT: [[OVER:%.*]] = extractvalue {{.*}} [[UWO]], 1
   // ALL-NEXT: [[SUM:%.*]] = extractvalue {{.*}} [[UWO]], 0
   // ALL-NEXT: [[RESULT:%.*]] = select i1 [[OVER]], i32 -1, i32 [[SUM]]
-  // SANE-NEXT: call noalias noundef nonnull ptr @_Znaj(i32 noundef [[RESULT]])
-  // SANENOT-NEXT: call noundef nonnull ptr @_Znaj(i32 noundef [[RESULT]])
+  // SANE-NEXT: call noalias noundef nonnull ptr @_Znaj(i32 noundef [[RESULT]]) [[ATTR:#[0-9]+]]
+  // SANENOT-NEXT: call noundef nonnull ptr @_Znaj(i32 noundef [[RESULT]]) [[ATTR:#[0-9]+]]
 }
 
 // ALL: declare noundef nonnull ptr @_Znaj(
+// SANE: attributes [[ATTR]] = { builtin allocsize(0) memory(inaccessiblemem: readwrite, errnomem: write) }
+// SANENOT: attributes [[ATTR]] = { builtin allocsize(0) }

``````````

</details>


https://github.com/llvm/llvm-project/pull/217652