<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 12, 2015 at 11:31 AM, Ahmed Bougacha <span dir="ltr"><<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Jun 11, 2015 at 2:17 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
> On Thu, Jun 11, 2015 at 2:13 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> wrote:<br>
>><br>
>> On Thu, Jun 11, 2015 at 11:19 AM, Ahmed Bougacha<br>
>> <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>> wrote:<br>
>>><br>
>>> Author: ab<br>
>>> Date: Thu Jun 11 13:19:34 2015<br>
>>> New Revision: 239549<br>
>>><br>
>>> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D239549-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4JwDiZ_y4WwFfhtY58bGDMG2wyaiW-0FNPV-T0V2VpE&s=_I_pdttd8ZxU9JsLP2CM7xTZykS76vJW8fy7NqIkXfk&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=239549&view=rev</a><br>
>>> Log:<br>
>>> [CodeGen] Emit Constants for immediate inlineasm arguments.<br>
>>><br>
>>> For inline assembly immediate constraints, we currently always use<br>
>>> EmitScalarExpr, instead of directly emitting the constant. When the<br>
>>> overflow sanitizer is enabled, this generates overflow intrinsics<br>
>>> instead of constants.<br>
>>><br>
>>> Instead, emit a constant for constraints that either require an<br>
>>> immediate (e.g. 'I' on X86), or only accepts constants (immediate<br>
>>> or symbolic; i.e., don't accept registers or memory).<br>
>>><br>
>>> Fixes PR19763.<br>
>>><br>
>>> Differential Revision: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10255&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4JwDiZ_y4WwFfhtY58bGDMG2wyaiW-0FNPV-T0V2VpE&s=z-x5WYGQvpxEn9k-IB_jgZIMKtAlYSXIRFkBo2p0nTA&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10255</a><br>
>>><br>
>>> Added:<br>
>>> cfe/trunk/test/CodeGen/inline-asm-immediate-ubsan.c<br>
>>> Modified:<br>
>>> cfe/trunk/lib/CodeGen/CGStmt.cpp<br>
>>><br>
>>> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp<br>
>>> URL:<br>
>>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_CodeGen_CGStmt.cpp-3Frev-3D239549-26r1-3D239548-26r2-3D239549-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=4JwDiZ_y4WwFfhtY58bGDMG2wyaiW-0FNPV-T0V2VpE&s=9srrTyPRh9-AXeVQ5a5MXTFYacH9dP_zDaUPXLs8eAg&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=239549&r1=239548&r2=239549&view=diff</a><br>
>>><br>
>>> ==============================================================================<br>
>>> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)<br>
>>> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Thu Jun 11 13:19:34 2015<br>
>>> @@ -1750,6 +1750,16 @@ llvm::Value* CodeGenFunction::EmitAsmInp<br>
>>> const<br>
>>> TargetInfo::ConstraintInfo &Info,<br>
>>> const Expr *InputExpr,<br>
>>> std::string &ConstraintStr) {<br>
>>> + // If this can't be a register or memory, i.e., has to be a constant<br>
>>> + // (immediate or symbolic), try to emit it as such.<br>
>>> + if (!Info.allowsRegister() && !Info.allowsMemory()) {<br>
>>> + llvm::APSInt Result;<br>
>>> + if (InputExpr->isIntegerConstantExpr(Result, getContext()))<br>
>><br>
>><br>
>> Please use EvaluateKnownConstInt here instead. This construct will<br>
>> redundantly re-check whether the expression is an ICE first.<br>
><br>
><br>
> Ah, sorry, you don't know that this is constant except in the<br>
> requiresImmediateConstant() case. You should be calling EvaluateAsInt here<br>
> instead.<br>
<br>
</div></div>Yes, I'm relying on the ICE-checking here, but I just realized Sema<br>
also calls EvaluateAsInt, and because of that doesn't check that the<br>
requiresImmediateConstant() operand is an ICE.<br>
<br>
Does the below patch make sense? It lets us emit an error for the<br>
overflowing "constant" expressions we want to catch here.<br></blockquote><div><br></div><div>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).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
With that, we can use EvaluateAsInt in CGStmt.cpp, and we'll do the<br>
right thing for all combinations except overflowing "i". I'm not<br>
quite sure how to handle that, if it's even possible in the general<br>
case: what happens if a link-time constant expression overflows?<br></blockquote><div><br></div><div>In C++11 onwards, it's not an ICE. In any case, the evaluator will produce the two's complement result.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks for having a look!<br>
-Ahmed<br>
<br>
<br>
diff --git a/cfe/trunk/lib/Sema/SemaStmtAsm.cpp<br>
b/cfe/trunk/lib/Sema/SemaStmtAsm.cpp<br>
--- a/cfe/trunk/lib/Sema/SemaStmtAsm.cpp (revision 239545)<br>
+++ b/cfe/trunk/lib/Sema/SemaStmtAsm.cpp (working copy)<br>
@@ -255,7 +255,7 @@<br>
<< InputExpr->getSourceRange());<br>
} else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {<br>
llvm::APSInt Result;<br>
- if (!InputExpr->EvaluateAsInt(Result, Context))<br>
+ if (!InputExpr->isIntegerConstantExpr(Result, Context))<br>
return StmtError(<br>
Diag(InputExpr->getLocStart(), diag::err_asm_immediate_expected)<br>
<< Info.getConstraintStr() << InputExpr->getSourceRange());<br>
</blockquote></div><br></div></div>