diff mbox

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

Message ID CAENS6EsokHUzO+5dvsQmxdOLqeZL_3T5+o9aeRANeHFzJaPQ5Q@mail.gmail.com
State New
Headers show

Commit Message

David Blaikie April 25, 2014, 5:32 a.m. UTC
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]
commit 36d650aa2b2fa688c049ba73b4a8fc26a0b6742b
Author: David Blaikie <dblaikie@gmail.com>
Date:   Thu Apr 24 22:02:40 2014 -0700

    Provide a test for GCC's default and part of Clang's -fstandalone-debug behavior.
    
    gdb/testsuite/
    	* gdb.base/pointer-to-def.exp: Verify that the compiler produces a
    	definition for a type only used via pointer but with a definition
    	available. Use Clang's -fstandalone-debug when running with clang to
    	force this emission.
    	* gdb.base/pointer-to-def.cc: Provides a defined type and a variable of
    	pointer to that type.
diff mbox

Patch

diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
index c028cd5..78c3164 100644
--- gdb/testsuite/ChangeLog
+++ gdb/testsuite/ChangeLog
@@ -1,5 +1,14 @@ 
 2014-04-24  David Blaikie  <dblaikie@gmail.com>
 
+	* gdb.base/pointer-to-def.exp: Verify that the compiler produces a
+	definition for a type only used via pointer but with a definition
+	available. Use Clang's -fstandalone-debug when running with clang to
+	force this emission.
+	* gdb.base/pointer-to-def.cc: Provides a defined type and a variable of
+	pointer to that type.
+
+2014-04-24  David Blaikie  <dblaikie@gmail.com>
+
 	* gdb.stabs/gdb11479.c (tag_dummy_enum): introduce a variable 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
diff --git gdb/testsuite/gdb.cp/pointer-to-def.cc gdb/testsuite/gdb.cp/pointer-to-def.cc
new file mode 100644
index 0000000..f6499b9
--- /dev/null
+++ gdb/testsuite/gdb.cp/pointer-to-def.cc
@@ -0,0 +1,8 @@ 
+struct foo {
+  int i;
+};
+
+foo *f;
+
+int main() {
+}
diff --git gdb/testsuite/gdb.cp/pointer-to-def.exp gdb/testsuite/gdb.cp/pointer-to-def.exp
new file mode 100644
index 0000000..2c51827
--- /dev/null
+++ gdb/testsuite/gdb.cp/pointer-to-def.exp
@@ -0,0 +1,35 @@ 
+# Copyright 2010-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if { [skip_cplus_tests] } { continue }
+
+if { [get_compiler_info] } {
+    return -1
+}
+
+set additional_flags ""
+
+if { [test_compiler_info {clang-*-*}] } {
+  set additional_flags "-fstandalone-debug"
+}
+
+standard_testfile .cc
+
+if { [prepare_for_testing ${testfile}.exp $testfile ${srcfile} "debug additional_flags=$additional_flags"] } {
+    untested pointer-to-def.exp
+    return -1
+}
+
+gdb_test "ptype f" "type = struct foo {\r\n    int i;\r\n} \\*" "definition of foo"