Fix inline assembler constraint validation

[joerg]

Summary

The current constraint logic is both too lax and too strict. It fails for input outside the [INT_MIN..INT_MAX] range, but it also implicitly accepts 0 as value when it should not. Adjust logic to handle both correctly.

[compnerd]

Nice find!

[efriedma]

include/clang/Basic/TargetInfo.h

860 ↗

(On Diff #188265)

Isn't the "ImmSet.count" call still wrong? It looks like it crashes on an 128-bit immediate, and implicitly truncates a 64-bit immediate. I guess you could check Value.isSignedIntN(32)?

[joerg]

include/clang/Basic/TargetInfo.h

860 ↗

(On Diff #188265)

I've added that and a test case for truncation as a follow-up.

cfe/trunk/include/clang/Basic/TargetInfo.h

enum {
       CI_ImmediateConstant = 0x10, // This operand must be an immediate constant
       CI_EarlyClobber = 0x20,      // "&" output constraint (early clobber).
     };
     unsigned Flags;
     int TiedOperand;
     struct {
       int Min;
       int Max;
+      bool isConstrained;
     } ImmRange;
     llvm::SmallSet<int, 4> ImmSet;
 
     std::string ConstraintStr;  // constraint: "=rm"
     std::string Name;           // Operand name: [foo] with no []'s.
   public:
     ConstraintInfo(StringRef ConstraintStr, StringRef Name)
         : Flags(0), TiedOperand(-1), ConstraintStr(ConstraintStr.str()),
           Name(Name.str()) {
       ImmRange.Min = ImmRange.Max = 0;
+      ImmRange.isConstrained = false;
     }
 
     const std::string &getConstraintStr() const { return ConstraintStr; }
     const std::string &getName() const { return Name; }
     bool isReadWrite() const { return (Flags & CI_ReadWrite) != 0; }
     bool earlyClobber() { return (Flags & CI_EarlyClobber) != 0; }
     bool allowsRegister() const { return (Flags & CI_AllowsRegister) != 0; }
     bool allowsMemory() const { return (Flags & CI_AllowsMemory) != 0; }
unsigned getTiedOperand() const {
       assert(hasTiedOperand() && "Has no tied operand!");
       return (unsigned)TiedOperand;
     }
 
     bool requiresImmediateConstant() const {
       return (Flags & CI_ImmediateConstant) != 0;
     }
     bool isValidAsmImmediate(const llvm::APInt &Value) const {
-      return (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max)) ||
-             ImmSet.count(Value.getZExtValue()) != 0;
+      if (!ImmSet.empty())
+        return ImmSet.count(Value.getZExtValue()) != 0;
+      return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max));
     }
 
     void setIsReadWrite() { Flags |= CI_ReadWrite; }
     void setEarlyClobber() { Flags |= CI_EarlyClobber; }
     void setAllowsMemory() { Flags |= CI_AllowsMemory; }
     void setAllowsRegister() { Flags |= CI_AllowsRegister; }
     void setHasMatchingInput() { Flags |= CI_HasMatchingInput; }
     void setRequiresImmediate(int Min, int Max) {
       Flags |= CI_ImmediateConstant;
       ImmRange.Min = Min;
       ImmRange.Max = Max;
+      ImmRange.isConstrained = true;
     }
     void setRequiresImmediate(llvm::ArrayRef<int> Exacts) {
       Flags |= CI_ImmediateConstant;
       for (int Exact : Exacts)
         ImmSet.insert(Exact);
     }
     void setRequiresImmediate(int Exact) {
       Flags |= CI_ImmediateConstant;
       ImmSet.insert(Exact);
     }
     void setRequiresImmediate() {
       Flags |= CI_ImmediateConstant;
-      ImmRange.Min = INT_MIN;
-      ImmRange.Max = INT_MAX;
     }
 
     /// Indicate that this is an input operand that is tied to
     /// the specified output operand.
     ///
     /// Copy over the various constraint information from the output.
     void setTiedOperand(unsigned N, ConstraintInfo &Output) {
       Output.setHasMatchingInput();

cfe/trunk/test/Sema/inline-asm-validate-x86.c

void K(int i, int j) {
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "K"(96)); // expected-no-error
 }
 
 void L(int i, int j) {
   static const int Invalid1 = 1;
   static const int Invalid2 = 42;
+  static const int Invalid3 = 0;
   static const int Valid1 = 0xff;
   static const int Valid2 = 0xffff;
   static const int Valid3 = 0xffffffff;
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "L"(j)); // expected-error{{constraint 'L' expects an integer constant expression}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "L"(Invalid1)); // expected-error{{value '1' out of range for constraint 'L'}}
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "L"(Invalid2)); // expected-error{{value '42' out of range for constraint 'L'}}
   __asm__("xorl %0,%2"
           : "=r"(i)
+          : "0"(i), "L"(Invalid3)); // expected-error{{value '0' out of range for constraint 'L'}}
+  __asm__("xorl %0,%2"
+          : "=r"(i)
           : "0"(i), "L"(Valid1)); // expected-no-error
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "L"(Valid2)); // expected-no-error
   __asm__("xorl %0,%2"
           : "=r"(i)
           : "0"(i), "L"(Valid3)); // expected-no-error
 }