From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@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: Fri, 25 Apr 2014 05:32:00 -0000 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 Content-Type: multipart/mixed; boundary=001a1133935ca928b904f7d74a20 X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg00525.txt.bz2 --001a1133935ca928b904f7d74a20 Content-Type: text/plain; charset=UTF-8 Content-length: 4959 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] --001a1133935ca928b904f7d74a20 Content-Type: text/plain; charset=US-ASCII; name="unlimited.diff" Content-Disposition: attachment; filename="unlimited.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_huf1adpx0 Content-length: 4148 Y29tbWl0IDM2ZDY1MGFhMmIyZmE2ODhjMDQ5YmE3M2I0YThmYzI2YTBiNjc0 MmIKQXV0aG9yOiBEYXZpZCBCbGFpa2llIDxkYmxhaWtpZUBnbWFpbC5jb20+ CkRhdGU6ICAgVGh1IEFwciAyNCAyMjowMjo0MCAyMDE0IC0wNzAwCgogICAg UHJvdmlkZSBhIHRlc3QgZm9yIEdDQydzIGRlZmF1bHQgYW5kIHBhcnQgb2Yg Q2xhbmcncyAtZnN0YW5kYWxvbmUtZGVidWcgYmVoYXZpb3IuCiAgICAKICAg IGdkYi90ZXN0c3VpdGUvCiAgICAJKiBnZGIuYmFzZS9wb2ludGVyLXRvLWRl Zi5leHA6IFZlcmlmeSB0aGF0IHRoZSBjb21waWxlciBwcm9kdWNlcyBhCiAg ICAJZGVmaW5pdGlvbiBmb3IgYSB0eXBlIG9ubHkgdXNlZCB2aWEgcG9pbnRl ciBidXQgd2l0aCBhIGRlZmluaXRpb24KICAgIAlhdmFpbGFibGUuIFVzZSBD bGFuZydzIC1mc3RhbmRhbG9uZS1kZWJ1ZyB3aGVuIHJ1bm5pbmcgd2l0aCBj bGFuZyB0bwogICAgCWZvcmNlIHRoaXMgZW1pc3Npb24uCiAgICAJKiBnZGIu YmFzZS9wb2ludGVyLXRvLWRlZi5jYzogUHJvdmlkZXMgYSBkZWZpbmVkIHR5 cGUgYW5kIGEgdmFyaWFibGUgb2YKICAgIAlwb2ludGVyIHRvIHRoYXQgdHlw ZS4KCmRpZmYgLS1naXQgZ2RiL3Rlc3RzdWl0ZS9DaGFuZ2VMb2cgZ2RiL3Rl c3RzdWl0ZS9DaGFuZ2VMb2cKaW5kZXggYzAyOGNkNS4uNzhjMzE2NCAxMDA2 NDQKLS0tIGdkYi90ZXN0c3VpdGUvQ2hhbmdlTG9nCisrKyBnZGIvdGVzdHN1 aXRlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE0IEBACiAyMDE0LTA0LTI0ICBE YXZpZCBCbGFpa2llICA8ZGJsYWlraWVAZ21haWwuY29tPgogCisJKiBnZGIu YmFzZS9wb2ludGVyLXRvLWRlZi5leHA6IFZlcmlmeSB0aGF0IHRoZSBjb21w aWxlciBwcm9kdWNlcyBhCisJZGVmaW5pdGlvbiBmb3IgYSB0eXBlIG9ubHkg dXNlZCB2aWEgcG9pbnRlciBidXQgd2l0aCBhIGRlZmluaXRpb24KKwlhdmFp bGFibGUuIFVzZSBDbGFuZydzIC1mc3RhbmRhbG9uZS1kZWJ1ZyB3aGVuIHJ1 bm5pbmcgd2l0aCBjbGFuZyB0bworCWZvcmNlIHRoaXMgZW1pc3Npb24uCisJ KiBnZGIuYmFzZS9wb2ludGVyLXRvLWRlZi5jYzogUHJvdmlkZXMgYSBkZWZp bmVkIHR5cGUgYW5kIGEgdmFyaWFibGUgb2YKKwlwb2ludGVyIHRvIHRoYXQg dHlwZS4KKworMjAxNC0wNC0yNCAgRGF2aWQgQmxhaWtpZSAgPGRibGFpa2ll QGdtYWlsLmNvbT4KKwogCSogZ2RiLnN0YWJzL2dkYjExNDc5LmMgKHRhZ19k dW1teV9lbnVtKTogaW50cm9kdWNlIGEgdmFyaWFibGUgdG8gY2F1c2UKIAlj bGFuZyB0byBlbWl0IHRoZSBmdWxsIGRlZmluaXRpb24gb2YgdHlwZSByZXF1 aXJlZCBieSB0aGUgdGVzdAogCSogZ2RiLnN0YWJzL2dkYjExNDc5LmV4cCAo ZG9fdGVzdCk6IGNvcnJlY3QgYSB0eXBvIGluIGEgdGVzdCBtZXNzYWdlCmRp ZmYgLS1naXQgZ2RiL3Rlc3RzdWl0ZS9nZGIuY3AvcG9pbnRlci10by1kZWYu Y2MgZ2RiL3Rlc3RzdWl0ZS9nZGIuY3AvcG9pbnRlci10by1kZWYuY2MKbmV3 IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMC4uZjY0OTliOQotLS0g L2Rldi9udWxsCisrKyBnZGIvdGVzdHN1aXRlL2dkYi5jcC9wb2ludGVyLXRv LWRlZi5jYwpAQCAtMCwwICsxLDggQEAKK3N0cnVjdCBmb28geworICBpbnQg aTsKK307CisKK2ZvbyAqZjsKKworaW50IG1haW4oKSB7Cit9CmRpZmYgLS1n aXQgZ2RiL3Rlc3RzdWl0ZS9nZGIuY3AvcG9pbnRlci10by1kZWYuZXhwIGdk Yi90ZXN0c3VpdGUvZ2RiLmNwL3BvaW50ZXItdG8tZGVmLmV4cApuZXcgZmls ZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwLi4yYzUxODI3Ci0tLSAvZGV2 L251bGwKKysrIGdkYi90ZXN0c3VpdGUvZ2RiLmNwL3BvaW50ZXItdG8tZGVm LmV4cApAQCAtMCwwICsxLDM1IEBACisjIENvcHlyaWdodCAyMDEwLTIwMTQg RnJlZSBTb2Z0d2FyZSBGb3VuZGF0aW9uLCBJbmMuCisKKyMgVGhpcyBwcm9n cmFtIGlzIGZyZWUgc29mdHdhcmU7IHlvdSBjYW4gcmVkaXN0cmlidXRlIGl0 IGFuZC9vciBtb2RpZnkKKyMgaXQgdW5kZXIgdGhlIHRlcm1zIG9mIHRoZSBH TlUgR2VuZXJhbCBQdWJsaWMgTGljZW5zZSBhcyBwdWJsaXNoZWQgYnkKKyMg dGhlIEZyZWUgU29mdHdhcmUgRm91bmRhdGlvbjsgZWl0aGVyIHZlcnNpb24g MyBvZiB0aGUgTGljZW5zZSwgb3IKKyMgKGF0IHlvdXIgb3B0aW9uKSBhbnkg bGF0ZXIgdmVyc2lvbi4KKyMKKyMgVGhpcyBwcm9ncmFtIGlzIGRpc3RyaWJ1 dGVkIGluIHRoZSBob3BlIHRoYXQgaXQgd2lsbCBiZSB1c2VmdWwsCisjIGJ1 dCBXSVRIT1VUIEFOWSBXQVJSQU5UWTsgd2l0aG91dCBldmVuIHRoZSBpbXBs aWVkIHdhcnJhbnR5IG9mCisjIE1FUkNIQU5UQUJJTElUWSBvciBGSVRORVNT IEZPUiBBIFBBUlRJQ1VMQVIgUFVSUE9TRS4gIFNlZSB0aGUKKyMgR05VIEdl bmVyYWwgUHVibGljIExpY2Vuc2UgZm9yIG1vcmUgZGV0YWlscy4KKyMKKyMg WW91IHNob3VsZCBoYXZlIHJlY2VpdmVkIGEgY29weSBvZiB0aGUgR05VIEdl bmVyYWwgUHVibGljIExpY2Vuc2UKKyMgYWxvbmcgd2l0aCB0aGlzIHByb2dy YW0uICBJZiBub3QsIHNlZSA8aHR0cDovL3d3dy5nbnUub3JnL2xpY2Vuc2Vz Lz4uCisKK2lmIHsgW3NraXBfY3BsdXNfdGVzdHNdIH0geyBjb250aW51ZSB9 CisKK2lmIHsgW2dldF9jb21waWxlcl9pbmZvXSB9IHsKKyAgICByZXR1cm4g LTEKK30KKworc2V0IGFkZGl0aW9uYWxfZmxhZ3MgIiIKKworaWYgeyBbdGVz dF9jb21waWxlcl9pbmZvIHtjbGFuZy0qLSp9XSB9IHsKKyAgc2V0IGFkZGl0 aW9uYWxfZmxhZ3MgIi1mc3RhbmRhbG9uZS1kZWJ1ZyIKK30KKworc3RhbmRh cmRfdGVzdGZpbGUgLmNjCisKK2lmIHsgW3ByZXBhcmVfZm9yX3Rlc3Rpbmcg JHt0ZXN0ZmlsZX0uZXhwICR0ZXN0ZmlsZSAke3NyY2ZpbGV9ICJkZWJ1ZyBh ZGRpdGlvbmFsX2ZsYWdzPSRhZGRpdGlvbmFsX2ZsYWdzIl0gfSB7CisgICAg dW50ZXN0ZWQgcG9pbnRlci10by1kZWYuZXhwCisgICAgcmV0dXJuIC0xCit9 CisKK2dkYl90ZXN0ICJwdHlwZSBmIiAidHlwZSA9IHN0cnVjdCBmb28ge1xy XG4gICAgaW50IGk7XHJcbn0gXFwqIiAiZGVmaW5pdGlvbiBvZiBmb28iCg== --001a1133935ca928b904f7d74a20--