This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] [gdb/testsuite] include a use of the definition of a type to cause clang to emit debug info


On Wed, Apr 23, 2014 at 5:29 PM, Doug Evans <dje@google.com> wrote:
> On Wed, Apr 23, 2014 at 5:20 PM, Doug Evans <dje@google.com> wrote:
>> David Blaikie writes:
>>  > Clang has an optimization that causes a the debug info to only include
>>  > the declaration of a type if the type is referenced but never used in
>>  > a context that requires a definition (eg: pointers are handed around
>>  > but never deferenced).
>>  >
>>  > This patch introduces a variable to one test file to cause clang to
>>  > emit the full definition of the type as well as fixing up a related
>>  > typo in the test message of the associated expect file.
>>  >
>>  > Like the difference between GCC and Clang in the emission of unused
>>  > static entities, I think this case is also a matter of degrees - both
>>  > GCC and Clang implement other similar optimizations* to the one
>>  > outlined here and the GDB test suite has managed to pass without
>>  > disabling those optimizations in GCC and I hope it's suitable to do
>>  > the same for Clang.
>>  >
>>  > Though admittedly I don't have much of the context of the history of
>>  > the testsuite, its priorities/preferences when it comes to
>>  > distinguishing testing compiler behavior versus debugger behavior,
>>  > etc.
>>  >
>>  > * the one I know of involves dynamic types: both GCC and Clang only
>>  > emit the debug info definition of such a type in any translation unit
>>  > that emits the key function. This means in many contexts where a full
>>  > definition is provided in the source only a declaration is provided in
>>  > the debug info.
>>  > commit 1128f6fb45483d45668d09e0696f4a590334e0c4
>>  > Author: David Blaikie <dblaikie@gmail.com>
>>  > Date:   Sat Apr 12 23:27:19 2014 -0700
>>  >
>>  >     Cause clang to emit the definition of a type used only by pointer
>>  >
>>  >     gdb/testsuite/
>>  >      * gdb.stabs/gdb11479.c: introduce a variable to cause clang to
>>  >      emit the full definition of type required by the test
>>  >      * gdb.stabs/gdb11479.exp: correct a typo in a test message
>>
>> ChangeLog conventions require one to document more specifically
>> where the change occurred.  E.g.,
>>
>>         * gdb.stabs/gdb11479.c (tag_dummy_enum): New global to cause clang to
>>         emit the full definition of type required by the test.
>>         * gdb.stabs/gdb11479.exp (do_test): Correct a typo in a test message.
>>
>> Plus note the capitalization and period.

Hmm, hopefully I got most of those edits right. Sorry if I missed some.

>> Plus conventions also say to specify the "why" of things in source
>> and not the changelog.

Makes sense.

>> I realize we're not going to the trouble
>> of adding comments to every non-static function to document why it
>> has to be non-static.  So I don't see a great need to do so here,
>> and I'd leave the ChangeLog entry as written.
>> I'm just writing this down in case the topic comes up.

Yep - for anything particularly non-obvious, I'll try to keep that in mind.

>>
>>  >
>>  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
>>  > index 730c116..07ba18e 100644
>>  > --- gdb/testsuite/ChangeLog
>>  > +++ gdb/testsuite/ChangeLog
>>  > @@ -1,3 +1,9 @@
>>  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
>>  > +
>>  > +        * gdb.stabs/gdb11479.c: introduce a variable to cause clang to
>>  > +    emit the full definition of type required by the test
>>  > +        * gdb.stabs/gdb11479.exp: correct a typo in a test message
>>  > +
>>
>> Mix of tabs and spaces.  Just use tabs.
>>
>> Ok with those changes.

Committed in 22842ff63e28b86e0cd40a87186757b2525578f4 and
22842ff63e28b86e0cd40a87186757b2525578f4

>
> Bleah.  Sorry Joel.  I didn't see your earlier mail.
>
> What do you think of adding a testcase that explicitly tests the
> user's expectation?

I've attached an example of such a test.

My thoughts are that this isn't so much a matter of user expectations
(I've cited similar behavior in GCC that fires when a type is dynamic
that would be equally surprising to a user, I imagine - yet there's no
GDB test ensuring that the compiler doesn't do that), nor of bugs
(I've provided several patches that workaround GCC bugs and make the
test suite pass when the bug can be worked around to allow the tests
to continue testing the GDB behavior they were intended to). Also this
seems to be a compiler test - a debugger's codepaths can be fully
exercised by providing an example with a declaration and an example
with a definition, regardless of how the source was written. A
compiler test would then ensure that the definition is provided in the
cases where the compiler developers desire it to be emitted, it's
redundant to test the debugger with N different ways the compiler
produces the same debug info.

But I'm still getting a feel for what this testsuite is used for, who
the owners/stakeholders are, what their priorities/goals are, etc.
Just some thoughts.

> [per Pedro's suggestion]

Attachment: unlimited.diff
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]