From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf41.google.com (mail-qv1-xf41.google.com [IPv6:2607:f8b0:4864:20::f41]) by sourceware.org (Postfix) with ESMTPS id 87B5A3858D35 for ; Tue, 30 Jun 2020 17:25:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 87B5A3858D35 Received: by mail-qv1-xf41.google.com with SMTP id e3so3315740qvo.10 for ; Tue, 30 Jun 2020 10:25:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=abiZYusvtemzNIjcP2RpW0W4sHCNILYqJvSBrqI2PA8=; b=tkf3ZC3WvDxV+FrxAkretfVwL05R6ksGPstVQc1nO4eOzQk4ye+SrmXT0n9DuAu18M LBkUTgwtEqcFYEMbZXB7c9DTcTPELDkHmIK5vBAwab3e8iClQrrWzF+8573pVqzdwz1P xFrAjVCWkC43xhYDTHPEtCUe21g0kMyuZ/oYR2bGY0fNhXJxGFjLB90UL6SJik3x6t5x M/dSRknLLjyixd1XrFWEKJPOMDDBBdCbjjBi367E1jKIKbFPk7TnEqUi2nTl1FhFAQHP HgXDRPvMOI2fiS1U3Aw6JSQjCllCM2J044aLVhV/mn0lPgAZ1HO4Br5IEx8ptYyXmgrx OWHg== X-Gm-Message-State: AOAM530G7e8P8M/wQFfFEl0VP68eGDpxzWG6JbZgfUI8CcsHyY1XIOm2 4gT6QVdqqBUBAo1U3fanHdmcoYxDCx0= X-Google-Smtp-Source: ABdhPJw/8iLrd32zvGkkZP5o4zrF9Krreifd21rkuzifJupridsWa4VeExq15yQgjBNRIMIV7vAIEA== X-Received: by 2002:ad4:4d83:: with SMTP id cv3mr21477640qvb.236.1593537938489; Tue, 30 Jun 2020 10:25:38 -0700 (PDT) Received: from ?IPv6:2804:7f0:8283:20c3:e01d:4330:d5b2:db25? ([2804:7f0:8283:20c3:e01d:4330:d5b2:db25]) by smtp.gmail.com with ESMTPSA id q5sm3595968qkq.36.2020.06.30.10.25.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 30 Jun 2020 10:25:37 -0700 (PDT) Subject: Re: [PATCH] Skip VLA structure field tests when compiling with clang To: Gary Benson , gdb-patches@sourceware.org References: <1593515555-31490-1-git-send-email-gbenson@redhat.com> From: Luis Machado Message-ID: Date: Tue, 30 Jun 2020 14:25:34 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <1593515555-31490-1-git-send-email-gbenson@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Jun 2020 17:25:41 -0000 Hi, As a general comment (for this and other patches like this), I think the preprocessor blocks intermixed with the test code make it harder to follow and update the test. If clang claims it will never support a particular feature, then maybe we should have a separate testcase clang can actually run and skip this entire test if the compiler is clang? Or maybe separate the bits clang doesn't support and put it into a separate test that clang-compiled hosts should skip? On 6/30/20 8:12 AM, Gary Benson via Gdb-patches wrote: > Hi all, > > Clang fails to compile gdb.base/vla-datatypes.c with the following > error: fields must have a constant size: 'variable length array in > structure' extension will never be supported. This patch adds > preprocessor contitionals to vla-datatypes.c to not compile any code > defining or using use VLA structure fields when compiling with clang, > and modifies vla-datatypes.exp to skip all tests that require the > omitted code as appropriate. > > Checked on Fedora 31 x86_64, GCC and clang. Ok to commit? > > Cheers, > Gary > > -- > gdb/testsuite/ChangeLog: > > * gdb.base/vla-datatypes.c (vla_factory): Wrap sections defining > and using VLA structure fields with "#if !defined(__clang__)". > * gdb.base/vla-datatypes.exp: Skip VLA structure field tests if > compiling with clang. > --- > gdb/testsuite/ChangeLog | 7 +++++ > gdb/testsuite/gdb.base/vla-datatypes.c | 8 ++++++ > gdb/testsuite/gdb.base/vla-datatypes.exp | 47 ++++++++++++++++++++------------ > 3 files changed, 44 insertions(+), 18 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/vla-datatypes.c b/gdb/testsuite/gdb.base/vla-datatypes.c > index 682319f..ce383a1 100644 > --- a/gdb/testsuite/gdb.base/vla-datatypes.c > +++ b/gdb/testsuite/gdb.base/vla-datatypes.c > @@ -46,6 +46,9 @@ struct foo > BAR bar_vla[n]; > int i; > > + /* Clang says it will never support variable length arrays in > + structures. */ > +#if !defined(__clang__) > struct vla_struct > { > int something; > @@ -89,6 +92,7 @@ struct foo > vla_struct_typedef_struct_member_object.something = n * 2; > vla_struct_typedef_struct_member_object.vla_object.something = n * 3; > vla_struct_typedef_union_member_object.vla_object.something = n + 1; > +#endif /* !defined(__clang__) */ > for (i = 0; i < n; i++) > { > int_vla[i] = i*2; > @@ -104,6 +108,7 @@ struct foo > foo_vla[i].a = i*2; > bar_vla[i].x = i*2; > bar_vla[i].y.a = i*2; > +#if !defined(__clang__) > vla_struct_object.vla_field[i] = i*2; > vla_union_object.vla_field[i] = i*2; > inner_vla_struct_object.vla_field[i] = i*2; > @@ -111,6 +116,7 @@ struct foo > = i * 3; > vla_struct_typedef_union_member_object.vla_object.vla_field[i] > = i * 3 - 1; > +#endif /* !defined(__clang__) */ > } > > size_t int_size = sizeof(int_vla); /* vlas_filled */ > @@ -124,9 +130,11 @@ struct foo > size_t uchar_size = sizeof(unsigned_char_vla); > size_t foo_size = sizeof(foo_vla); > size_t bar_size = sizeof(bar_vla); > +#if !defined(__clang__) > size_t vla_struct_object_size = sizeof(vla_struct_object); > size_t vla_union_object_size = sizeof(vla_union_object); > size_t inner_vla_struct_object_size = sizeof(inner_vla_struct_object); > +#endif /* !defined(__clang__) */ > > return; /* break_end_of_vla_factory */ > } > diff --git a/gdb/testsuite/gdb.base/vla-datatypes.exp b/gdb/testsuite/gdb.base/vla-datatypes.exp > index e8d8457..2924475 100644 > --- a/gdb/testsuite/gdb.base/vla-datatypes.exp > +++ b/gdb/testsuite/gdb.base/vla-datatypes.exp > @@ -15,6 +15,9 @@ > > standard_testfile > > +# Clang says it will never support variable length arrays in structures. > +set is_clang [test_compiler_info clang*] > + > if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { > return -1 > } > @@ -41,14 +44,16 @@ gdb_test "print foo_vla" \ > "\\\{\\\{a = 0\\\}, \\\{a = 2\\\}, \\\{a = 4\\\}, \\\{a = 6\\\}, \\\{a = 8\\\}\\\}" > gdb_test "print bar_vla" \ > "\\\{\\\{x = 0, y = \\\{a = 0\\\}\\\}, \\\{x = 2, y = \\\{a = 2\\\}\\\}, \\\{x = 4, y = \\\{a = 4\\\}\\\}, \\\{x = 6, y = \\\{a = 6\\\}\\\}, \\\{x = 8, y = \\\{a = 8\\\}\\\}\\\}" > -gdb_test "print vla_struct_object" \ > - "\\\{something = 5, vla_field = \\\{0, 2, 4, 6, 8\\\}\\\}" > -gdb_test "print vla_union_object" \ > - "\\\{vla_field = \\\{0, 2, 4, 6, 8\\\}\\\}" > -gdb_test "print vla_struct_typedef_struct_member_object" \ > - "\\\{something = 10, vla_object = \\\{something = 15, vla_field = \\\{0, 3, 6, 9, 12\\\}\\\}\\\}" > -gdb_test "print vla_struct_typedef_union_member_object" \ > - "\\\{something = 6, vla_object = \\\{something = 6, vla_field = \\\{-1, 2, 5, 8, 11\\\}\\\}\\\}" > +if {!$is_clang} { > + gdb_test "print vla_struct_object" \ > + "\\\{something = 5, vla_field = \\\{0, 2, 4, 6, 8\\\}\\\}" > + gdb_test "print vla_union_object" \ > + "\\\{vla_field = \\\{0, 2, 4, 6, 8\\\}\\\}" > + gdb_test "print vla_struct_typedef_struct_member_object" \ > + "\\\{something = 10, vla_object = \\\{something = 15, vla_field = \\\{0, 3, 6, 9, 12\\\}\\\}\\\}" > + gdb_test "print vla_struct_typedef_union_member_object" \ > + "\\\{something = 6, vla_object = \\\{something = 6, vla_field = \\\{-1, 2, 5, 8, 11\\\}\\\}\\\}" > +} > > # Check whatis of VLA's. > gdb_test "whatis int_vla" "type = int \\\[5\\\]" > @@ -65,8 +70,10 @@ gdb_test "whatis unsigned_short_vla" \ > gdb_test "whatis unsigned_char_vla" "type = unsigned char \\\[5\\\]" > gdb_test "whatis foo_vla" "type = struct foo \\\[5\\\]" > gdb_test "whatis bar_vla" "type = BAR \\\[5\\\]" > -gdb_test "whatis vla_struct_object" "type = vla_struct_typedef" > -gdb_test "whatis vla_union_object" "type = union vla_union" > +if {!$is_clang} { > + gdb_test "whatis vla_struct_object" "type = vla_struct_typedef" > + gdb_test "whatis vla_union_object" "type = union vla_union" > +} > > # Check ptype of VLA's. > gdb_test "ptype int_vla" "type = int \\\[5\\\]" > @@ -82,10 +89,12 @@ gdb_test "ptype unsigned_char_vla" "type = unsigned char \\\[5\\\]" > gdb_test "ptype foo_vla" "type = struct foo {\r\n\\s+int a;\r\n} \\\[5\\\]" > gdb_test "ptype bar_vla" \ > "type = struct bar {\r\n\\s+int x;\r\n\\s+struct foo y;\r\n} \\\[5\\\]" > -gdb_test "ptype vla_struct_object" \ > - "type = struct vla_struct {\r\n\\s+int something;\r\n\\s+int vla_field\\\[5\\\];\r\n}" > -gdb_test "ptype vla_union_object" \ > - "type = union vla_union {\r\n\\s+int vla_field\\\[5\\\];\r\n}" > +if {!$is_clang} { > + gdb_test "ptype vla_struct_object" \ > + "type = struct vla_struct {\r\n\\s+int something;\r\n\\s+int vla_field\\\[5\\\];\r\n}" > + gdb_test "ptype vla_union_object" \ > + "type = union vla_union {\r\n\\s+int vla_field\\\[5\\\];\r\n}" > +} > > # Check the size of the VLA's. > gdb_breakpoint [gdb_get_line_number "break_end_of_vla_factory"] > @@ -109,10 +118,12 @@ gdb_test "print uchar_size == sizeof(unsigned_char_vla)" " = 1" \ > "size of unsigned_char_vla" > gdb_test "print foo_size == sizeof(foo_vla)" " = 1" "size of foo_vla" > gdb_test "print bar_size == sizeof(bar_vla)" " = 1" "size of bar_vla" > -gdb_test "print vla_struct_object_size == sizeof(vla_struct_object)" \ > - " = 1" "size of vla_struct_object" > -gdb_test "print vla_union_object_size == sizeof(vla_union_object)" \ > - " = 1" "size of vla_union_object" > +if {!$is_clang} { > + gdb_test "print vla_struct_object_size == sizeof(vla_struct_object)" \ > + " = 1" "size of vla_struct_object" > + gdb_test "print vla_union_object_size == sizeof(vla_union_object)" \ > + " = 1" "size of vla_union_object" > +} > > # Check side effects for sizeof argument. > set sizeof_int [get_sizeof "int" 4] >