[PATCH] D105728: [clang][Codegen] Directly lower `(*((volatile int *)(0))) = 0;` into a `call void @llvm.trap()`

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 357648.
lebedev.ri marked 2 inline comments as done.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

In D105728#2868281 <https://reviews.llvm.org/D105728#2868281>, @erichkeane wrote:

> This seems logical to me.  The comment needs some love, so I made my best suggestion.

Thank you for taking a look!
Adjusted comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105728

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/please-crash-now.c


Index: clang/test/CodeGen/please-crash-now.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/please-crash-now.c
@@ -0,0 +1,62 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -x c   %s -triple x86_64-linux-gnu -emit-llvm -o -                                 | FileCheck %s --check-prefixes=ALL,WITHOUT_NULL -implicit-check-not="call void @llvm.trap()"
+// RUN: %clang_cc1 -x c++ %s -triple x86_64-linux-gnu -emit-llvm -o -                                 | FileCheck %s --check-prefixes=ALL,WITHOUT_NULL -implicit-check-not="call void @llvm.trap()"
+// RUN: %clang_cc1 -x c   %s -triple x86_64-linux-gnu -emit-llvm -o - -fno-delete-null-pointer-checks | FileCheck %s --check-prefixes=ALL              -implicit-check-not="call void @llvm.trap()"
+// RUN: %clang_cc1 -x c++ %s -triple x86_64-linux-gnu -emit-llvm -o - -fno-delete-null-pointer-checks | FileCheck %s --check-prefixes=ALL              -implicit-check-not="call void @llvm.trap()"
+
+// ALL-LABEL: define {{.*}}store_into_volatile_nullptr_via_dereference
+// WITHOUT_NULL: call void @llvm.trap()
+void store_into_volatile_nullptr_via_dereference() {
+  (*((volatile int *)(0))) = 0;
+}
+
+// Not the pattern, must be a simple dereference.
+// ALL-LABEL: define {{.*}}store_into_volatile_nullptr_via_subscript_operator
+// WITHOUT_NULL: call void @llvm.trap()
+void store_into_volatile_nullptr_via_subscript_operator() {
+  ((volatile int *)(0))[0] = 0;
+}
+
+// For the rest of the tests, we don't emit trap.
+
+// Just dereferencing is not enough.
+// ALL-LABEL: define {{.*}}n0
+void n0() {
+  *((int *)(0));
+}
+
+// Just dereferencing via subscript expression is not enough either.
+// ALL-LABEL: define {{.*}}n1
+void n1() {
+  ((int *)(0))[0];
+}
+
+// Just dereferencing of a volatile pointer is still not enough.
+// ALL-LABEL: define {{.*}}n2
+void n2() {
+  *((volatile int *)(0));
+}
+
+// Just dereferencing of a volatile pointer via subscript expression is not enough either.
+// ALL-LABEL: define {{.*}}n3
+void n3() {
+  ((volatile int *)(0))[0];
+}
+
+// Almost the pattern, but the pointer is not volatile.
+// ALL-LABEL: define {{.*}}n4
+void n4() {
+  (*((int *)(0))) = 0;
+}
+
+// Pointer subscript is not the pattern we are looking for.
+// ALL-LABEL: define {{.*}}n5
+void n5() {
+  ((int *)(0))[0] = 0;
+}
+
+// A pointer with integral value of `0` is valid in this address space.
+// ALL-LABEL: define {{.*}}n6
+void n6() {
+  (*((__attribute__((address_space(42))) volatile int *)(0))) = 0;
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1856,6 +1856,19 @@
     return;
   }
 
+  if (Volatile && isa<llvm::ConstantPointerNull>(Addr.getPointer()) &&
+      !llvm::NullPointerIsDefined(CurFn, Addr.getAddressSpace())) {
+    // If this is a simple volatile store to a null pointer
+    // (and said null pointer is not defined for this address space),
+    // then we have a common idiom "please crash now".
+    // However, that is not the semantics LLVM IR provides. Volatile operations
+    // are not defined as trapping in LLVM IR, so this store will be silently
+    // dropped by the optimization passes, and no trap will happen.
+    // Since this is a common idiom, for now, directly codegen it as trap.
+    EmitTrapCall(llvm::Intrinsic::trap);
+    return;
+  }
+
   llvm::StoreInst *Store = Builder.CreateStore(Value, Addr, Volatile);
   if (isNontemporal) {
     llvm::MDNode *Node =


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D105728.357648.patch
Type: text/x-patch
Size: 3622 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210709/e78ae2b4/attachment-0001.bin>