[PATCH] D78134: [Sema] Don't apply an lvalue-to-rvalue conversion to a discarded-value expression if it has an array type

[Akira Hatanaka via Phabricator via cfe-commits]

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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78134.257402.patch
Type: text/x-patch
Size: 2423 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200414/ba33c490/attachment.bin>