[patch] migrating DragonEgg to DIBuilder

[Duncan Sands]

Hi David,

On 18/02/13 09:23, David Blaikie wrote:
> In the interests of centralizing the debug info metadata generation in one place
> (DIBuilder) the following patches (to DragonEgg, LLVM (to add a minor breaking
> API change), & Clang (to update for the breaking change) - any non-interesting
> API changes necessary have already been committed).
>
> I'll be the first to admit that this is a fairly mechanical transformation,
> simply replacing the raw metadata generation inside DragonEgg's DIFactory with a
> nested DIBuilder. With further consideration it would be quite easy to remove
> DIFactory entirely & use DIBuilder directly. This would most obviously tidy up
> the switches in DIFactory::CreateCompositeType and DIFactory::CreateDerivedType.
>
> Beyond that there's a couple of finer points:
> 1) DIBuilder needed to be "finalized" - I chose to put that in DIFactory's dtor
> and add code to clean up the DIFactory at the appropriate point
> 2) there's one case of raw MDNode creation for debug info metadata - see the
> comment in the DragonEgg patch starting "FIXME: Remove this conditional". If you
> remove it you can see the failure with a test case involving a
> pointer-to-member-variable typedef. (it seems the call to getOrCreateType at
> line 953 maybe needs to look through pointer-to-member-variable & create the
> type of the member being pointed to - that was a bit more involved that I was
> comfortable fixing, but fix welcome)
>
> [side note: currently DragonEgg seems to produce all record types as "struct"
> debug info, never using "class" debug info - I'm sure this would account for
> several failures in the GDB test suite & some minor usability confusion for users
>
> There is one test I'm still regressing (& a few that have improved with this
> patch) in the GDB 7.5 test suite: gdb.opt/inline-locals.exp, but I think that's
> just luck. It was mostly failing already, for slightly different reasons
> previously.]

thanks so much for doing this!  The patch looks good, however it does cause some
test suite failures:

- dragonegg/test/validator/objc/2010-02-23-DbgInheritance.m
   This objc test fails because it doesn't want DW_TAG_member, but gets it:

!12 = metadata !{i32 786445, metadata !8, metadata !"", metadata !4, i32 8, i64 
0, i64 8, i64 0, i32 0, metadata !11} ; [ DW_TAG_member ] [line 8, size 0, align 
8, offset 0] [from P]

Do you have any thoughts on this?

- some Fortran testcases crash, but it's not your fault: it occurs with
pointers to arrays, due to hitting this:

   if (isa<ARRAY_TYPE>(type) && TYPE_STRING_FLAG(type) &&
       isa<INTEGER_TYPE>(TREE_TYPE(type))) {
     DEBUGASSERT(0 && "Don't support pascal strings");
     return DIType();
   }

Then the empty DIType is used by the pointer code to try and form a
derived type.  I'm not sure why this (ancient) debug code thinks there
is a problem with this kind of array, so I will try to fix it to handle
them properly.

Ciao, Duncan.