>>! In D2498#10, @jyoti.yalamanchili wrote:
> Hi David,
>
>>>! In D2498#9, @dblaikie wrote:
>>>>! In D2498#4, @jyoti.yalamanchili wrote:
>>> This bug appears only when typedef is used.
>>
>> Doesn't look like it.
> This was my previous analysis which was incorrect. This issue occurs whenever
> explicit casting occurs. The type to which we are explicitly casting is not
> emitted in debug info.
>>
>> Changing the code to cast to (S*) instead of (T) still demonstrates the bug
>> and is still present even with this patch applied.
>>
> As you mentioned, I verified this for the following code snippet where i am
> replacing T with struct S* and the result was that DW_TAG_structure_type &
> DW_TAG_member were emitted but not DW_TAG_typedef because T was not used. I
> thought this is expected behaviour. Please correct me if i am wrong.
Looks like I was confused (perhaps by the old title of this code review which
still mentions typedefs - and perhaps by Phab showing me the newer diff, or
maybe I was looking in the wrong place somehow and saw the old code which still
special-cased for typedefs). Now that I apply your latest patch properly I see
the behavior you describe - this fixes the issue regardless of whether a
typedef is used.
>
> typedef struct S {
> int i;
> } *T;
> #define M(p) (**(struct S*)**(p))
>
> void foo(void *p) { M(p)->i++; }
>
>
>> The problem is more general than typedefs and casts - arguably we should get
>> this (C++11) case too:
>>
>> struct S { int i; };
>> void func(S);
>> void func() {
>> func(S{42});
>> }
>>
>> but GCC doesn't get this either, so I'm less concerned.
>>
>> GCC does get the cast case - with or without a typedef. So perhaps we can
>> just deal with casts for now...
>>
> Okay.
>
>> The only other thing I can think of is far more invasive/more work - to get
>> this all right, what we really need is a tree of "completeness" - keeping
>> track of any time a type is required to be complete (not just a single bit
>> that says "this has been required to be complete") & then emitting any type
>> for which it was requried to be complete during a function we codegen'd.
>>
> Sorry, could you please explain this with an example ?
struct foo { };
inline void func() { foo f; } // this makes 'f' required to be complete,
which means we emit a definition of 'f' if we ever need any reference to 'f'
foo *f; // this causes us to emit 'f'
Without the second line, we just emit a declaration (in the default
-fno-standalone-debug mode).
> I could mention this in the testcase as a TODO note if you want so we don't
> loose track of this improvement scope.
That's OK - it's pretty much outside the scope of your change. It wouldn't
really belong in this test file.
>
>> This infrastructure would also allow us to do a better job minimizing debug
>> info for the limited debug info optimization where we currently fail:
>>
>> struct foo { ... };
>> inline void unused() { foo f; } // 'f' is required to be complete here,
>> but the function is never called
>> foo *f; // but that required completeness is never relied upon for this TU
>> - a declaration would've sufficed
>>
>> This hurts us in, for example, Clang's own Sema.h - the giant Sema class has
>> a single non-member inline function right after it that has dereferences a
>> Sema pointer, thus requiring Sema to be complete. From then on, if anything
>> else in the TU including Sema.h requires Sema (evern just a pointer to it
>> it) the debug info for the entire class will be emitted.
>
> If i understand this correctly, point of worry here is about the increase in
> debug info size, am i right?
The -fno-standalone-debug default option is an attempt to minimize debug info
size, yes. The issue is that it uses /some/ part of the powers necessary to
make our general debug info strategy more reliable and to fix this bug you're
addressing and possible others in a more general manner. Though I can't site
any particular examples similar to yours (places where a type is used, but no
named instance or reference to the type is ever needed)... actually I can:
struct S { };
void *v = new S;
Clang doesn't emit debug info describing 'S' (but then again, neither does
GCC). I imagine maybe there's a few other pieces of syntax that allow one to
reference a type without ever naming a variable of that type.
> But, we certainly need the debug info of any referenced variables isn't ?
Sure - in this case there was no variable, but it's still reasonable to
describe the type. The rough heuristic is: Any expression written in executing
code (global intializers, functions that are actually codegen'd, etc) should be
able to be written and executed in a sufficiently advanced debugger given the
information Clang produces.
Any cases where the information is fundamentally not there - are bugs.
================
Comment at: llvm/tools/clang/lib/CodeGen/CGExprScalar.cpp:279
@@ +278,3 @@
+ CGDebugInfo *DI = CGF.getDebugInfo();
+ if (DI &&
+ CGF.CGM.getCodeGenOpts().getDebugInfo() >=
----------------
Roll the DI variable into the if condition and sink the LimitedDebugInfo check
into the DI function (at least I think that's how we are/should be doing this -
keeping as much of the implementation details down in CGDebugInfo rather than
up in the CodeGen callsites)
================
Comment at: llvm/tools/clang/test/CodeGen/Debug-info-explicitcast.c:4
@@ +3,3 @@
+typedef struct S { int i; } *T;
+#define M(p) ((T)(p))
+
----------------
Could you drop the macro usage and the typedef as they're not needed to
demonstrate the fundamental bug here?
http://reviews.llvm.org/D2498
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
