[AST, analyzer] Transform rvalue cast outputs to lvalues (fheinous-gnu-extensions)

Despite the fact that cast expressions return rvalues, GCC still
handles such outputs as lvalues when compiling inline assembler.
In this commit, we are treating it by removing LValueToRValue
casts inside GCCAsmStmt outputs.

Differential Revision: https://reviews.llvm.org/D45416

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@344864 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/SemaStmtAsm.cpp b/lib/Sema/SemaStmtAsm.cpp
index 359bfb44645272863bd7e620cdb6456cba6778ef..e6d9b34dee917601347e5349e9932e40fadfa79f 100644 (file)
--- a/lib/Sema/SemaStmtAsm.cpp
+++ b/lib/Sema/SemaStmtAsm.cpp
@@ -27,6 +27,58 @@
 using namespace clang;
 using namespace sema;
 
+/// Remove the upper-level LValueToRValue cast from an expression.
+static void removeLValueToRValueCast(Expr *E) {
+  Expr *Parent = E;
+  Expr *ExprUnderCast = nullptr;
+  SmallVector<Expr *, 8> ParentsToUpdate;
+
+  while (true) {
+    ParentsToUpdate.push_back(Parent);
+    if (auto *ParenE = dyn_cast<ParenExpr>(Parent)) {
+      Parent = ParenE->getSubExpr();
+      continue;
+    }
+
+    Expr *Child = nullptr;
+    CastExpr *ParentCast = dyn_cast<CastExpr>(Parent);
+    if (ParentCast)
+      Child = ParentCast->getSubExpr();
+    else
+      return;
+
+    if (auto *CastE = dyn_cast<CastExpr>(Child))
+      if (CastE->getCastKind() == CK_LValueToRValue) {
+        ExprUnderCast = CastE->getSubExpr();
+        // LValueToRValue cast inside GCCAsmStmt requires an explicit cast.
+        ParentCast->setSubExpr(ExprUnderCast);
+        break;
+      }
+    Parent = Child;
+  }
+
+  // Update parent expressions to have same ValueType as the underlying.
+  assert(ExprUnderCast &&
+         "Should be reachable only if LValueToRValue cast was found!");
+  auto ValueKind = ExprUnderCast->getValueKind();
+  for (Expr *E : ParentsToUpdate)
+    E->setValueKind(ValueKind);
+}
+
+/// Emit a warning about usage of "noop"-like casts for lvalues (GNU extension)
+/// and fix the argument with removing LValueToRValue cast from the expression.
+static void emitAndFixInvalidAsmCastLValue(const Expr *LVal, Expr *BadArgument,
+                                           Sema &S) {
+  if (!S.getLangOpts().HeinousExtensions) {
+    S.Diag(LVal->getBeginLoc(), diag::err_invalid_asm_cast_lvalue)
+        << BadArgument->getSourceRange();
+  } else {
+    S.Diag(LVal->getBeginLoc(), diag::warn_invalid_asm_cast_lvalue)
+        << BadArgument->getSourceRange();
+  }
+  removeLValueToRValueCast(BadArgument);
+}
+
 /// CheckAsmLValue - GNU C has an extremely ugly extension whereby they silently
 /// ignore "noop" casts in places where an lvalue is required by an inline asm.
 /// We emulate this behavior when -fheinous-gnu-extensions is specified, but
@@ -34,7 +86,7 @@ using namespace sema;
 ///
 /// This method checks to see if the argument is an acceptable l-value and
 /// returns false if it is a case we can handle.
-static bool CheckAsmLValue(const Expr *E, Sema &S) {
+static bool CheckAsmLValue(Expr *E, Sema &S) {
   // Type dependent expressions will be checked during instantiation.
   if (E->isTypeDependent())
     return false;
@@ -46,12 +98,7 @@ static bool CheckAsmLValue(const Expr *E, Sema &S) {
   // are supposed to allow.
   const Expr *E2 = E->IgnoreParenNoopCasts(S.Context);
   if (E != E2 && E2->isLValue()) {
-    if (!S.getLangOpts().HeinousExtensions)
-      S.Diag(E2->getBeginLoc(), diag::err_invalid_asm_cast_lvalue)
-          << E->getSourceRange();
-    else
-      S.Diag(E2->getBeginLoc(), diag::warn_invalid_asm_cast_lvalue)
-          << E->getSourceRange();
+    emitAndFixInvalidAsmCastLValue(E2, E, S);
     // Accept, even if we emitted an error diagnostic.
     return false;
   }
@@ -264,13 +311,7 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
       break;
     case Expr::MLV_LValueCast: {
       const Expr *LVal = OutputExpr->IgnoreParenNoopCasts(Context);
-      if (!getLangOpts().HeinousExtensions) {
-        Diag(LVal->getBeginLoc(), diag::err_invalid_asm_cast_lvalue)
-            << OutputExpr->getSourceRange();
-      } else {
-        Diag(LVal->getBeginLoc(), diag::warn_invalid_asm_cast_lvalue)
-            << OutputExpr->getSourceRange();
-      }
+      emitAndFixInvalidAsmCastLValue(LVal, OutputExpr, *this);
       // Accept, even if we emitted an error diagnostic.
       break;
     }

diff --git a/test/Analysis/asm.cpp b/test/Analysis/asm.cpp
new file mode 100644 (file)
index 0000000..1180063
--- /dev/null
+++ b/test/Analysis/asm.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker debug.ExprInspection -fheinous-gnu-extensions -w %s -verify
+
+int clang_analyzer_eval(int);
+
+int global;
+void testRValueOutput() {
+  int &ref = global;
+  ref = 1;
+  __asm__("" : "=r"(((int)(global))));  // don't crash on rvalue output operand
+  clang_analyzer_eval(global == 1); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(ref == 1);    // expected-warning{{UNKNOWN}}
+}

diff --git a/test/Analysis/cfg.cpp b/test/Analysis/cfg.cpp
index 85c5be5a9819fbe64c819c25e69e8348fac448a6..f43a809c77ef9c5cd47763a9d5166d5a9d976922 100644 (file)
--- a/test/Analysis/cfg.cpp
+++ b/test/Analysis/cfg.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -fheinous-gnu-extensions -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -fheinous-gnu-extensions -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
 
 // This file tests how we construct two different flavors of the Clang CFG -
@@ -84,6 +84,24 @@ void checkDeclStmts() {
   static_assert(1, "abc");
 }
 
+
+// CHECK-LABEL: void checkGCCAsmRValueOutput()
+// CHECK: [B2 (ENTRY)]
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B1]
+// CHECK-NEXT:   1: int arg
+// CHECK-NEXT:   2: arg
+// CHECK-NEXT:   3: (int)[B1.2] (CStyleCastExpr, NoOp, int)
+// CHECK-NEXT:   4: asm ("" : "=r" ([B1.3]));
+// CHECK-NEXT:   5: arg
+// CHECK-NEXT:   6: asm ("" : "=r" ([B1.5]));
+void checkGCCAsmRValueOutput() {
+  int arg;
+  __asm__("" : "=r"((int)arg));  // rvalue output operand
+  __asm__("" : "=r"(arg));       // lvalue output operand
+}
+
+
 // CHECK-LABEL: void F(EmptyE e)
 // CHECK: ENTRY
 // CHECK-NEXT: Succs (1): B1