<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Feb 26, 2015 at 12:46 PM, Romanova, Katya <span dir="ltr"><<a href="mailto:Katya_Romanova@playstation.sony.com" target="_blank">Katya_Romanova@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Our debugger guys argument is  that the debugger just follows the rules laid out by the debug information. If there is no import, the debugger will not import it. There is nothing in the  DWARF spec that clearly states an empty name is a magic import.<br>
<br>
Personally I don't have a strong opinion about this issue. It was very simple to do in the compiler and it hardly increased size of the DWARF info. Apparently, it was much tougher to fix this in the debugger.<br>
<br>
If it is such a nuisance, maybe I could add an option and set it to false to all of the platforms, except PS4?<br>
Or I could follow Fred's suggestion and add DW_AT_artificial attribute?<br>
<span class=""><br>
>> However I don’t think the patch will break any debugger (although that might be worth testing).<br>
<br>
</span>I could manually test GDB.<br></blockquote><div><br>I imagine GDB behaves fine (you're changing the output to what GCC produces) & we have a buildbot to check that sort of thing (it runs clang against the GDB 7.5 test suite).<br><br>I don't imagine this would hurt LLDB, but it might be worth checking.<br><br>Mostly I just vaguely object on principles - Paul Robinson often makes the argument that DWARF is all about source fidelity, and this seems a case where that argument seems quite sound.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Fred, which other debuggers do you suggest to test to make sure that this patch didn't break them? Are there any existing debugger tests in LLVM?<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888">Katya<br>
</font></span><span class="im HOEnZb"><br>
-----Original Message-----<br>
From: Frédéric Riss [mailto:<a href="mailto:friss@apple.com">friss@apple.com</a>]<br>
Sent: Thursday, February 26, 2015 9:46 AM<br>
To: <a href="mailto:reviews%2BD7878%2Bpublic%2B0975bde262e84f91@reviews.llvm.org">reviews+D7878+public+0975bde262e84f91@reviews.llvm.org</a><br>
Cc: Romanova, Katya; <a href="mailto:echristo@gmail.com">echristo@gmail.com</a>; <a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>; <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
Subject: Re: [PATCH] Anonymous namespaces are missing import DW_TAG_imported_module.<br>
<br>
<br>
</span><div class="HOEnZb"><div class="h5">> On Feb 25, 2015, at 2:25 PM, Paul Robinson <<a href="mailto:Paul_Robinson@playstation.sony.com">Paul_Robinson@playstation.sony.com</a>> wrote:<br>
><br>
> Our debugger guys get pretty pedantic about interpreting the DWARF spec. I know GDB will auto-import info from an anonymous namespace, but:<br>
><br>
> - the size cost here is really minimal (what, 5 or 6 bytes per anonymous namespace scope);<br>
> - they claimed that deciding whether to skip the namespace based on the name would be a problem for them;<br>
> - the compiler implementation is obviously trivial;<br>
><br>
> so I didn't really feel motivated to argue the point.<br>
<br>
Basically your debugger guys told you this could be described by standard constructs, so there is no point in adding extra logic to the debugger to handle it. IMHO this is not a very strong argument, because DWARF tries to be language agnostic, and thus every debugger needs to have some layer that adds language semantics to the raw debug information (for example, anonymous unions pose a very similar problem).<br>
<br>
However I don’t think the patch will break any debugger (although that might be worth testing). I know that it will have some impact on Darwin’s dsymutil though, because it considers the targets of every ‘import’ directive as required when it decides which DIEs are needed in the final link. If the patch is going in, I think it would be nice to have a DW_AT_artificial in the importing DIE (which shouldn’t add any size to the debug info expect for one additional abbreviation).<br>
<br>
Fred<br>
<br>
> <a href="http://reviews.llvm.org/D7878" target="_blank">http://reviews.llvm.org/D7878</a><br>
><br>
> EMAIL PREFERENCES<br>
>  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>