From: Aleksei Sidorin <a.sidorin@samsung.com>
Date: Sat, 20 Oct 2018 22:49:23 +0000 (+0000)
Subject: [AST, analyzer] Transform rvalue cast outputs to lvalues (fheinous-gnu-extensions)
X-Git-Tag: llvmorg-8.0.0-rc1~6195
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=55365e4b39fd10bec58d7cb1c436af9767c8dad2;p=platform%2Fupstream%2Fllvm.git

[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

llvm-svn: 344864
---

diff --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp
index 359bfb4..e6d9b34 100644
--- a/clang/lib/Sema/SemaStmtAsm.cpp
+++ b/clang/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/clang/test/Analysis/asm.cpp b/clang/test/Analysis/asm.cpp
new file mode 100644
index 0000000..1180063
--- /dev/null
+++ b/clang/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/clang/test/Analysis/cfg.cpp b/clang/test/Analysis/cfg.cpp
index 85c5be5..f43a809 100644
--- a/clang/test/Analysis/cfg.cpp
+++ b/clang/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