[patch] migrating DragonEgg to DIBuilder

[David Blaikie]

On Tue, Feb 19, 2013 at 12:25 PM, Duncan Sands <baldrick at free.fr> wrote:

> 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?
>

Hmm, I'll have to admit to having only run the GDB 7.5 test suite, not the
DragonEgg tests.

Trying now, I get this error:

aking DragonEgg 'test/dragonegg-lit.site.cfg' file...
sed: file lit.tmp line 3: unknown option to `s'
make: *** [test/dragonegg-lit.site.cfg] Error 1

do you have any idea about that?

(running "make check-validator" again produces a different error:

Running test suite 'validator'
Traceback (most recent call last):
  File "/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit.py", line
5, in <module>
    lit.main()
  File "/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit/main.py",
line 280, in main
    tests = lit.discovery.find_tests_for_inputs(litConfig, inputs)
  File
"/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit/discovery.py",
line 201, in find_tests_for_inputs
    test_suite_cache, local_config_cache)[1])
  File
"/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit/discovery.py",
line 101, in getTests
    ts,path_in_suite = getTestSuite(path, litConfig, testSuiteCache)
  File
"/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit/discovery.py",
line 70, in getTestSuite
    ts, relative = search(item)
  File
"/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit/discovery.py",
line 54, in search
    cache[path] = res = search1(path)
  File
"/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit/discovery.py",
line 45, in search1
    cfg = TestingConfig.frompath(cfgpath, None, litConfig, mustExist = True)
  File
"/usr/local/google/home/blaikie/dev/llvm/src/utils/lit/lit/TestingConfig.py",
line 61, in frompath
    exec f in cfg_globals
  File
"/usr/local/google/home/blaikie/dev/dragonegg/src/test/validator/validator-lit.cfg",
line 26, in <module>
    path = os.path.pathsep.join((config.llvm_tools_dir,
config.environment['PATH']))
AttributeError: TestingConfig instance has no attribute 'llvm_tools_dir'
make: *** [check-validator] Error 1
)

For the record, I'm running this like:

GCC=$HOME/install/bin/gcc-4.7
CC=$HOME/dev/llvm/install/clang/debug/bin/clang
CXX=$HOME/dev/llvm/install/clang/debug/bin/clang++  make check-validator


>
> - 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.


Thanks muchly,

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130219/8a729de5/attachment.html>