From 55365e4b39fd10bec58d7cb1c436af9767c8dad2 Mon Sep 17 00:00:00 2001 From: Aleksei Sidorin Date: Sat, 20 Oct 2018 22:49:23 +0000 Subject: [PATCH] [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 --- clang/lib/Sema/SemaStmtAsm.cpp | 69 +++++++++++++++++++++++++++++++++--------- clang/test/Analysis/asm.cpp | 12 ++++++++ clang/test/Analysis/cfg.cpp | 22 ++++++++++++-- 3 files changed, 87 insertions(+), 16 deletions(-) create mode 100644 clang/test/Analysis/asm.cpp 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 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->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 -- 2.7.4