[patch] migrating DragonEgg to DIBuilder
David Blaikie
dblaikie at gmail.com
Sun Feb 24 17:20:15 PST 2013
On Sun, Feb 24, 2013 at 2:53 PM, Eric Christopher <echristo at gmail.com>wrote:
> Hi Dave,
>
> Looks pretty good, just a few comments:
>
> - DW_TAG_inheritance, findRegion(TYPE_CONTEXT(type)),
> + DW_TAG_inheritance, findRegion(type),
>
> This seems curious?
>
Yeah, seems DragonEgg was creating DW_TAG_inheritance metadata that made
basically no sense. Given a simple source file:
struct base {
};
struct derived: base {
};
int main() {
derived d;
}
The (abbreviated) old IR looks like this:
...
!1 = metadata !{i32 655401, metadata !"inherit.cpp", metadata
!"/usr/local/google/home/blaikie/dev/cte/build/", metadata !2}
...
!9 = metadata !{i32 655388, metadata !1, metadata !"", null, i32 0, i64 0,
i64 0, i64 0, i32 0, metadata !10}
!10 = metadata !{i32 655379, metadata !1, metadata !"base", metadata !1,
i32 1, i64 8, i64 8, i64 0, i32 0, null, metadata !11, i32 0, null}
Which basically says "DICompositeType 'base' derives from DIFile
'inherit.cpp'" which doesn't seem particularly useful/correct.
Once ported to DIBuilder this caused assertion failures because the DIType
passed to createInheritance was really a DIFile & failed the 'is valid'
check in createInheritance.
With the change the debug info looks like:
...
!9 = metadata !{i32 786451, metadata !4, metadata !"derived", metadata !4,
i32 4, i64 8, i64 8, i32 0, i32 0, null, metadata !10, i32 0, null, null} ;
[ DW_TAG_structure_type ] [derived] [line 4, size 8, align 8, offset 0]
[from ]
...
!11 = metadata !{i32 786460, metadata !9, null, null, i32 0, i64 0, i64 0,
i64 0, i32 0, metadata !12} ; [ DW_TAG_inheritance ] [line 0, size 0, align
0, offset 0] [from base]
!12 = metadata !{i32 786451, metadata !4, metadata !"base", metadata !4,
i32 1, i64 8, i64 8, i32 0, i32 0, null, metadata !1, i32 0, null, null} ;
[ DW_TAG_structure_type ] [base] [line 1, size 8, align 8, offset 0] [from ]
"derived is derived from base" - hooray
>
> + case dwarf::DW_TAG_typedef:
> + // FIXME: Remove this conditional, it's firing at least for member
> pointer
> + // fix getOrCreateType to see through member data pointers
>
> Can you expand on this comment (also needs a period) since it's not
> obvious what's going on and why?
>
More detail provided:
// FIXME: This code should use DIBuilder but it's creating something
// invalid that DIBuilder doesn't allow. DerivedFrom is invalid due to
// getOrCreateType failing to provide the underlying member type for a
// pointer to member. The invalid DerivedType causes this code to create
// an invalid typedef that is a typedef of no other type. (hence the
null
// value as the last parameter in Elts, below)
Let me know if that's unclear/needs further detail.
>
> + case dwarf::DW_TAG_structure_type:
> + return Builder.createStructType(Context, Name, F, LineNumber,
> SizeInBits, AlignInBits, Flags, DerivedFrom, Elements, 0, ContainingType);
>
> 80-col?
>
Wrapped.
>
> + return Builder.createFunction(
> + SP.getContext(), SP.getName(), SP.getLinkageName(), SP.getFile(),
> + SP.getLineNumber(), SP.getType(), SP.isLocalToUnit(), true, LineNo,
> + SP.getFlags(), SP.isOptimized(), Fn, SP.getTemplateParams(), SP);
>
> Awkward formatting?
>
> + DICompositeType createStructType(
> + DIDescriptor Scope, StringRef Name, DIFile File, unsigned
> LineNumber,
> + uint64_t SizeInBits, uint64_t AlignInBits, unsigned Flags,
> + DIType DerivedFrom, DIArray Elements, unsigned RunTimeLang = 0,
> + MDNode *VTableHolder = 0);
>
> Ditto.
>
> +DICompositeType DIBuilder::createStructType(
> + DIDescriptor Context, StringRef Name, DIFile File, unsigned
> LineNumber,
> + uint64_t SizeInBits, uint64_t AlignInBits, unsigned Flags,
> + DIType DerivedFrom, DIArray Elements, unsigned RunTimeLang,
> + MDNode *VTableHolder) {
>
> Here too.
>
Formatting all made consistent with the previous style (wrapped/aligned to
the '(').
>
> Otherwise OK.
>
Thanks - committed in 176003 (Clang), 176004 (LLVM), and r176006
(DragonEgg).
Thanks all,
- David
>
> -eric
>
>
> On Sun, Feb 24, 2013 at 10:25 AM, David Blaikie <dblaikie at gmail.com>wrote:
>
>>
>>
>>
>> On Sun, Feb 24, 2013 at 12:23 AM, Duncan Sands <baldrick at free.fr> wrote:
>>
>>> Hi David, here is the original commit.
>>
>>
>> Ah, I see - you inherited these test cases from llvm-gcc. Makes sense.
>>
>>
>>> Since dragonegg just uses the provided
>>> gcc (i.e. it can't modify gcc), if gcc hasn't set DECL_IGNORED_P then
>>> there is
>>> nothing that dragonegg can do about it. I've removed the test case.
>>> Please
>>> feel free to apply your debug info patches.
>>>
>>
>> Great - thanks for your time/thoughts/fixes here. I know Eric mentioned
>> he had some thoughts on these patches so I'll wait for him to have a chance
>> & then commit these & carry on with my debug info refactoring work. (in
>> doing so I'll try to keep an eye on the dragonegg test cases too - though
>> I apologize in advance if a few breaks slip through & please do let me know
>> if that happens)
>>
>> - David
>>
>>
>>>
>>> Ciao, Duncan.
>>>
>>> r96974 | dpatel | 2010-02-23 20:36:49 +0100 (Tue, 23 Feb 2010) | 4 lines
>>>
>>> Do not rely on write_symbols to disable debug info for super class added
>>> as an invisible member of derived class. write_symbols controls debug_hooks
>>> which are used to emit debug info in various formats in gcc. llvm-gcc does
>>> not use this debug_hooks interface to emit debugging information.
>>>
>>> Test case is at llvm/test/FrontendObjC/2010-**02-23-DbgInheritance.m
>>>
>>>
>>> Index: gcc/toplev.c
>>> ==============================**==============================**=======
>>> --- gcc/toplev.c (revision 96973)
>>> +++ gcc/toplev.c (revision 96974)
>>> @@ -1926,6 +1926,8 @@
>>>
>>> /* LLVM LOCAL begin */
>>> #ifdef ENABLE_LLVM
>>> + // write_symbols set up debug_hooks. llvm-gcc does not use this hooks
>>> + // to emit debug info.
>>> write_symbols = NO_DEBUG;
>>> #endif
>>> /* LLVM LOCAL end */
>>> Index: gcc/objc/objc-act.c
>>> ==============================**==============================**=======
>>> --- gcc/objc/objc-act.c (revision 96973)
>>> +++ gcc/objc/objc-act.c (revision 96974)
>>> @@ -3407,8 +3407,10 @@
>>> DECL_ALIGN (base) = 1;
>>> DECL_FIELD_CONTEXT (base) = s;
>>> /* APPLE LOCAL begin radar 4477797 */
>>> - if (write_symbols == DWARF2_DEBUG)
>>> - DECL_IGNORED_P (base) = 1;
>>> + /* LLVM LOCAL begin */
>>> + /* Do not check write-symbols in llvm-gcc. */
>>> + DECL_IGNORED_P (base) = 1;
>>> + /* LLVM LOCAL end */
>>> /* APPLE LOCAL end radar 4477797 */
>>> #ifdef OBJCPLUS
>>> DECL_FIELD_IS_BASE (base) = 1;
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130224/89c82d92/attachment.html>
More information about the llvm-commits
mailing list