ahatanak created this revision.
ahatanak added reviewers: efriedma, rsmith, rjmccall.
ahatanak added a project: clang.
Herald added subscribers: ributzka, dexonsmith, jkorous.
This fixes a bug which causes clang to emit incorrect IR for the following code
in C++:
int foo(void) {
unsigned a = get_size();
volatile char b[a];
b;
return b[0];
}
The code crashes because the destination of the memcpy that is generated to do
the lvalue-to-rvalue conversion of discarded-value expression `b` is incorrect.
To fix this bug, I've added a check to Sema which prevents an lvalue cast
expression to be emitted if the expression has an array type. My understanding
is that lvalue-to-rvalue conversion isn't applied to array type expressions, so
I think we should detect this in Sema rather than fixing the code in IRGen that
emits the lvalue for the destination of the lvalue-to-rvalue conversion. I've
also made changes to `Sema::DiagnoseUnusedExprResult` so that clang doesn't
tell users to assign a reference to a volatile array to a variable to force its
value to be loaded.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78134
Files:
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/CXX/expr/p10-0x.cpp
clang/test/SemaCXX/warn-unused-value-cxx11.cpp
Index: clang/test/SemaCXX/warn-unused-value-cxx11.cpp
===================================================================
--- clang/test/SemaCXX/warn-unused-value-cxx11.cpp
+++ clang/test/SemaCXX/warn-unused-value-cxx11.cpp
@@ -41,4 +41,13 @@
(void)noexcept(s.g() = 5); // Ok
}
-}
\ No newline at end of file
+}
+
+namespace volatile_array {
+void test() {
+ char a[10];
+ volatile char b[10];
+ a; // expected-warning-re {{expression result unused{{$}}}}
+ b; // expected-warning-re {{expression result unused{{$}}}}
+}
+}
Index: clang/test/CXX/expr/p10-0x.cpp
===================================================================
--- clang/test/CXX/expr/p10-0x.cpp
+++ clang/test/CXX/expr/p10-0x.cpp
@@ -44,3 +44,12 @@
refcall();
1 ? refcall() : *x;
}
+
+// CHECK: define void @_Z2f3v()
+// CHECK-NOT: load
+// CHECK-NOT: memcpy
+
+void f3(void) {
+ volatile char a[10];
+ a;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -370,7 +370,10 @@
}
}
- if (E->isGLValue() && E->getType().isVolatileQualified()) {
+ // Tell the user to assign it into a variable to force a volatile load if
this
+ // isn't an array.
+ if (E->isGLValue() && E->getType().isVolatileQualified() &&
+ !E->getType()->isArrayType()) {
Diag(Loc, diag::warn_unused_volatile) << R1 << R2;
return;
}
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7756,11 +7756,15 @@
// conversion.
if (getLangOpts().CPlusPlus11 && E->isGLValue() &&
E->getType().isVolatileQualified()) {
- if (IsSpecialDiscardedValue(E)) {
- ExprResult Res = DefaultLvalueConversion(E);
- if (Res.isInvalid())
- return E;
- E = Res.get();
+ if (IsSpecialDiscardedValue(E)) {
+ // Don't try to apply lvalue-to-rvalue conversion to expressions of
+ // array types.
+ if (!E->getType()->isArrayType()) {
+ ExprResult Res = DefaultLvalueConversion(E);
+ if (Res.isInvalid())
+ return E;
+ E = Res.get();
+ }
} else {
// Per C++2a [expr.ass]p5, a volatile assignment is not deprecated if
// it occurs as a discarded-value expression.
Index: clang/test/SemaCXX/warn-unused-value-cxx11.cpp
===================================================================
--- clang/test/SemaCXX/warn-unused-value-cxx11.cpp
+++ clang/test/SemaCXX/warn-unused-value-cxx11.cpp
@@ -41,4 +41,13 @@
(void)noexcept(s.g() = 5); // Ok
}
-}
\ No newline at end of file
+}
+
+namespace volatile_array {
+void test() {
+ char a[10];
+ volatile char b[10];
+ a; // expected-warning-re {{expression result unused{{$}}}}
+ b; // expected-warning-re {{expression result unused{{$}}}}
+}
+}
Index: clang/test/CXX/expr/p10-0x.cpp
===================================================================
--- clang/test/CXX/expr/p10-0x.cpp
+++ clang/test/CXX/expr/p10-0x.cpp
@@ -44,3 +44,12 @@
refcall();
1 ? refcall() : *x;
}
+
+// CHECK: define void @_Z2f3v()
+// CHECK-NOT: load
+// CHECK-NOT: memcpy
+
+void f3(void) {
+ volatile char a[10];
+ a;
+}
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -370,7 +370,10 @@
}
}
- if (E->isGLValue() && E->getType().isVolatileQualified()) {
+ // Tell the user to assign it into a variable to force a volatile load if this
+ // isn't an array.
+ if (E->isGLValue() && E->getType().isVolatileQualified() &&
+ !E->getType()->isArrayType()) {
Diag(Loc, diag::warn_unused_volatile) << R1 << R2;
return;
}
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7756,11 +7756,15 @@
// conversion.
if (getLangOpts().CPlusPlus11 && E->isGLValue() &&
E->getType().isVolatileQualified()) {
- if (IsSpecialDiscardedValue(E)) {
- ExprResult Res = DefaultLvalueConversion(E);
- if (Res.isInvalid())
- return E;
- E = Res.get();
+ if (IsSpecialDiscardedValue(E)) {
+ // Don't try to apply lvalue-to-rvalue conversion to expressions of
+ // array types.
+ if (!E->getType()->isArrayType()) {
+ ExprResult Res = DefaultLvalueConversion(E);
+ if (Res.isInvalid())
+ return E;
+ E = Res.get();
+ }
} else {
// Per C++2a [expr.ass]p5, a volatile assignment is not deprecated if
// it occurs as a discarded-value expression.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
