[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

[Eli Friedman]

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>
>
> Added:
>     cfe/trunk/test/CodeGen/x86_32-inline-asm.c
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/include/clang/Basic/TargetInfo.h
>     cfe/trunk/lib/Basic/Targets.cpp
>     cfe/trunk/lib/Sema/SemaStmtAsm.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=167717&r1=167716&r2=167717&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Nov 12 00:42:51 2012
> @@ -5141,6 +5141,8 @@
>      "%diff{$ matching output with type $|}0,1">;
>    def err_asm_unknown_register_name : Error<"unknown register name '%0' in asm">;
>    def err_asm_empty : Error<"__asm used with no assembly instructions">;
> +  def err_asm_invalid_input_size : Error<
> +    "invalid input size for constraint '%0'">;
>    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">;
>
> Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=167717&r1=167716&r2=167717&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
> +++ cfe/trunk/include/clang/Basic/TargetInfo.h Mon Nov 12 00:42:51 2012
> @@ -517,6 +517,10 @@
>    bool validateInputConstraint(ConstraintInfo *OutputConstraints,
>                                 unsigned NumOutputs,
>                                 ConstraintInfo &info) const;
> +  virtual bool validateInputSize(StringRef /*Constraint*/,
> +                                 unsigned /*Size*/) const {
> +    return true;
> +  }
>    virtual bool validateConstraintModifier(StringRef /*Constraint*/,
>                                            const char /*Modifier*/,
>                                            unsigned /*Size*/) const {
>
> Modified: cfe/trunk/lib/Basic/Targets.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=167717&r1=167716&r2=167717&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/Targets.cpp (original)
> +++ cfe/trunk/lib/Basic/Targets.cpp Mon Nov 12 00:42:51 2012
> @@ -2595,6 +2595,19 @@
>      if (RegNo == 1) return 2;
>      return -1;
>    }
> +  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;
> +  }
>  };
>  } // end anonymous namespace
>
>
> Modified: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAsm.cpp?rev=167717&r1=167716&r2=167717&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaStmtAsm.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp Mon Nov 12 00:42:51 2012
> @@ -179,6 +179,17 @@
>
>      Exprs[i] = Result.take();
>      InputConstraintInfos.push_back(Info);
> +
> +    const Type *Ty = Exprs[i]->getType().getTypePtr();
> +    if (Ty->isDependentType() || Ty->isIncompleteType())
> +      continue;

Completely skipping checking incomplete types is probably wrong; the
following crashes at the moment:

struct S;
void f(struct S* s) { __asm(""::"a"(*s)); }

Also, calling isIncompleteType() is usually not the right thing to do,
because whether it returns true or false for template types depends on
whether the type is instantiated yet, which in this context is
basically arbitrary. The way to check for a complete type is
RequireCompleteType().

> +    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());
>    }

This checks input constraints, but not output constraints.

Also, it would look a little nicer to move this check beside the check
for isVoidType() (might require refactoring the code a bit).


Even if the constraints are crazy, LLVM shouldn't crash on IR that
passes the validator.  Please file a bug.

-Eli