On Thu, Jun 20, 2013 at 4:48 AM, Keith Walker <[email protected]> wrote:
>> On Wed, Jun 19, 2013 at 5:34 AM, Keith Walker <[email protected]>
>> wrote:
>> > This patch addresses an issue introduced by r183296 that means that
>> the
>> > members of a structure are no longer output in the DWARF debugging
>> > information if the structure is only referenced by a pointer or
>> typedef
>> > (reported in PR16214).
>>
>> Does r184252 address your issue?
>
> Unfortunately no.
>
>> What's the particular regression you're trying to fix related to
>> pointers without typedefs? I'm pretty sure even pre-r183296 a simple
>
> Actually it appears I simplified my example program I was using to
> investigate the problem too much.
>
> The problem is actually to do with a typdef'ed pointer to a structure as
> mentioned in PR16214.
>
> This program when compiled with "-g -O0" demonstrates the problem that the
> member i of struct s in not output (unless you use -fno-limit-debug-info).
>
> -----------------------------------
> #include <stdlib.h>
>
> struct s {
> int i;
> };
>
> typedef struct s *sptr;
>
> int main()
> {
> sptr sp = (sptr)malloc(sizeof(struct s));
> sp->i = 1;
> return sp->i;
> }
> -----------------------------------
>
> Unfortunately during my investigation I had taken one step further and
> simplified it to ....
>
> -----------------------------------
> #include <stdlib.h>
>
> struct s {
> int i;
> };
>
>
> int main()
> {
> struct s *sp = (struct s*)malloc(sizeof(struct s));
> sp->i = 1;
> }
> -----------------------------------
>
> Which generated the same debugging information so I mistakenly thought the
> problem was not due to the typedef.
Right - the fact that the return causes us to emit the full definition
of the type but the sp->i doesn't is an unrelated bug. The answer to
that one is sort of what I was mentioning previously - we should
figure out a way to ensure the full definition is emitted whenever we
see any code that requires the definition. Currently this is done in a
very ad-hoc manner (the particular code that causes this to work in
the presence of the return is in CGExprScalar.cpp:1004 - there are
various similar bits of code scattered throughout CodeGen & they're
nowhere near complete). I've talked to Richard Smith about a more
principled approach where we could add a new ASTConsumer callback that
fires whenever Sema discovers that a complete type is required
(Sema::RequireCompleteType is called for the first time on a type).
But for the particular recent /regression/, the reason that the
original test case failed when a typedef was inserted, is somewhat
annoying - but an obvious problem with my approach. So I changed the
way we create types to specify whether we needed the declaration or
the definition of a type when requesting it (see the 3rd parameter to
CGDebugInfo::getOrCreateType). When we create a typedef & ask for a
"declaration" we just create a declaration of the underlying type -
but the typedef itself is still, technically, done. So next time we go
to retrieve it, even if this time we say we need a definition, we
retrieve the cached typedef type and use that - never pushing further
into it to emit the definition of the nested type.
I'm working on a fix now, just not sure how tidy/general it will be.
>
> However, upon further investigation adding back the return statement so there
> is a read of the member sp->i and hey-presto the expected member information
> is generated.
>
> So actually the problem I was attempting to fix is that which is mentioned it
> PR16214.
>
>> piece of code involving a struct definition and the declaration of a
>> variable that is a pointer to that type would've only emitted the
>> declaration of that type. Did you observe different behavior?
>
> Previously to r183296 the member information was generated .... but I guess
> that is actually because the -flimit-debug-info was not limiting the
> generated information in all cases.
>
>>
>> > Files:
>> > test/CodeGenCXX/debug-info-namespace.cpp
>> > test/CodeGenCXX/debug-info.cpp
>> > lib/CodeGen/CGDebugInfo.cpp
>> >
>> > Index: test/CodeGenCXX/debug-info-namespace.cpp
>> > ===================================================================
>> > --- test/CodeGenCXX/debug-info-namespace.cpp (revision 184291)
>> > +++ test/CodeGenCXX/debug-info-namespace.cpp (working copy)
>> > @@ -57,7 +57,7 @@
>> > // CHECK: [[M6]] = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]],
>> metadata
>> > [[FOO:![0-9]*]], i32 21} ; [ DW_TAG_imported_declaration ]
>> > // CHECK: [[FOO]] {{.*}} ; [ DW_TAG_structure_type ] [foo] [line 5,
>> size 0,
>> > align 0, offset 0] [fwd] [from ]
>> > // CHECK: [[M7]] = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]],
>> metadata
>> > [[BAR:![0-9]*]], i32 22} ; [ DW_TAG_imported_declaration ]
>> > -// CHECK: [[BAR]] {{.*}} ; [ DW_TAG_structure_type ] [bar] [line 6,
>> {{.*}}]
>> > [fwd] [from ]
>> > +// CHECK: [[BAR]] {{.*}} ; [ DW_TAG_structure_type ] [bar] [line 6,
>> {{.*}}]
>> > [from ]
>> > // CHECK: [[M8]] = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]],
>> metadata
>> > [[F1]], i32 23} ; [ DW_TAG_imported_declaration ]
>> > // CHECK: [[M9]] = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]],
>> metadata
>> > [[I]], i32 24} ; [ DW_TAG_imported_declaration ]
>> > // CHECK: [[M10]] = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]],
>> metadata
>> > [[BAZ:![0-9]*]], i32 25} ; [ DW_TAG_imported_declaration ]
>> > Index: test/CodeGenCXX/debug-info.cpp
>> > ===================================================================
>> > --- test/CodeGenCXX/debug-info.cpp (revision 184291)
>> > +++ test/CodeGenCXX/debug-info.cpp (working copy)
>> > @@ -102,18 +102,26 @@
>> > typedef a at;
>> >
>> > struct b {
>> > + int j;
>> > };
>> >
>> > +struct c;
>> > +
>> > typedef b bt;
>> > +typedef c ct;
>> >
>> > void func() {
>> > at a_inst;
>> > bt *b_ptr_inst;
>> > const bt *b_cnst_ptr_inst;
>> > + ct *c_ptr_inst;
>> > }
>> >
>> > // CHECK: metadata [[A_MEM:![0-9]*]], i32 0, null, null} ; [
>> > DW_TAG_structure_type ] [a]
>> > // CHECK: [[A_MEM]] = metadata !{metadata [[A_I:![0-9]*]], metadata
>> > !{{[0-9]*}}}
>> > // CHECK: [[A_I]] = {{.*}} ; [ DW_TAG_member ] [i] {{.*}} [from int]
>> > -// CHECK: ; [ DW_TAG_structure_type ] [b] {{.*}}[fwd]
>> > +// CHECK: metadata [[B_MEM:![0-9]*]], i32 0, null, null} ; [
>> > DW_TAG_structure_type ] [b]
>> > +// CHECK: [[B_MEM]] = metadata !{metadata [[B_J:![0-9]*]]}
>> > +// CHECK: [[B_J]] = {{.*}} ; [ DW_TAG_member ] [j] {{.*}} [from int]
>> > +// CHECK: ; [ DW_TAG_structure_type ] [c] {{.*}}[fwd]
>> > }
>> > Index: lib/CodeGen/CGDebugInfo.cpp
>> > ===================================================================
>> > --- lib/CodeGen/CGDebugInfo.cpp (revision 184291)
>> > +++ lib/CodeGen/CGDebugInfo.cpp (working copy)
>> > @@ -615,7 +615,8 @@
>> > llvm::DIFile
>> Unit) {
>> > if (DebugKind > CodeGenOptions::LimitedDebugInfo)
>> > return getOrCreateType(PointeeTy, Unit);
>> > - return getOrCreateType(PointeeTy, Unit, true);
>> > + bool Declaration = isa<RecordType>(PointeeTy);
>> > + return getOrCreateType(PointeeTy, Unit, Declaration);
>>
>> This is not the right fix - we don't want to always emit definitions
>> for types referenced via pointers or typedefs in all cases.
>>
>> Limited debug info (r184252 probably addresses your issue when
>> building with -fno-limit-debug-info) is intended to reduce debug info
>> by taking steps including emitting only the declaration of types that
>> are only used in ways that a definition would not be needed.
>>
>> In your test case there's no use of the concrete type 'b' - only some
>> pointers to 'b' that are never dereferenced. (the test here is "if a
>> struct definition were replaced with a declaration, would the code
>> still compile" - if that condition is met, then the intent is that
>> Clang emit only a declaration for that type when building with
>> -flimit-debug-info (which is the default))
>
> I hadn't realised -flimit-debug-info was the default ...silly me!
>
>> Does that make sense?
>
> Yes.
>
>> As for how to fix it - we'd have to look at how to plumb in hooks for
>> the debug info to be invoked (to request the full definition of a
>> type) when we visit expressions that require the full definition (eg
>> in Adrian's r184252 the commented line in the test case that
>> dereferences the pointer).
>
> Yes .... this look exactly like the problem that I was wanting to fix ....
> unfortunately it looks like my attempted fix was a sledge hammer approach. :-)
>
>>
>> > }
>> >
>> > llvm::DIType CGDebugInfo::CreatePointerLikeType(unsigned Tag,
>
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
