[PATCH] Anonymous namespaces are missing import DW_TAG_imported_module.

[Romanova, Katya]

OK. Now it's clear. Thanks!
Katya.

-----Original Message-----
From: Adrian Prantl [mailto:aprantl at apple.com] 
Sent: Tuesday, March 10, 2015 8:48 AM
To: Romanova, Katya
Cc: reviews+D7895+public+7827be49c0b04087 at reviews.llvm.org; Robinson, Paul; echristo at gmail.com; friss at apple.com; dexonsmith at apple.com; dblaikie at gmail.com; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] Anonymous namespaces are missing import DW_TAG_imported_module.


> On Mar 10, 2015, at 12:01 AM, Romanova, Katya <Katya_Romanova at playstation.sony.com> wrote:
> 
> Hi Adrian,
> 
>>> Removing the line/file names would be great, because it would solve 
>>> the LTO bug described in 
>>> http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-June/073801.html
> 
> I could modify my patch to remove line/file names for anonymous namespaces. 
> BTW, was this bug fixed? I can't reproduce this assertion with the latest compiler...

I was working on this when it was discussed on the mailing list, but then got sidetracked by other things and never completed it.
> 
>>> but we need to be careful to not accidentally unique anonymous namespaces as outlined later in that thread.
> 
> How could I make sure that accidental uniquing is not happening? I simply wanted to skip generation of line/file number info for the anonymous namespace. Something like that (see below). Am I missing something?
> 
> -  addSourceLine(NDie, NS);
> +  if (IsAnonymous) {
> +    // For anonymous namespaces, add a DW_TAG_imported_module tag
> +    // containing a DW_AT_import attribute with the reference to this
> +    // namespace entry.
> +    DIE &IDie = createAndAddDIE(dwarf::DW_TAG_imported_module, *ContextDIE);
> +    addDIEEntry(IDie, dwarf::DW_AT_import, NDie);
> +    addFlag(IDie, dwarf::DW_AT_artificial);  }  else
> +    addSourceLine(NDie, NS);
>   return &NDie;

Since the patch in http://reviews.llvm.org/D7895 is only touching the DWARF backend, changing it to not emit line and file information will neither fix the above bug nor cause it to become any worse. The uniquing that the thread is referencing happens earlier at the IR level when multiple LLVM modules are linked together.

The IR representation of debug info has changed in significant ways since that email thread, most notably, uniquing does not longer happen implicitly (previously it used to be a bunch of MDNodes in a FoldingSet). I believe that the main reason why we were including line and file information in the namespaces at all was to prevent accidental uniquing of anonymous namespaces. Line numbers on namespaces are pretty useless otherwise as namespaces can be opened and closed at will.
So rather than dropping the line number at DWARF emission time it should not be recorded in the IR in the first place, but we need to make sure that whatever debug IR uniquing is happening during llvm-link does not unique anonymous namespaces.

I realize that this is out of the scope of your proposed patch, so this shouldn’t hold up the review of this patch.

-- adrian