<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 4, 2015 at 4:46 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 2015 Jun 4, at 16:36, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Thu, Jun 4, 2015 at 4:28 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
><br>
> > On 2015 Jun 4, at 16:09, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> ><br>
> > Judging by the code, does this cause us not to create a variable if it has no locations? That's probably not quite right - even if we have no locations, it's probably a good idea/correct to describe the variable so shadowing, etc, works correctly (if someone tries to print the variable the compiler still finds the right variable but just reports that it doesn't know the location)<br>
><br>
> So the code here already has an early return if there are no ranges:<br>
><br>
>     for (const auto &I : DbgValues) {<br>
>       InlinedVariable IV = I.first;<br>
>       if (Processed.count(IV))<br>
>         continue;<br>
><br>
>       // Instruction ranges, specifying where IV is accessible.<br>
>       const auto &Ranges = I.second;<br>
>       if (Ranges.empty())<br>
>         continue;<br>
><br>
> My patch effectively hits this `continue` more often.  The difference<br>
> is cases where we *thought* we knew how to emit ranges, but in fact<br>
> didn't know how.<br>
><br>
> I suppose I assumed the intended behaviour was "don't emit the<br>
> variable if we don't have any ranges for it" since that's what was<br>
> going on, but are you saying this `continue` is a bug?  (Or am I<br>
> totally missing your point?)<br>
><br>
> How do we end up with an empty range list here? That seems strange - I assume we're just lazily creating the rang lists in calculateDbgValueHistory... (I could sort of understand empty entries in a range list - when it turns out the dbg_value intrinsic describes no instructions due to things being optimized away, hoisted here or there, etc).<br>
<br>
</span>No idea, I just assume it happens because there's code.<br></blockquote><div><br>Not sure what you mean.<br><br>What I'm trying to understand is whether or not we should be fixing this bug closer to the source (in calculateDbgValueHistory) that's producing empty ranges or empty entries.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">What's the right thing to do? </blockquote><div><br>I don't have all the context here to know that off-hand. Trying to discuss it to try to wrap my head around enough of it to offer a useful opinion - sorry it takes a while.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Should we create a variable the same<br>
was as the for loop below for function arguments (that doesn't<br>
attach ranges)?  Should we not create a variable? </blockquote><div><br>If we get the same output between creating it here or in the later loop, it doesn't matter in the broad sense.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Is it actually<br>
*useful* that we create a .debug_loc list with no entries and point<br>
to it?<br></blockquote><div><br>No, there's no benefit to pointing to an empty loc list compared to just having no DW_AT_location at all.<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
><br>
> > On 2015 Jun 4, at 16:13, Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br>
> ><br>
> > Even worse, when it is a function argument, omitting the variable will effectively change the function signature. (Yes, there is also the subroutine type...).<br>
><br>
> No, there's a separate loop that guarantees all the function arguments<br>
> get created.<br>
><br>
>     // Collect info for variables that were optimized out.<br>
>     for (const DILocalVariable *DV : SP->getVariables()) {<br>
>       if (!Processed.insert(InlinedVariable(DV, nullptr)).second)<br>
>         continue;<br>
>       if (LexicalScope *Scope = LScopes.findLexicalScope(DV->getScope())) {<br>
>         ensureAbstractVariableIsCreatedIfScoped(InlinedVariable(DV, nullptr),<br>
>                                                 Scope->getScopeNode());<br>
>         ConcreteVariables.push_back(make_unique<DbgVariable>(<br>
>             DV, /* IA */ nullptr, /* Expr */ nullptr, this));<br>
>         InfoHolder.addScopeVariable(Scope, ConcreteVariables.back().get());<br>
>       }<br>
>     }<br>
><br>
> I haven't touched this.<br>
<br>
</div></div></blockquote></div><br></div></div>