From: Marina Yatsina Date: Mon, 26 Dec 2016 12:23:42 +0000 (+0000) Subject: [inline-asm]No error for conflict between inputs\outputs and clobber list X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=158bdde29231fe764b5849ef5eb21398b6345536;p=clang [inline-asm]No error for conflict between inputs\outputs and clobber list According to extended asm syntax, a case where the clobber list includes a variable from the inputs or outputs should be an error - conflict. for example: const long double a = 0.0; int main() { char b; double t1 = a; __asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)"); return 0; } This should conflict with the output - t1 which is st, and st which is st aswell. The patch fixes it. Commit on behald of Ziv Izhar. Differential Revision: https://reviews.llvm.org/D15075 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@290539 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 5f59a10c1c..1ac5dfce0d 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7069,6 +7069,10 @@ let CategoryName = "Inline Assembly Issue" in { "constraint '%0' is already present here">; } + def error_inoutput_conflict_with_clobber : Error< + "asm-specifier for input or output variable conflicts with asm" + " clobber list">; + let CategoryName = "Semantic Issue" in { def err_invalid_conversion_between_vectors : Error< diff --git a/include/clang/Basic/TargetInfo.h b/include/clang/Basic/TargetInfo.h index dcd5a088df..208f8e8c71 100644 --- a/include/clang/Basic/TargetInfo.h +++ b/include/clang/Basic/TargetInfo.h @@ -605,8 +605,16 @@ public: /// \brief Returns the "normalized" GCC register name. /// - /// For example, on x86 it will return "ax" when "eax" is passed in. - StringRef getNormalizedGCCRegisterName(StringRef Name) const; + /// ReturnCannonical true will return the register name without any additions + /// such as "{}" or "%" in it's canonical form, for example: + /// ReturnCanonical = true and Name = "rax", will return "ax". + StringRef getNormalizedGCCRegisterName(StringRef Name, + bool ReturnCanonical = false) const; + + virtual StringRef getConstraintRegister(const StringRef &Constraint, + const StringRef &Expression) const { + return ""; + } struct ConstraintInfo { enum { diff --git a/lib/Basic/TargetInfo.cpp b/lib/Basic/TargetInfo.cpp index e37f13a05b..b1b01e5f58 100644 --- a/lib/Basic/TargetInfo.cpp +++ b/lib/Basic/TargetInfo.cpp @@ -410,8 +410,8 @@ bool TargetInfo::isValidGCCRegisterName(StringRef Name) const { return false; } -StringRef -TargetInfo::getNormalizedGCCRegisterName(StringRef Name) const { +StringRef TargetInfo::getNormalizedGCCRegisterName(StringRef Name, + bool ReturnCanonical) const { assert(isValidGCCRegisterName(Name) && "Invalid register passed in"); // Get rid of any register prefix. @@ -436,7 +436,7 @@ TargetInfo::getNormalizedGCCRegisterName(StringRef Name) const { // Make sure the register that the additional name is for is within // the bounds of the register names from above. if (AN == Name && ARN.RegNum < Names.size()) - return Name; + return ReturnCanonical ? Names[ARN.RegNum] : Name; } // Now check aliases. diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp index dd88646b17..85a83bca00 100644 --- a/lib/Basic/Targets.cpp +++ b/lib/Basic/Targets.cpp @@ -2789,6 +2789,40 @@ public: const char *getClobbers() const override { return "~{dirflag},~{fpsr},~{flags}"; } + + StringRef getConstraintRegister(const StringRef &Constraint, + const StringRef &Expression) const override { + StringRef::iterator I, E; + for (I = Constraint.begin(), E = Constraint.end(); I != E; ++I) { + if (isalpha(*I)) + break; + } + if (I == E) + return ""; + switch (*I) { + // For the register constraints, return the matching register name + case 'a': + return "ax"; + case 'b': + return "bx"; + case 'c': + return "cx"; + case 'd': + return "dx"; + case 'S': + return "si"; + case 'D': + return "di"; + // In case the constraint is 'r' we need to return Expression + case 'r': + return Expression; + default: + // Default value if there is no constraint for the register + return ""; + } + return ""; + } + void getTargetDefines(const LangOptions &Opts, MacroBuilder &Builder) const override; static void setSSELevel(llvm::StringMap &Features, X86SSEEnum Level, diff --git a/lib/Headers/intrin.h b/lib/Headers/intrin.h index 76cbb9acb1..7c91ebaee8 100644 --- a/lib/Headers/intrin.h +++ b/lib/Headers/intrin.h @@ -1010,40 +1010,33 @@ __readgsqword(unsigned long __offset) { #if defined(__i386__) || defined(__x86_64__) static __inline__ void __DEFAULT_FN_ATTRS __movsb(unsigned char *__dst, unsigned char const *__src, size_t __n) { - __asm__("rep movsb" : : "D"(__dst), "S"(__src), "c"(__n) - : "%edi", "%esi", "%ecx"); + __asm__("rep movsb" : : "D"(__dst), "S"(__src), "c"(__n)); } static __inline__ void __DEFAULT_FN_ATTRS __movsd(unsigned long *__dst, unsigned long const *__src, size_t __n) { - __asm__("rep movsl" : : "D"(__dst), "S"(__src), "c"(__n) - : "%edi", "%esi", "%ecx"); + __asm__("rep movsl" : : "D"(__dst), "S"(__src), "c"(__n)); } static __inline__ void __DEFAULT_FN_ATTRS __movsw(unsigned short *__dst, unsigned short const *__src, size_t __n) { - __asm__("rep movsw" : : "D"(__dst), "S"(__src), "c"(__n) - : "%edi", "%esi", "%ecx"); + __asm__("rep movsw" : : "D"(__dst), "S"(__src), "c"(__n)); } static __inline__ void __DEFAULT_FN_ATTRS __stosd(unsigned long *__dst, unsigned long __x, size_t __n) { - __asm__("rep stosl" : : "D"(__dst), "a"(__x), "c"(__n) - : "%edi", "%ecx"); + __asm__("rep stosl" : : "D"(__dst), "a"(__x), "c"(__n)); } static __inline__ void __DEFAULT_FN_ATTRS __stosw(unsigned short *__dst, unsigned short __x, size_t __n) { - __asm__("rep stosw" : : "D"(__dst), "a"(__x), "c"(__n) - : "%edi", "%ecx"); + __asm__("rep stosw" : : "D"(__dst), "a"(__x), "c"(__n)); } #endif #ifdef __x86_64__ static __inline__ void __DEFAULT_FN_ATTRS __movsq(unsigned long long *__dst, unsigned long long const *__src, size_t __n) { - __asm__("rep movsq" : : "D"(__dst), "S"(__src), "c"(__n) - : "%edi", "%esi", "%ecx"); + __asm__("rep movsq" : : "D"(__dst), "S"(__src), "c"(__n)); } static __inline__ void __DEFAULT_FN_ATTRS __stosq(unsigned __int64 *__dst, unsigned __int64 __x, size_t __n) { - __asm__("rep stosq" : : "D"(__dst), "a"(__x), "c"(__n) - : "%edi", "%ecx"); + __asm__("rep stosq" : : "D"(__dst), "a"(__x), "c"(__n)); } #endif diff --git a/lib/Sema/SemaStmtAsm.cpp b/lib/Sema/SemaStmtAsm.cpp index 9a18244984..5036622209 100644 --- a/lib/Sema/SemaStmtAsm.cpp +++ b/lib/Sema/SemaStmtAsm.cpp @@ -22,6 +22,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/MC/MCParser/MCAsmParser.h" using namespace clang; using namespace sema; @@ -137,6 +138,57 @@ static bool checkExprMemoryConstraintCompat(Sema &S, Expr *E, return false; } +// Extracting the register name from the Expression value, +// if there is no register name to extract, returns "" +static StringRef extractRegisterName(const Expr *Expression, + const TargetInfo &Target) { + Expression = Expression->IgnoreImpCasts(); + if (const DeclRefExpr *AsmDeclRef = dyn_cast(Expression)) { + // Handle cases where the expression is a variable + const VarDecl *Variable = dyn_cast(AsmDeclRef->getDecl()); + if (Variable && Variable->getStorageClass() == SC_Register) { + if (AsmLabelAttr *Attr = Variable->getAttr()) + if (Target.isValidGCCRegisterName(Attr->getLabel())) + return Target.getNormalizedGCCRegisterName(Attr->getLabel(), true); + } + } + return ""; +} + +// Checks if there is a conflict between the input and output lists with the +// clobbers list. If there's a conflict, returns the location of the +// conflicted clobber, else returns nullptr +static SourceLocation +getClobberConflictLocation(MultiExprArg Exprs, StringLiteral **Constraints, + StringLiteral **Clobbers, int NumClobbers, + const TargetInfo &Target, ASTContext &Cont) { + llvm::StringSet<> InOutVars; + // Collect all the input and output registers from the extended asm + // statement + // in order to check for conflicts with the clobber list + for (int i = 0; i < Exprs.size(); ++i) { + StringRef Constraint = Constraints[i]->getString(); + StringRef InOutReg = Target.getConstraintRegister( + Constraint, extractRegisterName(Exprs[i], Target)); + if (InOutReg != "") + InOutVars.insert(InOutReg); + } + // Check for each item in the clobber list if it conflicts with the input + // or output + for (int i = 0; i < NumClobbers; ++i) { + StringRef Clobber = Clobbers[i]->getString(); + // We only check registers, therefore we don't check cc and memory + // clobbers + if (Clobber == "cc" || Clobber == "memory") + continue; + Clobber = Target.getNormalizedGCCRegisterName(Clobber, true); + // Go over the output's registers we collected + if (InOutVars.count(Clobber)) + return Clobbers[i]->getLocStart(); + } + return SourceLocation(); +} + StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, bool IsVolatile, unsigned NumOutputs, unsigned NumInputs, IdentifierInfo **Names, @@ -543,6 +595,13 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, return StmtError(); } + // Check for conflicts between clobber list and input or output lists + SourceLocation ConstraintLoc = + getClobberConflictLocation(Exprs, Constraints, Clobbers, NumClobbers, + Context.getTargetInfo(), Context); + if (ConstraintLoc.isValid()) + return Diag(ConstraintLoc, diag::error_inoutput_conflict_with_clobber); + return NS; } diff --git a/test/Sema/asm.c b/test/Sema/asm.c index 69c33f7ccf..e49a1663a8 100644 --- a/test/Sema/asm.c +++ b/test/Sema/asm.c @@ -28,6 +28,16 @@ void clobbers() { asm ("nop" : : : "204"); // expected-error {{unknown register name '204' in asm}} asm ("nop" : : : "-1"); // expected-error {{unknown register name '-1' in asm}} asm ("nop" : : : "+1"); // expected-error {{unknown register name '+1' in asm}} + register void *clobber_conflict asm ("%rcx"); + register void *no_clobber_conflict asm ("%rax"); + int a,b,c; + asm ("nop" : "=r" (no_clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "r" (clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=c" (a) : "r" (no_clobber_conflict) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (no_clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=r" (clobber_conflict) : "c" (c) : "%rcx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} + asm ("nop" : "=a" (a) : "b" (b) : "%rcx", "%rbx"); // expected-error {{asm-specifier for input or output variable conflicts with asm clobber list}} } // rdar://6094010