From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 79471 invoked by alias); 4 Sep 2018 15:59:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 79462 invoked by uid 89); 4 Sep 2018 15:59:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.2 spammy=solo, rights X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Sep 2018 15:59:11 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id D8F8FB0B9; Tue, 4 Sep 2018 15:59:08 +0000 (UTC) Subject: Re: [PATCH] DWARF: add DW_AT_count to zero-length arrays To: Jason Merrill Cc: Omar Sandoval , Andrew Pinski , Richard Biener , GCC Patches References: <20180817042900.GA19666@vader> <20180817065453.GA22101@vader> <20180817071607.GB22101@vader> <20180828182026.GC18173@vader> From: Tom de Vries Message-ID: Date: Tue, 04 Sep 2018 15:59:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180828182026.GC18173@vader> Content-Type: multipart/mixed; boundary="------------8194D20EE5D0192D0BAF0454" X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00206.txt.bz2 This is a multi-part message in MIME format. --------------8194D20EE5D0192D0BAF0454 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 4417 [ Adding Jason as addressee ] On 08/28/2018 08:20 PM, Omar Sandoval wrote: > On Fri, Aug 17, 2018 at 12:16:07AM -0700, Omar Sandoval wrote: >> On Thu, Aug 16, 2018 at 11:54:53PM -0700, Omar Sandoval wrote: >>> On Thu, Aug 16, 2018 at 10:27:48PM -0700, Andrew Pinski wrote: >>>> On Thu, Aug 16, 2018 at 9:29 PM Omar Sandoval wrote: >>>>> >>>>> Hi, >>>>> >>>>> This fixes the issue that it is impossible to distinguish a zero-length array >>>>> type from a flexible array type given the DWARF produced by GCC (which I >>>>> reported here [1]). We do so by adding a DW_AT_count attribute with a value of >>>>> zero only for zero-length arrays (this is what clang does in this case, too). >>>>> >>>>> 1: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86985 >>>>> >>>>> The reproducer from the PR now produces the expected output: >>>>> >>>>> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c zero_length.c >>>>> $ ~/gcc-build/gcc/xgcc -B ~/gcc-build/gcc -g -c flexible.c >>>>> $ gdb -batch -ex 'ptype baz' zero_length.o >>>>> type = struct { >>>>> int foo; >>>>> int bar[0]; >>>>> } >>>>> $ gdb -batch -ex 'ptype baz' flexible.o >>>>> type = struct { >>>>> int foo; >>>>> int bar[]; >>>>> } >>>>> >>>>> This was bootstrapped and tested on x86_64-pc-linux-gnu. >>>>> >>>>> I don't have commit rights (first time contributor), so if this change is okay >>>>> could it please be applied? >> >> [snip] >> >> Here's the patch with the is_c () helper instead. >> >> 2018-08-17 Omar Sandoval >> I've added the PR number here. >> * dwarf2out.c (is_c): New. >> (add_subscript_info): Add DW_AT_count of 0 for C zero-length arrays. >> >> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c >> index 5a74131d332..189f9bb381f 100644 >> --- a/gcc/dwarf2out.c >> +++ b/gcc/dwarf2out.c >> @@ -3671,6 +3671,7 @@ static const char *get_AT_string (dw_die_ref, enum dwarf_attribute); >> static int get_AT_flag (dw_die_ref, enum dwarf_attribute); >> static unsigned get_AT_unsigned (dw_die_ref, enum dwarf_attribute); >> static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute); >> +static bool is_c (void); >> static bool is_cxx (void); >> static bool is_cxx (const_tree); >> static bool is_fortran (void); >> @@ -5434,6 +5435,19 @@ get_AT_file (dw_die_ref die, enum dwarf_attribute attr_kind) >> return a ? AT_file (a) : NULL; >> } >> >> +/* Return TRUE if the language is C. */ >> + >> +static inline bool >> +is_c (void) >> +{ >> + unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language); >> + >> + return (lang == DW_LANG_C || lang == DW_LANG_C89 || lang == DW_LANG_C99 >> + || lang == DW_LANG_C11 || lang == DW_LANG_ObjC); >> + >> + >> +} >> + >> /* Return TRUE if the language is C++. */ >> >> static inline bool >> @@ -20918,12 +20932,24 @@ add_subscript_info (dw_die_ref type_die, tree type, bool collapse_p) >> dimension arr(N:*) >> Since the debugger is definitely going to need to know N >> to produce useful results, go ahead and output the lower >> - bound solo, and hope the debugger can cope. */ >> + bound solo, and hope the debugger can cope. >> + >> + For C and C++, if upper is NULL, this may be a zero-length array >> + or a flexible array; we'd like to be able to distinguish between >> + the two. Set DW_AT_count to 0 for the former. TYPE_SIZE is NULL >> + for the latter. */ >> I found inserting this comment here confusing, I've moved it down and shortened it. >> if (!get_AT (subrange_die, DW_AT_lower_bound)) >> add_bound_info (subrange_die, DW_AT_lower_bound, lower, NULL); >> - if (upper && !get_AT (subrange_die, DW_AT_upper_bound)) >> - add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL); >> + if (!get_AT (subrange_die, DW_AT_upper_bound) >> + && !get_AT (subrange_die, DW_AT_count)) >> + { >> + if (upper) >> + add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL); >> + else if ((is_c () || is_cxx ()) && TYPE_SIZE (type)) >> + add_bound_info (subrange_die, DW_AT_count, >> + build_int_cst (TREE_TYPE (lower), 0), NULL); >> + } >> } >> >> /* Otherwise we have an array type with an unspecified length. The > > Ping. Does this version look okay for trunk? > Looks ok to me (but I can't approve). Also, I've added a testcase. OK for trunk? Thanks, - Tom --------------8194D20EE5D0192D0BAF0454 Content-Type: text/x-patch; name="0001-debug-DWARF-add-DW_AT_count-to-zero-length-arrays.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-debug-DWARF-add-DW_AT_count-to-zero-length-arrays.patch" Content-length: 3031 [debug] DWARF: add DW_AT_count to zero-length arrays 2018-09-04 Omar Sandoval Tom de Vries PR debug/86985 * dwarf2out.c (is_c): New function. (add_subscript_info): Add DW_AT_count of 0 for C zero-length arrays. * gcc.dg/guality/zero-length-array.c: New test. --- gcc/dwarf2out.c | 26 ++++++++++++++++++++++-- gcc/testsuite/gcc.dg/guality/zero-length-array.c | 21 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 77317ed2575..58c3906cdbf 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -3679,6 +3679,7 @@ static const char *get_AT_string (dw_die_ref, enum dwarf_attribute); static int get_AT_flag (dw_die_ref, enum dwarf_attribute); static unsigned get_AT_unsigned (dw_die_ref, enum dwarf_attribute); static inline dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute); +static bool is_c (void); static bool is_cxx (void); static bool is_cxx (const_tree); static bool is_fortran (void); @@ -5442,6 +5443,19 @@ get_AT_file (dw_die_ref die, enum dwarf_attribute attr_kind) return a ? AT_file (a) : NULL; } +/* Return TRUE if the language is C. */ + +static inline bool +is_c (void) +{ + unsigned int lang = get_AT_unsigned (comp_unit_die (), DW_AT_language); + + return (lang == DW_LANG_C || lang == DW_LANG_C89 || lang == DW_LANG_C99 + || lang == DW_LANG_C11 || lang == DW_LANG_ObjC); + + +} + /* Return TRUE if the language is C++. */ static inline bool @@ -20952,8 +20966,16 @@ add_subscript_info (dw_die_ref type_die, tree type, bool collapse_p) if (!get_AT (subrange_die, DW_AT_lower_bound)) add_bound_info (subrange_die, DW_AT_lower_bound, lower, NULL); - if (upper && !get_AT (subrange_die, DW_AT_upper_bound)) - add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL); + if (!get_AT (subrange_die, DW_AT_upper_bound) + && !get_AT (subrange_die, DW_AT_count)) + { + if (upper) + add_bound_info (subrange_die, DW_AT_upper_bound, upper, NULL); + else if ((is_c () || is_cxx ()) && TYPE_SIZE (type)) + /* Zero-length array. */ + add_bound_info (subrange_die, DW_AT_count, + build_int_cst (TREE_TYPE (lower), 0), NULL); + } } /* Otherwise we have an array type with an unspecified length. The diff --git a/gcc/testsuite/gcc.dg/guality/zero-length-array.c b/gcc/testsuite/gcc.dg/guality/zero-length-array.c new file mode 100644 index 00000000000..33f34d98ac2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/guality/zero-length-array.c @@ -0,0 +1,21 @@ +/* PR debug/86985 */ +/* { dg-do run } */ +/* { dg-options "-g" } */ + +struct { + int foo; + int bar[0]; +} zla; /* Zero length array. */ + +struct { + int foo; + int bar[]; +} fam; /* Flexible array member. */ + +int +main () +{ + /* { dg-final { gdb-test . "type:zla" "struct { int foo; int bar[0]; }" } } */ + /* { dg-final { gdb-test . "type:fam" "struct { int foo; int bar[]; }" } } */ + return 0; +} --------------8194D20EE5D0192D0BAF0454--