Fix assertion in inline assembler IR gen

[amusman]

Summary

Several inputs may not refer to one output constraint in inline assembler insertions, clang fails on assertion on such test case here:

llvm/lib/IR/InlineAsm.cpp:46: llvm::InlineAsm::InlineAsm(llvm::FunctionType*, const string&, const string&, bool, bool, llvm::InlineAsm::AsmDialect): Assertion `Verify(getFunctionType(), constraints) && "Function type not legal for constraints!"' failed.

I suggest to check this case in Sema.

Thanks,
Alexander

[aaron.ballman]

Generally LGTM, with one question about guarding against overflow.

lib/Sema/SemaStmtAsm.cpp

447

Is it possible for InputConstraintsInfos.size() to be greater than OutputConstrainInfos.size()? Basically, I'm worried about buffer overflow here with TiedTo.

[amusman]

lib/Sema/SemaStmtAsm.cpp

447

The check above (which emits error err_asm_invalid_input_constraint) seems to capture this case so I always get correct TiedTo value here.
What if I add an assertion that TiedTo>=0 && TiedTo <InputMatchedToOutput.size()?

[amusman]

Added an assertion for TiedTo value.

[aaron.ballman]

With one minor change, LGTM! Thank you for working on this.

~Aaron

lib/Sema/SemaStmtAsm.cpp

447

No need for the TiedTo >= 0 since it's an unsigned value anyway. Our usual way of writing this sort of assert is:

assert(TiedTo < InputMatchedToOutput.size() && "TiedTo value out of range");

[amusman]

Thanks, I've commited the changes in revision 248158 (but forgot to add link in svn commit message).
Alexander

include/clang/Basic/DiagnosticSemaKinds.td

let CategoryName = "Inline Assembly Issue" in {
   def err_invalid_asm_cast_lvalue : Error<
     "invalid use of a cast in a inline asm context requiring an l-value: "
     "remove the cast or build with -fheinous-gnu-extensions">;
   def err_invalid_asm_value_for_constraint
       : Error <"value '%0' out of range for constraint '%1'">;
   def err_asm_non_addr_value_in_memory_constraint : Error <
     "reference to a %select{bit-field|vector element|global register variable}0"
     " in asm %select{input|output}1 with a memory constraint '%2'">;
+  def err_asm_input_duplicate_match : Error<
+    "more than one input constraint matches the same output '%0'">;
 
   def warn_asm_label_on_auto_decl : Warning<
     "ignored asm label '%0' on automatic variable">;
   def warn_invalid_asm_cast_lvalue : Warning<
     "invalid use of a cast in an inline asm context requiring an l-value: "
     "accepted due to -fheinous-gnu-extensions, but clang may remove support "
     "for this in the future">;
   def warn_asm_mismatched_size_modifier : Warning<
     "value size does not match register size specified by the constraint "
     "and modifier">,
     InGroup<ASMOperandWidths>;
 
   def note_asm_missing_constraint_modifier : Note<
     "use constraint modifier \"%0\"">;
+  def note_asm_input_duplicate_first : Note<
+    "constraint '%0' is already present here">;
 }
 
 let CategoryName = "Semantic Issue" in {
 
 def err_invalid_conversion_between_vectors : Error<
   "invalid conversion between vector type%diff{ $ and $|}0,1 of different "
   "size">;
 def err_invalid_conversion_between_vector_and_integer : Error<

lib/Sema/SemaStmtAsm.cpp

for (unsigned i = 0, e = OutputConstraintInfos.size(); i != e; ++i) {
     unsigned AltCount = ConstraintStr.count(',') + 1;
     if (NumAlternatives == ~0U)
       NumAlternatives = AltCount;
     else if (NumAlternatives != AltCount)
       return StmtError(Diag(NS->getOutputExpr(i)->getLocStart(),
                             diag::err_asm_unexpected_constraint_alternatives)
                        << NumAlternatives << AltCount);
   }
+  SmallVector<size_t, 4> InputMatchedToOutput(OutputConstraintInfos.size(),
+                                              ~0U);
   for (unsigned i = 0, e = InputConstraintInfos.size(); i != e; ++i) {
     TargetInfo::ConstraintInfo &Info = InputConstraintInfos[i];
     StringRef ConstraintStr = Info.getConstraintStr();
     unsigned AltCount = ConstraintStr.count(',') + 1;
     if (NumAlternatives == ~0U)
       NumAlternatives = AltCount;
     else if (NumAlternatives != AltCount)
       return StmtError(Diag(NS->getInputExpr(i)->getLocStart(),
                             diag::err_asm_unexpected_constraint_alternatives)
                        << NumAlternatives << AltCount);
 
     // If this is a tied constraint, verify that the output and input have
     // either exactly the same type, or that they are int/ptr operands with the
     // same size (int/long, int*/long, are ok etc).
     if (!Info.hasTiedOperand()) continue;
 
     unsigned TiedTo = Info.getTiedOperand();
     unsigned InputOpNo = i+NumOutputs;
     Expr *OutputExpr = Exprs[TiedTo];
     Expr *InputExpr = Exprs[InputOpNo];
 
+    // Make sure no more than one input constraint matches each output.
+    assert(TiedTo >= 0 && TiedTo < InputMatchedToOutput.size());
+    if (InputMatchedToOutput[TiedTo] != ~0U) {
+      Diag(NS->getInputExpr(i)->getLocStart(),
+           diag::err_asm_input_duplicate_match)
+          << TiedTo;
+      Diag(NS->getInputExpr(InputMatchedToOutput[TiedTo])->getLocStart(),
+           diag::note_asm_input_duplicate_first)
+          << TiedTo;
+      return StmtError();
+    }
+    InputMatchedToOutput[TiedTo] = i;
+
     if (OutputExpr->isTypeDependent() || InputExpr->isTypeDependent())
       continue;
 
     QualType InTy = InputExpr->getType();
     QualType OutTy = OutputExpr->getType();
     if (Context.hasSameType(InTy, OutTy))
       continue;  // All types can be tied to themselves.
 

test/Sema/asm.c

__asm__("movl $5, %0"
           : "m" (b[3])); // expected-error {{reference to a vector element in asm output with a memory constraint 'm'}}
   __asm__("movl $5, %0"
           : "=rm" (test16_baz)); // expected-error {{reference to a global register variable in asm input with a memory constraint '=rm'}}
   __asm__("movl $5, %0"
           :
           : "m" (test16_baz)); // expected-error {{reference to a global register variable in asm output with a memory constraint 'm'}}
 }
 
+int test17(int t0)
+{
+  int r0, r1;
+  __asm ("addl %2, %2\n\t"
+         "movl $123, %0"
+         : "=a" (r0),
+           "=&r" (r1)
+         : "1" (t0),   // expected-note {{constraint '1' is already present here}}
+           "1" (t0));  // expected-error {{more than one input constraint matches the same output '1'}}
+  return r0 + r1;
+}
+