diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h index 790c8e3..9ce6d22 100644 --- a/include/clang/AST/Stmt.h +++ b/include/clang/AST/Stmt.h @@ -1580,18 +1580,17 @@ public: Kind MyKind; std::string Str; unsigned OperandNo; + SourceLocation Loc; public: AsmStringPiece(const std::string &S) : MyKind(String), Str(S) {} - AsmStringPiece(unsigned OpNo, char Modifier) - : MyKind(Operand), Str(), OperandNo(OpNo) { - Str += Modifier; + AsmStringPiece(unsigned OpNo, const std::string &S, SourceLocation L) + : MyKind(Operand), Str(S), OperandNo(OpNo), Loc(L) { } bool isString() const { return MyKind == String; } bool isOperand() const { return MyKind == Operand; } const std::string &getString() const { - assert(isString()); return Str; } @@ -1600,12 +1599,13 @@ public: return OperandNo; } + SourceLocation getLocation() const { + return Loc; + } + /// getModifier - Get the modifier for this operand, if present. This /// returns '\0' if there was no modifier. - char getModifier() const { - assert(isOperand()); - return Str[0]; - } + char getModifier() const; }; /// AnalyzeAsmString - Analyze the asm string of the current asm, decomposing diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index a7b2ba2..4d53ade 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -773,6 +773,10 @@ private: /// or modify at a particular position. SmallVector DiagFixItHints; + /// \brief Caret location. This is set only when the caret location is + /// different from the current diagnostic location in CurDiagLoc. + SourceLocation CaretLoc; + DiagnosticMapping makeUserMapping(diag::Severity Map, SourceLocation L) { bool isPragma = L.isValid(); DiagnosticMapping Mapping = @@ -998,6 +1002,10 @@ public: DiagObj->DiagFixItHints.push_back(Hint); } + void setCaretLocation(SourceLocation L) { + DiagObj->CaretLoc = L; + } + void addFlagValue(StringRef V) const { DiagObj->FlagValue = V; } }; @@ -1110,6 +1118,7 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(SourceLocation Loc, CurDiagLoc = Loc; CurDiagID = DiagID; FlagValue.clear(); + CaretLoc = SourceLocation(); return DiagnosticBuilder(this); } @@ -1228,6 +1237,10 @@ public: return DiagObj->DiagFixItHints; } + SourceLocation getCaretLocation() const { + return DiagObj->CaretLoc; + } + /// \brief Format this diagnostic into a string, substituting the /// formal arguments into the %0 slots. /// diff --git a/include/clang/Basic/TargetInfo.h b/include/clang/Basic/TargetInfo.h index edef7c0..e26f595 100644 --- a/include/clang/Basic/TargetInfo.h +++ b/include/clang/Basic/TargetInfo.h @@ -581,9 +581,11 @@ public: unsigned /*Size*/) const { return true; } - virtual bool validateConstraintModifier(StringRef /*Constraint*/, - const char /*Modifier*/, - unsigned /*Size*/) const { + virtual bool + validateConstraintModifier(StringRef /*Constraint*/, + char /*Modifier*/, + unsigned /*Size*/, + std::string &/*SuggestedModifier*/) const { return true; } bool resolveSymbolicName(const char *&Name, diff --git a/include/clang/Frontend/DiagnosticRenderer.h b/include/clang/Frontend/DiagnosticRenderer.h index ce1dc90..1a4cae4 100644 --- a/include/clang/Frontend/DiagnosticRenderer.h +++ b/include/clang/Frontend/DiagnosticRenderer.h @@ -88,7 +88,8 @@ protected: DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, ArrayRef Hints, - const SourceManager &SM) = 0; + const SourceManager &SM, + SourceLocation CaretLoc) = 0; virtual void emitIncludeLocation(SourceLocation Loc, PresumedLoc PLoc, const SourceManager &SM) = 0; @@ -116,7 +117,7 @@ private: void emitModuleBuildStack(const SourceManager &SM); void emitCaret(SourceLocation Loc, DiagnosticsEngine::Level Level, ArrayRef Ranges, ArrayRef Hints, - const SourceManager &SM); + const SourceManager &SM, SourceLocation CaretLoc); void emitMacroExpansions(SourceLocation Loc, DiagnosticsEngine::Level Level, ArrayRef Ranges, @@ -132,17 +133,19 @@ public: /// information needed based on macros whose expansions impact the /// diagnostic. /// - /// \param Loc The location for this caret. + /// \param Error location. /// \param Level The level of the diagnostic to be emitted. /// \param Message The diagnostic message to emit. /// \param Ranges The underlined ranges for this code snippet. /// \param FixItHints The FixIt hints active for this diagnostic. /// \param SM The SourceManager; will be null if the diagnostic came from the /// frontend, thus \p Loc will be invalid. + /// \param CaretLoc The location for this caret. void emitDiagnostic(SourceLocation Loc, DiagnosticsEngine::Level Level, StringRef Message, ArrayRef Ranges, ArrayRef FixItHints, const SourceManager *SM, + SourceLocation CaretLoc, DiagOrStoredDiag D = (Diagnostic *)nullptr); void emitStoredDiagnostic(StoredDiagnostic &Diag); diff --git a/include/clang/Frontend/TextDiagnostic.h b/include/clang/Frontend/TextDiagnostic.h index acebb90..79f9d72 100644 --- a/include/clang/Frontend/TextDiagnostic.h +++ b/include/clang/Frontend/TextDiagnostic.h @@ -91,8 +91,9 @@ protected: DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, ArrayRef Hints, - const SourceManager &SM) override { - emitSnippetAndCaret(Loc, Level, Ranges, Hints, SM); + const SourceManager &SM, + SourceLocation CaretLoc) override { + emitSnippetAndCaret(Loc, Level, Ranges, Hints, SM, CaretLoc); } void emitIncludeLocation(SourceLocation Loc, PresumedLoc PLoc, @@ -110,7 +111,7 @@ private: void emitSnippetAndCaret(SourceLocation Loc, DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, ArrayRef Hints, - const SourceManager &SM); + const SourceManager &SM, SourceLocation CaretLoc); void emitSnippet(StringRef SourceLine); diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp index d12642d..d6dba10 100644 --- a/lib/AST/Stmt.cpp +++ b/lib/AST/Stmt.cpp @@ -357,6 +357,11 @@ unsigned AsmStmt::getNumPlusOperands() const { return Res; } +char GCCAsmStmt::AsmStringPiece::getModifier() const { + assert(isOperand()); + return isLetter(Str[0]) ? Str[0] : '\0'; +} + StringRef GCCAsmStmt::getClobber(unsigned i) const { return getClobberStringLiteral(i)->getString(); } @@ -518,21 +523,23 @@ unsigned GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl&Pieces, } // Handle %x4 and %x[foo] by capturing x as the modifier character. - char Modifier = '\0'; + const char *Begin = CurPtr - 1; if (isLetter(EscapedChar)) { if (CurPtr == StrEnd) { // Premature end. DiagOffs = CurPtr-StrStart-1; return diag::err_asm_invalid_escape; } - Modifier = EscapedChar; EscapedChar = *CurPtr++; } + SourceLocation LocStart = getAsmString()->getLocStart(); + if (isDigit(EscapedChar)) { // %n - Assembler operand n unsigned N = 0; --CurPtr; + DiagOffs = CurPtr - StrStart; while (CurPtr != StrEnd && isDigit(*CurPtr)) N = N*10 + ((*CurPtr++)-'0'); @@ -543,7 +550,8 @@ unsigned GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl&Pieces, return diag::err_asm_invalid_operand_number; } - Pieces.push_back(AsmStringPiece(N, Modifier)); + Pieces.push_back(AsmStringPiece(N, std::string(Begin, CurPtr - Begin), + LocStart.getLocWithOffset(DiagOffs))); continue; } @@ -566,7 +574,9 @@ unsigned GCCAsmStmt::AnalyzeAsmString(SmallVectorImpl&Pieces, DiagOffs = CurPtr-StrStart; return diag::err_asm_unknown_symbolic_operand_name; } - Pieces.push_back(AsmStringPiece(N, Modifier)); + Pieces.push_back(AsmStringPiece(N, + std::string(Begin, NameEnd - Begin + 1), + LocStart.getLocWithOffset(DiagOffs))); CurPtr = NameEnd+1; continue; diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp index 2b8de8a..51fb73b 100644 --- a/lib/Basic/Targets.cpp +++ b/lib/Basic/Targets.cpp @@ -4148,8 +4148,10 @@ public: } return R; } - bool validateConstraintModifier(StringRef Constraint, const char Modifier, - unsigned Size) const override { + bool + validateConstraintModifier(StringRef Constraint, const char Modifier, + unsigned Size, + std::string &SuggestedModifier) const override { bool isOutput = (Constraint[0] == '='); bool isInOut = (Constraint[0] == '+'); @@ -4595,9 +4597,10 @@ public: return false; } - virtual bool validateConstraintModifier(StringRef Constraint, - const char Modifier, - unsigned Size) const { + bool + validateConstraintModifier(StringRef Constraint, const char Modifier, + unsigned Size, + std::string &SuggestedModifier) const override { // Strip off constraint modifiers. while (Constraint[0] == '=' || Constraint[0] == '+' || Constraint[0] == '&') Constraint = Constraint.substr(1); @@ -4616,7 +4619,11 @@ public: default: // By default an 'r' constraint will be in the 'x' // registers. - return Size == 64; + if (Size == 64) + return true; + + SuggestedModifier = "w"; + return false; } } } diff --git a/lib/Frontend/DiagnosticRenderer.cpp b/lib/Frontend/DiagnosticRenderer.cpp index bea85b0..8a0e628 100644 --- a/lib/Frontend/DiagnosticRenderer.cpp +++ b/lib/Frontend/DiagnosticRenderer.cpp @@ -126,6 +126,7 @@ void DiagnosticRenderer::emitDiagnostic(SourceLocation Loc, ArrayRef Ranges, ArrayRef FixItHints, const SourceManager *SM, + SourceLocation CaretLoc, DiagOrStoredDiag D) { assert(SM || Loc.isInvalid()); @@ -155,6 +156,7 @@ void DiagnosticRenderer::emitDiagnostic(SourceLocation Loc, // Find the ultimate expansion location for the diagnostic. Loc = SM->getFileLoc(Loc); + CaretLoc = SM->getFileLoc(CaretLoc); PresumedLoc PLoc = SM->getPresumedLoc(Loc, DiagOpts->ShowPresumedLoc); @@ -164,7 +166,7 @@ void DiagnosticRenderer::emitDiagnostic(SourceLocation Loc, // Next, emit the actual diagnostic message and caret. emitDiagnosticMessage(Loc, PLoc, Level, Message, Ranges, SM, D); - emitCaret(Loc, Level, MutableRanges, FixItHints, *SM); + emitCaret(Loc, Level, MutableRanges, FixItHints, *SM, CaretLoc); // If this location is within a macro, walk from UnexpandedLoc up to Loc // and produce a macro backtrace. @@ -187,7 +189,7 @@ void DiagnosticRenderer::emitStoredDiagnostic(StoredDiagnostic &Diag) { Diag.getRanges(), Diag.getFixIts(), Diag.getLocation().isValid() ? &Diag.getLocation().getManager() : nullptr, - &Diag); + SourceLocation(), &Diag); } void DiagnosticRenderer::emitBasicNote(StringRef Message) { @@ -388,10 +390,11 @@ void DiagnosticRenderer::emitCaret(SourceLocation Loc, DiagnosticsEngine::Level Level, ArrayRef Ranges, ArrayRef Hints, - const SourceManager &SM) { + const SourceManager &SM, + SourceLocation CaretLoc) { SmallVector SpellingRanges; mapDiagnosticRanges(Loc, Ranges, SpellingRanges, &SM); - emitCodeContext(Loc, Level, SpellingRanges, Hints, SM); + emitCodeContext(Loc, Level, SpellingRanges, Hints, SM, CaretLoc); } /// \brief Recursively emit notes for each macro expansion and caret @@ -469,7 +472,7 @@ void DiagnosticRenderer::emitMacroExpansions(SourceLocation Loc, else Message << "expanded from macro '" << MacroName << "'"; emitDiagnostic(SpellingLoc, DiagnosticsEngine::Note, Message.str(), - SpellingRanges, None, &SM); + SpellingRanges, None, &SM, SourceLocation()); } DiagnosticNoteRenderer::~DiagnosticNoteRenderer() {} diff --git a/lib/Frontend/SerializedDiagnosticPrinter.cpp b/lib/Frontend/SerializedDiagnosticPrinter.cpp index 29c58a8..c263abd 100644 --- a/lib/Frontend/SerializedDiagnosticPrinter.cpp +++ b/lib/Frontend/SerializedDiagnosticPrinter.cpp @@ -79,7 +79,8 @@ protected: DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, ArrayRef Hints, - const SourceManager &SM) override; + const SourceManager &SM, + SourceLocation CaretLoc) override; void beginDiagnostic(DiagOrStoredDiag D, DiagnosticsEngine::Level Level) override; @@ -561,7 +562,7 @@ void SDiagsWriter::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, State->diagBuf.str(), Info.getRanges(), Info.getFixItHints(), - &Info.getSourceManager(), + &Info.getSourceManager(), Info.getCaretLocation(), &Info); } @@ -676,7 +677,8 @@ void SDiagsRenderer::emitCodeContext(SourceLocation Loc, DiagnosticsEngine::Level Level, SmallVectorImpl &Ranges, ArrayRef Hints, - const SourceManager &SM) { + const SourceManager &SM, + SourceLocation CaretLoc) { Writer.EmitCodeContext(Ranges, Hints, SM); } diff --git a/lib/Frontend/TextDiagnostic.cpp b/lib/Frontend/TextDiagnostic.cpp index 6e6f3dd..ab7f8e2 100644 --- a/lib/Frontend/TextDiagnostic.cpp +++ b/lib/Frontend/TextDiagnostic.cpp @@ -1051,14 +1051,15 @@ static std::string buildFixItInsertionLine(unsigned LineNo, /// /// This routine emits a single line's code snippet and caret line.. /// -/// \param Loc The location for the caret. +/// \param Loc The error position. /// \param Ranges The underlined ranges for this code snippet. /// \param Hints The FixIt hints active for this diagnostic. +/// \param CaretLoc The location for the caret. void TextDiagnostic::emitSnippetAndCaret( SourceLocation Loc, DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, ArrayRef Hints, - const SourceManager &SM) { + const SourceManager &SM, SourceLocation CaretLoc) { assert(!Loc.isInvalid() && "must have a valid source location here"); assert(Loc.isFileID() && "must have a file location here"); @@ -1123,7 +1124,13 @@ void TextDiagnostic::emitSnippetAndCaret( highlightRange(*I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts); // Next, insert the caret itself. + if (CaretLoc.isValid()) { + std::pair CaretLocInfo = SM.getDecomposedLoc(CaretLoc); + ColNo = SM.getColumnNumber(CaretLocInfo.first, CaretLocInfo.second); + } + ColNo = sourceColMap.byteToContainingColumn(ColNo-1); + if (CaretLine.size()emitDiagnostic(Info.getLocation(), Level, DiagMessageStream.str(), Info.getRanges(), Info.getFixItHints(), - &Info.getSourceManager()); + &Info.getSourceManager(), + Info.getCaretLocation()); OS.flush(); } diff --git a/lib/Sema/SemaStmtAsm.cpp b/lib/Sema/SemaStmtAsm.cpp index 5d076ca..7d8383e 100644 --- a/lib/Sema/SemaStmtAsm.cpp +++ b/lib/Sema/SemaStmtAsm.cpp @@ -257,11 +257,23 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, continue; unsigned Size = Context.getTypeSize(Ty); - if (!Context.getTargetInfo() - .validateConstraintModifier(Literal->getString(), Piece.getModifier(), - Size)) - Diag(Exprs[ConstraintIdx]->getLocStart(), - diag::warn_asm_mismatched_size_modifier); + std::string SuggestedModifier; + const TargetInfo &TI = Context.getTargetInfo(); + if (!TI.validateConstraintModifier(Literal->getString(), + Piece.getModifier(), Size, + SuggestedModifier)) { + SourceLocation Loc = Exprs[ConstraintIdx]->getLocStart(); + auto B = Diag(Loc, diag::warn_asm_mismatched_size_modifier); + + if (!SuggestedModifier.empty()) { + std::string AsmStr = Piece.getString(); + SuggestedModifier = "\"%" + AsmStr.insert(0, SuggestedModifier) + "\""; + SourceLocation FixitLoc = Piece.getLocation(); + B.AddFixItHint(FixItHint::CreateInsertion(FixitLoc.getLocWithOffset(-1), + SuggestedModifier)); + B.setCaretLocation(FixitLoc); + } + } } // Validate tied input operands for type mismatches. diff --git a/test/Sema/inline-asm-validate-aarch64.c b/test/Sema/inline-asm-validate-aarch64.c new file mode 100644 index 0000000..ff149bf --- /dev/null +++ b/test/Sema/inline-asm-validate-aarch64.c @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -triple arm64-apple-darwin -fsyntax-only %s 2>&1 | FileCheck %s -strict-whitespace + +typedef unsigned char uint8_t; + +uint8_t constraint_r(uint8_t *addr) { + uint8_t byte; + + __asm__ volatile("ldrb %0, [%1]" : "=r" (byte) : "r" (addr) : "memory"); +// CHECK: warning: value size does not match register size specified by the constraint and modifier +// CHECK:{{^}} ^ +// CHECK:{{^}} "%w0" + + return byte; +} + +uint8_t constraint_r_symbolic(uint8_t *addr) { + uint8_t byte; + + __asm__ volatile("ldrb %[s0], %[s1]" : [s0] "=r" (byte) : [s1] "r" (addr) : "memory"); +// CHECK: warning: value size does not match register size specified by the constraint and modifier +// CHECK:{{^}} ^ +// CHECK:{{^}} "%w[s0]" + + return byte; +} diff --git a/tools/libclang/CIndexDiagnostic.cpp b/tools/libclang/CIndexDiagnostic.cpp index 0d97ebd..e4806bc 100644 --- a/tools/libclang/CIndexDiagnostic.cpp +++ b/tools/libclang/CIndexDiagnostic.cpp @@ -140,7 +140,8 @@ public: DiagnosticsEngine::Level Level, SmallVectorImpl& Ranges, ArrayRef Hints, - const SourceManager &SM) override {} + const SourceManager &SM, + SourceLocation CaretLoc) override {} void emitNote(SourceLocation Loc, StringRef Message, const SourceManager *SM) override {