From patchwork Fri Apr 25 05:32:29 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Blaikie X-Patchwork-Id: 675 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id C6F153606E1 for ; Thu, 24 Apr 2014 22:32:36 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14314964) id 761F04F90312; Thu, 24 Apr 2014 22:32:36 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 3BF4F4F90336 for ; Thu, 24 Apr 2014 22:32:36 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; q=dns; s=default; b= MJv2gmnwAPdnD9cuIEEq90l0kggKvC3rKjUverZiUmxBOiOA3sn8ny84UomgPIU1 8a31dHhYy7dZr4e4mMqud8PDcyRaOX7mjtAM+pNYQiu5pwWSyNpLEfXof1P4pLEQ ru/BEtjg5u+HjO2LrDA2kzpqv3zqiYQBwRt0PyHw3Kc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; s=default; bh=KDs5d 1ZfMbq3CNwcNUpEYxYd5sI=; b=xrFTDvm9v9JA0ZzBewNN/GLBXfgyTqsv6eRxb GOxHRZ3QhdUcryyI0ro22CurnY2uowHMul1UKP5g2LNWT8LBzDYByaI5HCA2Dubl yb6m5m8Cwy9ip7UlEZGKocQYSFiuVAd5mXVgDreSI2qYTiLvlxaOEGbNYIY07QU1 cNSerw= Received: (qmail 14950 invoked by alias); 25 Apr 2014 05:32:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 14937 invoked by uid 89); 25 Apr 2014 05:32:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f170.google.com Received: from mail-vc0-f170.google.com (HELO mail-vc0-f170.google.com) (209.85.220.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 25 Apr 2014 05:32:31 +0000 Received: by mail-vc0-f170.google.com with SMTP id hr9so4256866vcb.1 for ; Thu, 24 Apr 2014 22:32:29 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.221.29.137 with SMTP id ry9mr4937212vcb.6.1398403949447; Thu, 24 Apr 2014 22:32:29 -0700 (PDT) Received: by 10.58.186.176 with HTTP; Thu, 24 Apr 2014 22:32:29 -0700 (PDT) In-Reply-To: References: <21336.22730.235280.1770@ruffy.mtv.corp.google.com> Date: Thu, 24 Apr 2014 22:32:29 -0700 Message-ID: Subject: Re: [patch] [gdb/testsuite] include a use of the definition of a type to cause clang to emit debug info From: David Blaikie To: Doug Evans Cc: Joel Brobecker , Pedro Alves , gdb-patches X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in On Wed, Apr 23, 2014 at 5:29 PM, Doug Evans wrote: > On Wed, Apr 23, 2014 at 5:20 PM, Doug Evans 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 >> > 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 >> > + >> > + * 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 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 --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 + * 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 + * 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 . + +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"