From patchwork Tue Jul 1 18:11:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Wielaard X-Patchwork-Id: 1842 Received: (qmail 26306 invoked by alias); 1 Jul 2014 18:11:57 -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 26225 invoked by uid 89); 1 Jul 2014 18:11:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.7 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 01 Jul 2014 18:11:49 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s61IBmJG007914 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 1 Jul 2014 14:11:48 -0400 Received: from [10.36.116.113] (ovpn-116-113.ams2.redhat.com [10.36.116.113]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s61IBkh4010137; Tue, 1 Jul 2014 14:11:47 -0400 Subject: Re: [PATCH] Handle volatile array types in dwarf2read.c. From: Mark Wielaard To: Tom Tromey Cc: gdb-patches@sourceware.org In-Reply-To: <87simlrroa.fsf@fleche.redhat.com> References: <1404164071-24432-1-git-send-email-mjw@redhat.com> <87simlrroa.fsf@fleche.redhat.com> Date: Tue, 01 Jul 2014 20:11:45 +0200 Message-ID: <1404238306.3766.17.camel@bordewijk.wildebeest.org> Mime-Version: 1.0 On Tue, 2014-07-01 at 10:04 -0600, Tom Tromey wrote: > Mark> + the cv-qualifiers. This is mimics section 6.7.3 of the C99 > Mark> + specification. */ > > "This is" -> "This". Fixed. > Mark> +static struct type * > Mark> +add_array_cv_type (struct die_info *die, struct dwarf2_cu *cu, > Mark> + struct type *base_type, int cnst, int voltl) > > gdb usually puts a blank line between the intro comment and the function > head. Fixed. > Mark> + while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY) > Mark> + { > Mark> + TYPE_TARGET_TYPE (inner_array) = > Mark> + copy_type (TYPE_TARGET_TYPE (inner_array)); > Mark> + inner_array = TYPE_TARGET_TYPE (inner_array); > Mark> + } > > I think this may be overly eager in the case where the underlying type > already has the needed qualifications. But we have to look anyway. This is just the existing code, moved into its own function. What do you suggest should be changed? > Mark> + cnst |= TYPE_CONST (el_type); > Mark> + voltl |= TYPE_VOLATILE (el_type); > Mark> + TYPE_TARGET_TYPE (inner_array) = make_cv_type (cnst, voltl, el_type, NULL); > > make_cv_type preserves the already existing qualifiers so you don't need > to track "cnst" and "voltl". You can just pass in the ones you want to > add. The original code already did it this way. And I don't think it is true that make_cv_type preserves existing const or volatile qualifiers. The first thing make_cv_type does is mask away the old const and volatile flags. Without explicitly preserving the existing flags 2 of the new testcases, the const volatile arrays tests, fail (note one of the qualifiers is missing): ptype vindictive type = const char [2] (gdb) FAIL: gdb.base/volatile.exp: ptype vindictive ptype vegetation type = const unsigned char [2] (gdb) FAIL: gdb.base/volatile.exp: ptype vegetation > Mark> + /* In case the volatile qualifier is applied to an array type, the > Mark> + element type is so qualified, not the array type (section 6.7.3 > Mark> + of C99). */ > Mark> + if (TYPE_CODE (base_type) == TYPE_CODE_ARRAY) > Mark> + return add_array_cv_type (die, cu, base_type, 0, 1); > > I wonder about typedefs to array type. > Calling check_typedef here is probably not so great but I assume we can > just ignore incomplete types. I am not sure I understand precisely what you are wondering about. Do you have an example? Then I can add a testcase for it. This is just precisely as is done for the const case. So the issue you are thinking about is probably the same for both cases and might already be existing. Thanks, Mark commit 8782e0bef99e1c282364d283f5cbf6cc32f437ee Author: Mark Wielaard Date: Mon Jun 30 23:21:52 2014 +0200 Handle volatile array types in dwarf2read.c. read_tag_const_type propagates the cv-qualifier to the array element type, but read_tag_volatile_type didn't. Make sure that both cv-qualifiers that apply to array types are handled the same. gdb/ChangeLog * dwarf2read.c (add_array_cv_type): New function. (read_tag_const_type): Call add_array_cv_type for TYPE_CODE_ARRAY. (read_tag_volatile_type): Likewise. gdb/testsuite/ChangeLog * gdb.base/constvars.c (violent, violet, vips, virgen, vulgar, vulture, vilify, villar): New volatile array constants. (vindictive, vegetation): New const volatile array constants. * gdb.base/volatile.exp: Test volatile and const volatile array types. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 8d304ac..ccfae0c 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2014-06-30 Mark Wielaard + + * dwarf2read.c (add_array_cv_type): New function. + (read_tag_const_type): Call add_array_cv_type for TYPE_CODE_ARRAY. + (read_tag_volatile_type): Likewise. + 2014-06-30 Tom Tromey * symtab.c (operator_chars): Make parameters and return type diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index fc4f7cb..8b600fb 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -14097,6 +14097,36 @@ read_tag_reference_type (struct die_info *die, struct dwarf2_cu *cu) return set_die_type (die, type, cu); } +/* Add the given cv-qualifiers to the element type of the array. GCC + outputs DWARF type qualifiers that apply to an array, not the + element type. But GDB relies on the array element type to carry + the cv-qualifiers. This mimics section 6.7.3 of the C99 + specification. */ + +static struct type * +add_array_cv_type (struct die_info *die, struct dwarf2_cu *cu, + struct type *base_type, int cnst, int voltl) +{ + struct type *el_type, *inner_array; + + base_type = copy_type (base_type); + inner_array = base_type; + + while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY) + { + TYPE_TARGET_TYPE (inner_array) = + copy_type (TYPE_TARGET_TYPE (inner_array)); + inner_array = TYPE_TARGET_TYPE (inner_array); + } + + el_type = TYPE_TARGET_TYPE (inner_array); + cnst |= TYPE_CONST (el_type); + voltl |= TYPE_VOLATILE (el_type); + TYPE_TARGET_TYPE (inner_array) = make_cv_type (cnst, voltl, el_type, NULL); + + return set_die_type (die, base_type, cu); +} + static struct type * read_tag_const_type (struct die_info *die, struct dwarf2_cu *cu) { @@ -14112,25 +14142,7 @@ read_tag_const_type (struct die_info *die, struct dwarf2_cu *cu) /* In case the const qualifier is applied to an array type, the element type is so qualified, not the array type (section 6.7.3 of C99). */ if (TYPE_CODE (base_type) == TYPE_CODE_ARRAY) - { - struct type *el_type, *inner_array; - - base_type = copy_type (base_type); - inner_array = base_type; - - while (TYPE_CODE (TYPE_TARGET_TYPE (inner_array)) == TYPE_CODE_ARRAY) - { - TYPE_TARGET_TYPE (inner_array) = - copy_type (TYPE_TARGET_TYPE (inner_array)); - inner_array = TYPE_TARGET_TYPE (inner_array); - } - - el_type = TYPE_TARGET_TYPE (inner_array); - TYPE_TARGET_TYPE (inner_array) = - make_cv_type (1, TYPE_VOLATILE (el_type), el_type, NULL); - - return set_die_type (die, base_type, cu); - } + return add_array_cv_type (die, cu, base_type, 1, 0); cv_type = make_cv_type (1, TYPE_VOLATILE (base_type), base_type, 0); return set_die_type (die, cv_type, cu); @@ -14148,6 +14160,12 @@ read_tag_volatile_type (struct die_info *die, struct dwarf2_cu *cu) if (cv_type) return cv_type; + /* In case the volatile qualifier is applied to an array type, the + element type is so qualified, not the array type (section 6.7.3 + of C99). */ + if (TYPE_CODE (base_type) == TYPE_CODE_ARRAY) + return add_array_cv_type (die, cu, base_type, 0, 1); + cv_type = make_cv_type (TYPE_CONST (base_type), 1, base_type, 0); return set_die_type (die, cv_type, cu); } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 751848f..90be171 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2014-06-30 Mark Wielaard + + * gdb.base/constvars.c (violent, violet, vips, virgen, vulgar, + vulture, vilify, villar): New volatile array constants. + (vindictive, vegetation): New const volatile array constants. + * gdb.base/volatile.exp: Test volatile and const volatile array + types. + 2014-06-30 Andreas Arnez * gdb.base/watchpoint-reuse-slot.exp: Handle the case that the diff --git a/gdb/testsuite/gdb.base/constvars.c b/gdb/testsuite/gdb.base/constvars.c index 4228822..60cca2a 100644 --- a/gdb/testsuite/gdb.base/constvars.c +++ b/gdb/testsuite/gdb.base/constvars.c @@ -127,6 +127,16 @@ main (void) volatile float * volatile vitality = &vacuity; volatile double * volatile voracity = &vertigo; + /* volatile arrays */ + volatile char violent[2] = {vox, vox}; + volatile unsigned char violet[2] = {victuals, victuals}; + volatile short vips[2] = {vixen, vixen}; + volatile unsigned short virgen[2] = {vitriol, vitriol}; + volatile long vulgar[2] = {vellum, vellum}; + volatile unsigned long vulture[2] = {valve, valve}; + volatile float vilify[2] = {vacuity, vacuity}; + volatile double villar[2] = {vertigo, vertigo}; + /* const volatile vars */ const volatile char victor = 'Y'; @@ -177,6 +187,10 @@ main (void) const volatile char * const volatile vagary = &victor; const volatile unsigned char * const volatile vendor = &vicar; + /* const volatile arrays */ + const volatile char vindictive[2] = {victor, victor}; + const volatile unsigned char vegetation[2] = {vicar, vicar}; + /* various structs with const members */ struct crass { char * const ptr; } crass = { lamprey }; diff --git a/gdb/testsuite/gdb.base/volatile.exp b/gdb/testsuite/gdb.base/volatile.exp index 7cd7254..0e106fc 100644 --- a/gdb/testsuite/gdb.base/volatile.exp +++ b/gdb/testsuite/gdb.base/volatile.exp @@ -229,6 +229,30 @@ gdb_test "ptype vagary" "type = const volatile char \\* const volatile.*" local_compiler_xfail_check gdb_test "ptype vendor" "type = const volatile unsigned char \\* const volatile.*" +# volatile arrays +local_compiler_xfail_check +gdb_test "ptype violent" "type = volatile char \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype violet" "type = volatile unsigned char \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype vips" "type = volatile short( int)? \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype virgen" "type = volatile (unsigned short|short unsigned)( int)? \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype vulgar" "type = volatile long( int)? \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype vulture" "type = volatile (unsigned long|long unsigned)( int)? \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype vilify" "type = volatile float \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype villar" "type = volatile double \\\[2\\\]" + +# const volatile arrays +local_compiler_xfail_check +gdb_test "ptype vindictive" "type = const volatile char \\\[2\\\]" +local_compiler_xfail_check +gdb_test "ptype vegetation" "type = const volatile unsigned char \\\[2\\\]" + # test function parameters local_compiler_xfail_check local_compiler_xfail_check_2