<div dir="ltr">Thanks for the info! For now, I've committed the patch without the diagnostic and will come back to it when I get some time. I'll think about whether this diagnostic is better left out. I'm not sure if I'll decide to not include it at all since you, as you mentioned, it would miss a lot of major cases.<div><br><div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">Zola Bridges</div></div></div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 10, 2019 at 12:06 PM Eli Friedman <<a href="mailto:efriedma@codeaurora.org">efriedma@codeaurora.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<div class="gmail-m_-7943172813147780136moz-cite-prefix">On 1/10/2019 10:44 AM, Zola Bridges via
cfe-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><span id="gmail-m_-7943172813147780136gmail-docs-internal-guid-da7b979b-7fff-e5c3-3332-96e5ad759afc">
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Hi everyone,</span></p>
<p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><b style="font-weight:normal" id="gmail-m_-7943172813147780136gmail-docs-internal-guid-897a36b4-7fff-8dec-cb8b-c86a658190ad"><br>
</b></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">I'm working on adding function level Clang attributes for the Speculative Load Hardening </span><span style="background-color:transparent;color:rgb(0,0,0);font-family:Arial;font-size:11pt;white-space:pre-wrap">(SLH) feature, so devs who know what they are doing can enable or disable SLH function by </span><span style="background-color:transparent;color:rgb(0,0,0);font-family:Arial;font-size:11pt;white-space:pre-wrap">function. There are two attributes 'no_speculative_load_hardening' and 'speculative_load_hardening.'</span></p>
<p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><b style="font-weight:normal"><br>
</b></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">As a part of this, I want to diagnose a special case where a function marked</span></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">'no_speculative_load_hardening' will still have 'speculative_load_hardening' enabled.</span></p>
<p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><b style="font-weight:normal"><br>
</b></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Whenever a function marked 'speculative_load_hardening' is inlined into another function, </span><span style="background-color:transparent;color:rgb(0,0,0);font-family:Arial;font-size:11pt;white-space:pre-wrap">then that function that it was inlined into will have SLH enabled no matter what. [If you want </span><span style="background-color:transparent;color:rgb(0,0,0);font-family:Arial;font-size:11pt;white-space:pre-wrap">more info on the rationale for this, check out the patch comments here: </span><a href="https://reviews.llvm.org/D54909?id=175599#inline-487979" style="text-decoration-line:none" target="_blank"><span style="font-size:11pt;font-family:Arial;background-color:transparent;font-variant-numeric:normal;font-variant-east-asian:normal;text-decoration-line:underline;vertical-align:baseline;white-space:pre-wrap">https://reviews.llvm.org/D54909?id=175599#inline-487979</span></a><span style="background-color:transparent;font-variant-numeric:normal;font-variant-east-asian:normal;font-size:11pt;font-family:Arial;color:rgb(0,0,0);vertical-align:baseline;white-space:pre-wrap">]</span></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">
</span></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">I want to diagnose cases similar to this example:</span></p>
<p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><b style="font-weight:normal"><br>
</b></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:"Courier New";color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">__attribute__((speculative_load_hardening)) inline int foo(int i) {</span></p>
</span></div>
</blockquote>
<br>
<p>The "inline" keyword here is basically irrelevant; we aren't
required to inline functions marked inline, and we can inline
functions which are not marked "inline". (The "inline" keyword
has other effects related to linkage, but they don't have any
effect on the arguments here.)</p>
<p>In general, it's impossible to catch all cases of a
no_speculative_load_hardening calling a speculative_load_hardening
function in the frontend: a no_speculative_load_hardening might
call a speculative_load_hardening indirectly, or the callee might
be in a different translation unit. (The optimizer may or may not
discover the call later, depending on the exact construct in
question and the optimization flags.)</p>
<p>Given that, I'm not sure your proposed diagnostic makes sense;
even if you could diagnose constructs like your example, you'd be
missing all the important cases.</p>
<p>Also, if the general rule is "the optimizer may choose to perform
SLH on a function even if it's marked
no_speculative_load_hardening", you should probably avoid
specifically mentioning inlining in the documentation, since it's
misleading. (Other optimizations might be affected by this rule,
like function merging or outlining.)</p>
<blockquote type="cite">
<div dir="ltr"><span id="gmail-m_-7943172813147780136gmail-docs-internal-guid-da7b979b-7fff-e5c3-3332-96e5ad759afc">
<p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><b style="font-weight:normal"><br>
</b></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">2. Add the diagnosis to CheckFunctionCall or checkCall in SemaChecking.cpp</span></p>
<p dir="ltr" style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><b style="font-weight:normal"><br>
</b></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Why doesn't this work?</span></p>
<p dir="ltr" style="line-height:1.656;margin-top:0pt;margin-bottom:0pt;margin-left:18pt;padding:0pt 0pt 0pt 18pt"><span style="font-size:11pt;font-family:Arial;color:rgb(0,0,0);background-color:transparent;font-weight:400;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">The context about the caller of the callee isn't available. I need to know whether the caller has the 'no_speculative_load_hardening' attribute.</span></p>
</span></div>
</blockquote>
<p>The context should be available; you can call
Sema::getEnclosingFunction(), or something like that.<br>
</p>
<p>-Eli<br>
</p>
<pre class="gmail-m_-7943172813147780136moz-signature" cols="72">--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project</pre>
</div>
</blockquote></div>