[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

llvm-svn: 290539

7 files changed, 127 insertions, 19 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5f59a10c1c1..1ac5dfce0d0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/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/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index dcd5a088dfc..208f8e8c713 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/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/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index e37f13a05b9..b1b01e5f584 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/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/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index dd88646b17f..85a83bca002 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/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<bool> &Features, X86SSEEnum Level,
diff --git a/clang/lib/Headers/intrin.h b/clang/lib/Headers/intrin.h
index 76cbb9acb1c..7c91ebaee8c 100644
--- a/clang/lib/Headers/intrin.h
+++ b/clang/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/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp
index 9a182449845..50366222093 100644
--- a/clang/lib/Sema/SemaStmtAsm.cpp
+++ b/clang/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<DeclRefExpr>(Expression)) {
+    // Handle cases where the expression is a variable
+    const VarDecl *Variable = dyn_cast<VarDecl>(AsmDeclRef->getDecl());
+    if (Variable && Variable->getStorageClass() == SC_Register) {
+      if (AsmLabelAttr *Attr = Variable->getAttr<AsmLabelAttr>())
+        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/clang/test/Sema/asm.c b/clang/test/Sema/asm.c
index 69c33f7ccf2..e49a1663a89 100644
--- a/clang/test/Sema/asm.c
+++ b/clang/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