On Sun, Nov 11, 2012 at 10:42 PM, Bill Wendling <[email protected]> wrote:
> Author: void > Date: Mon Nov 12 00:42:51 2012 > New Revision: 167717 > > URL: http://llvm.org/viewvc/llvm-project?rev=167717&view=rev > Log: > Check that the input size is correct for the given constraint. > > The 'a', 'c', and 'd' constraints on i386 mean a 32-bit register. We cannot > place a 64-bit value into the 32-bit register. Error out instead of > causing the > compiler to spew general badness. > <rdar://problem/12415959> > A few more comments in addition to the stuff Eli mentioned. > + virtual bool validateInputSize(StringRef Constraint, > + unsigned Size) const { > + switch (Constraint[0]) { > + default: break; > + case 'a': > + case 'b': > + case 'c': > + case 'd': > + return Size == 32; > + } > + > + return true; > + } > > This needs some comments as well. It's both not obvious and (as you later saw) not quite correct. You should document the assumptions. Also, it makes review easier. > > Exprs[i] = Result.take(); > InputConstraintInfos.push_back(Info); > + > + const Type *Ty = Exprs[i]->getType().getTypePtr(); > + if (Ty->isDependentType() || Ty->isIncompleteType()) > + continue; > + > + unsigned Size = Context.getTypeSize(Ty); > + if (!Context.getTargetInfo().validateInputSize(Literal->getString(), > + Size)) > + return StmtError(Diag(InputExpr->getLocStart(), > + diag::err_asm_invalid_input_size) > + << Info.getConstraintStr()); > } > > Thoughts on whether or not this should be a conversion warning or asm warning instead? This isn't much different than the standard truncate warnings, except that it applies to inline assembly. -eric
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
