[cfe-commits] r167717 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/Sema/SemaStmtAsm.cpp test/CodeGen/x86_32-inline-asm.c

Eric Christopher echristo at gmail.com
Tue Nov 13 10:00:57 PST 2012


On Sun, Nov 11, 2012 at 10:42 PM, Bill Wendling <isanbard at gmail.com> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121113/cc07670a/attachment.html>


More information about the cfe-commits mailing list