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 =
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 =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
