[PATCH] Anonymous namespaces are missing import DW_TAG_imported_module.

Frédéric Riss friss at apple.com
Thu Feb 26 15:04:39 PST 2015


> On Feb 26, 2015, at 2:39 PM, Robinson, Paul <Paul_Robinson at playstation.sony.com> wrote:
> 
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu <mailto:llvm-commits-bounces at cs.uiuc.edu> [mailto:llvm-commits-
>> bounces at cs.uiuc.edu <mailto:bounces at cs.uiuc.edu>] On Behalf Of Frédéric Riss
>> Sent: Thursday, February 26, 2015 1:43 PM
>> To: Romanova, Katya
>> Cc: llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>;
>> reviews+D7878+public+0975bde262e84f91 at reviews.llvm.org <mailto:reviews+D7878+public+0975bde262e84f91 at reviews.llvm.org>
>> Subject: Re: [PATCH] Anonymous namespaces are missing import
>> DW_TAG_imported_module.
>> 
>> 
>>> On Feb 26, 2015, at 12:46 PM, Romanova, Katya
>> <Katya_Romanova at playstation.sony.com <mailto:Katya_Romanova at playstation.sony.com>> wrote:
>>> 
>>> Hi,
>>> 
>>> 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.
>> 
>> There is nothing in the dwarf spec that specifies that namespaces should
>> be separated by ‘::’ in qualified names, yet the debugger has to know it
>> :-) 
> 
> Only if the debugger requires you to manually enter qualified identifiers
> in imitation C++ syntax.  Some do, but I've also used a GUI drill-down 
> interface where separator syntax was totally irrelevant.
> 
> COBOL (he said, waxing reminiscent at the slightest provocation) lets
> you omit the names of parent structure levels as long as the final name
> is unambiguous. (And if some omitted level is an array, no problem--just
> tack the subscript on wherever it's convenient.)  This is a major pain
> for a command-line oriented debugger used to less forgiving languages like 
> C++. ;-)  But with a GUI, people kind of expected to have to drill down 
> every level, which had the advantage that the debugger didn't have to be 
> as language-sensitive.
> 
> [I used to own the COBOL compiler for HP NonStop, and the debugger had
> a GUI client.  Just to fill in the context there.]
> 
> So (returning to relevance) encoding that particular bit of C++ oddness
> into a DWARF directive makes a certain amount of sense to me.

“makes a certain amount of sense” is exactly how I’m feeling. We are totally on the same page here :-)

> And, as David points out and I had forgotten, GCC does it that way too.

Oh, I had misread his comment. I thought he meant that we diverged from GCC’s behavior. That’s a good point in favor of doing it then!

>> My point simply was that the debugger has to have some knowledge about
>> language semantics, and all the debuggers we know about already get this
>> right without help from the compiler. 
> 
> So, now your knowledge of debuggers has increased? :-)

I actually know quite a lot about debuggers (GDB and 2 other commercial codebases), but I know nothing about Sony’s one.

>> We could also emit a DW_AT_name
>> pointing to “(anonymous namespace)”, but we don’t and debuggers synthesize
>> this themselves. There is a line somewhere between the language semantics
>> encoded in the Dwarf and those implicitly known by the debugger. I guess
>> this patch is near this line, I can’t decide on which side it stands.
>> 
>>> 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.
>>> 
>>> If it is such a nuisance, maybe I could add an option and set it to
>> false to all of the platforms, except PS4?
>> 
>> The added complexity wouldn’t make sense IMHO.
>> 
>>> Or I could follow Fred's suggestion and add DW_AT_artificial attribute?
>> 
>> Well, there is no ‘using’ or anything in the source, thus adding a
>> DW_AT_artificial seems sound to me.
> 
> Yes, DW_AT_artificial on the DW_TAG_imported_module would be correct here.

glad we agree.

Fred

> --paulr
> 
>> 
>>>>> However I don’t think the patch will break any debugger (although that
>> might be worth testing).
>>> 
>>> I could manually test GDB.
>>> 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?
>> 
>> Not that I expect any issue, but we care about lldb and gdb.
>> 
>> Fred.
>> 
>>> Thanks!
>>> Katya
>>> 
>>> -----Original Message-----
>>> From: Frédéric Riss [mailto:friss at apple.com]
>>> Sent: Thursday, February 26, 2015 9:46 AM
>>> To: reviews+D7878+public+0975bde262e84f91 at reviews.llvm.org
>>> Cc: Romanova, Katya; echristo at gmail.com; dblaikie at gmail.com; llvm-
>> commits at cs.uiuc.edu
>>> Subject: Re: [PATCH] Anonymous namespaces are missing import
>> DW_TAG_imported_module.
>>> 
>>> 
>>>> On Feb 25, 2015, at 2:25 PM, Paul Robinson
>> <Paul_Robinson at playstation.sony.com> wrote:
>>>> 
>>>> Our debugger guys get pretty pedantic about interpreting the DWARF
>> spec. I know GDB will auto-import info from an anonymous namespace, but:
>>>> 
>>>> - the size cost here is really minimal (what, 5 or 6 bytes per
>> anonymous namespace scope);
>>>> - they claimed that deciding whether to skip the namespace based on the
>> name would be a problem for them;
>>>> - the compiler implementation is obviously trivial;
>>>> 
>>>> so I didn't really feel motivated to argue the point.
>>> 
>>> 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).
>>> 
>>> 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).
>>> 
>>> Fred
>>> 
>>>> http://reviews.llvm.org/D7878
>>>> 
>>>> EMAIL PREFERENCES
>>>> http://reviews.llvm.org/settings/panel/emailpreferences/
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150226/bbf130c0/attachment.html>


More information about the llvm-commits mailing list