On Fri, Jun 12, 2015 at 11:31 AM, Ahmed Bougacha <[email protected]> wrote:
> On Thu, Jun 11, 2015 at 2:17 PM, Richard Smith <[email protected]> > wrote: > > On Thu, Jun 11, 2015 at 2:13 PM, Richard Smith <[email protected]> > > wrote: > >> > >> On Thu, Jun 11, 2015 at 11:19 AM, Ahmed Bougacha > >> <[email protected]> wrote: > >>> > >>> Author: ab > >>> Date: Thu Jun 11 13:19:34 2015 > >>> New Revision: 239549 > >>> > >>> URL: http://llvm.org/viewvc/llvm-project?rev=239549&view=rev > >>> Log: > >>> [CodeGen] Emit Constants for immediate inlineasm arguments. > >>> > >>> For inline assembly immediate constraints, we currently always use > >>> EmitScalarExpr, instead of directly emitting the constant. When the > >>> overflow sanitizer is enabled, this generates overflow intrinsics > >>> instead of constants. > >>> > >>> Instead, emit a constant for constraints that either require an > >>> immediate (e.g. 'I' on X86), or only accepts constants (immediate > >>> or symbolic; i.e., don't accept registers or memory). > >>> > >>> Fixes PR19763. > >>> > >>> Differential Revision: http://reviews.llvm.org/D10255 > >>> > >>> Added: > >>> cfe/trunk/test/CodeGen/inline-asm-immediate-ubsan.c > >>> Modified: > >>> cfe/trunk/lib/CodeGen/CGStmt.cpp > >>> > >>> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp > >>> URL: > >>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=239549&r1=239548&r2=239549&view=diff > >>> > >>> > ============================================================================== > >>> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original) > >>> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Thu Jun 11 13:19:34 2015 > >>> @@ -1750,6 +1750,16 @@ llvm::Value* CodeGenFunction::EmitAsmInp > >>> const > >>> TargetInfo::ConstraintInfo &Info, > >>> const Expr *InputExpr, > >>> std::string > &ConstraintStr) { > >>> + // If this can't be a register or memory, i.e., has to be a constant > >>> + // (immediate or symbolic), try to emit it as such. > >>> + if (!Info.allowsRegister() && !Info.allowsMemory()) { > >>> + llvm::APSInt Result; > >>> + if (InputExpr->isIntegerConstantExpr(Result, getContext())) > >> > >> > >> Please use EvaluateKnownConstInt here instead. This construct will > >> redundantly re-check whether the expression is an ICE first. > > > > > > Ah, sorry, you don't know that this is constant except in the > > requiresImmediateConstant() case. You should be calling EvaluateAsInt > here > > instead. > > Yes, I'm relying on the ICE-checking here, but I just realized Sema > also calls EvaluateAsInt, and because of that doesn't check that the > requiresImmediateConstant() operand is an ICE. > > Does the below patch make sense? It lets us emit an error for the > overflowing "constant" expressions we want to catch here. > Yes, that looks fine. Please also include a testcase (if you need an example of a non-ICE that we can evaluate, something like "(long)(void*)0" should work). With that, we can use EvaluateAsInt in CGStmt.cpp, and we'll do the > right thing for all combinations except overflowing "i". I'm not > quite sure how to handle that, if it's even possible in the general > case: what happens if a link-time constant expression overflows? > In C++11 onwards, it's not an ICE. In any case, the evaluator will produce the two's complement result. Thanks for having a look! > -Ahmed > > > diff --git a/cfe/trunk/lib/Sema/SemaStmtAsm.cpp > b/cfe/trunk/lib/Sema/SemaStmtAsm.cpp > --- a/cfe/trunk/lib/Sema/SemaStmtAsm.cpp (revision 239545) > +++ b/cfe/trunk/lib/Sema/SemaStmtAsm.cpp (working copy) > @@ -255,7 +255,7 @@ > << InputExpr->getSourceRange()); > } else if (Info.requiresImmediateConstant() && > !Info.allowsRegister()) { > llvm::APSInt Result; > - if (!InputExpr->EvaluateAsInt(Result, Context)) > + if (!InputExpr->isIntegerConstantExpr(Result, Context)) > return StmtError( > Diag(InputExpr->getLocStart(), > diag::err_asm_immediate_expected) > << Info.getConstraintStr() << InputExpr->getSourceRange()); >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
