[PATCH] Output debug information for structure members only referenced by a pointer or typedef

David Blaikie dblaikie at gmail.com
Thu Jun 20 14:17:24 PDT 2013 

On Thu, Jun 20, 2013 at 4:48 AM, Keith Walker <Keith.Walker at arm.com> wrote:
>> On Wed, Jun 19, 2013 at 5:34 AM, Keith Walker <keith.walker at arm.com>
>> 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.
>

More information about the cfe-commits mailing list