Index: lib/Sema/SemaStmtAsm.cpp =================================================================== --- lib/Sema/SemaStmtAsm.cpp +++ 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 ParentsToUpdate; + + while (true) { + ParentsToUpdate.push_back(Parent); + if (auto *ParenE = dyn_cast(Parent)) { + Parent = ParenE->getSubExpr(); + continue; + } + + Expr *Child = nullptr; + CastExpr *ParentCast = dyn_cast(Parent); + if (ParentCast) + Child = ParentCast->getSubExpr(); + else + return; + + if (auto *CastE = dyn_cast(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->getLocStart(), diag::err_invalid_asm_cast_lvalue) + << BadArgument->getSourceRange(); + } else { + S.Diag(LVal->getLocStart(), 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 @@ /// /// 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 @@ // are supposed to allow. const Expr *E2 = E->IgnoreParenNoopCasts(S.Context); if (E != E2 && E2->isLValue()) { - if (!S.getLangOpts().HeinousExtensions) - S.Diag(E2->getLocStart(), diag::err_invalid_asm_cast_lvalue) - << E->getSourceRange(); - else - S.Diag(E2->getLocStart(), 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 @@ break; case Expr::MLV_LValueCast: { const Expr *LVal = OutputExpr->IgnoreParenNoopCasts(Context); - if (!getLangOpts().HeinousExtensions) { - Diag(LVal->getLocStart(), diag::err_invalid_asm_cast_lvalue) - << OutputExpr->getSourceRange(); - } else { - Diag(LVal->getLocStart(), diag::warn_invalid_asm_cast_lvalue) - << OutputExpr->getSourceRange(); - } + emitAndFixInvalidAsmCastLValue(LVal, OutputExpr, *this); // Accept, even if we emitted an error diagnostic. break; } Index: test/Analysis/asm.cpp =================================================================== --- /dev/null +++ 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}} +} Index: test/Analysis/cfg.cpp =================================================================== --- test/Analysis/cfg.cpp +++ test/Analysis/cfg.cpp @@ -1,6 +1,6 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -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 -analyzer-config cfg-temporary-dtors=true -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 -analyzer-config cfg-temporary-dtors=true -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 -analyzer-config cfg-temporary-dtors=true -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 @@ 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