[LLVMdev] Issues with clang-llvm debug info validity

[Robinson, Paul]

> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
> Behalf Of Adrian Prantl
> 
> On Jul 9, 2014, at 2:15 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> > On Fri, Jun 27, 2014 at 2:19 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >> On Fri, Jun 27, 2014 at 2:15 PM, Adrian Prantl <aprantl at apple.com>
> wrote:
> >>>
> >>>> On Jun 27, 2014, at 1:58 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>>
> >>>> On Fri, Jun 27, 2014 at 1:49 PM, Adrian Prantl <aprantl at apple.com>
> wrote:
> >>>>> I looked into this a little.
> >>>>
> >>>> Thanks for taking a look!
> >>>>
> >>>>> My proposal is this:
> >>>>>
> >>>>> - extend DINamespace with an extra UniqueID filed (analogous to
> DICompositeType)
> >>>>> - add an extra case to DIScope::getRef()
> >>>>> - Let CGDebugInfo create a mangled name for each namespace to use as
> UID
> >>>>> -> the UID of the anonymous namespace is null.
> >>>>>
> >>>>> For example:
> >>>>>
> >>>>> namespace a { namespace b {} } -> "_ZN1a", "_ZN1a1b"
> >>>>>
> >>>>> !0 = [compile unit]
> >>>>> !1 = metadata !{i32 786489, metadata !0, metadata !"_ZN1a", metadata
> !"b", i32 1, metadata !"_ZN1a1b"} ; [ DW_TAG_namespace ] [a]
> >>>>> !2 = metadata !{i32 786489, metadata !0, null, metadata !"a", i32 1,
> metadata !"_ZN1a"} ; [ DW_TAG_namespace ] [a] [line 1]
> >>>>>
> >>>>> namespace a1b {} -> "_ZN3a1b"
> >>>>> namespace {} -> null (!= "") // Anonymous namespaces will remain
> old-fashioned MDNode references to prevent uniquing.
> >>>>
> >>>> Will this do the right thing if there's a named namespace inside or
> >>>> outside the anonymous namespace?
> >>>
> >>> This is actually really tricky. Normally the ValueSymbolTable takes
> care of adding a unique suffix to all conflicting symbols. i think the
> most reasonable thing to do here is to add a rule:
> >>> -> the UID of any namespace that has an anonymous namespace in its
> parent chain is null.
> >>
> >> Yep - I assume that's the right thing to do, just not sure if it
> >> would/will require a little extra work & wanted to make sure it was on
> >> the radar.
> >>
> >>>>> With this change we will get the full type uniquing benefits. As a
> side effect, definitions that are in the same namespace will be children
> of the same DW_TAG_namespace in the debug info, which I'd argue is the
> expected behavior for LTO anyway.
> >>>>
> >>>> Sort of - what happens to functions defined in that namespace? Will
> >>>> they all end up in one CU with one instance of the namespace under
> >>>> LTO? Could do weird things to address ranges? (would we end up with
> >>>> the address range in the CU the function was originally in - but the
> >>>> subprogram DIE ends up in some other CU that doesn't include that
> >>>> address range)
> >>>
> >>> I think you just convinced me to save this for later and clone a
> namespace for every compile unit instead. The decl_file/decl_line will
> have to identical for all CUs though (and come from the module linked in
> first).
> >>
> >> OK - so we'll put one namespace in the metadata and just use that for
> >> each CU? Should be easy enough.
> >>
> >> One issue: what keeps the namespace metadata nodes live? Where do we
> >> find them? Are we going to add another list to the CUs?
> >
> > Alternatively... does anyone really care about the source location of
> > a namespace? (given that we have to pick one arbitrary location that a
> > namespace starts at?) Should we just omit it, then namespace nodes
> > would deduplicate simply.
> 
> At least for C++ the source location for a namespace is arbitrary and not
> very useful. We would have to do something with the context field though
> (the second one): for a top-level namespace it currently points to the
> compile unit, which would counter any uniquing attempts.
> 
> -- Adrian

Source location is useless on namespace entries, it would save a small amount
of space to nuke it. I'd say go ahead.
--paulr