From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id TvZ2Geke3V8vfQAAWB0awg (envelope-from ) for ; Fri, 18 Dec 2020 16:28:09 -0500 Received: by simark.ca (Postfix, from userid 112) id 5A7D71F0AA; Fri, 18 Dec 2020 16:28:09 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 6F0811E552 for ; Fri, 18 Dec 2020 16:28:06 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BFF3B3854809; Fri, 18 Dec 2020 21:28:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BFF3B3854809 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1608326885; bh=NOGykB+F6daGBQ3TcyKTLvlnvkMeIteQpnkBFF5X7uY=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=U5KfaPi2wbS0sBnWKo8c/aE94NvIDRHXF+uPC7MgxUptkv+RFUQLlV8tU3iyD1CVi x+dykE3H7LItl9+Kv9sV493UkA4e1t0XIkmXl+bjYcuwE2A/mUbIL2hOW5cQTKAj9P s+QwfOIzro9zV6gZ5ylxchRng5Ra/iq2pzpLB1Qo= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 685003854809 for ; Fri, 18 Dec 2020 21:28:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 685003854809 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 0BILRv9E016700 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 18 Dec 2020 16:28:02 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 0BILRv9E016700 Received: from [10.0.0.213] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C1DFB1E552; Fri, 18 Dec 2020 16:27:57 -0500 (EST) Subject: Re: [PATCH v2] Don't compare types of enum fields To: Hannes Domani , gdb-patches@sourceware.org References: <20201217192912.1981-1-ssbssa.ref@yahoo.de> <20201217192912.1981-1-ssbssa@yahoo.de> Message-ID: Date: Fri, 18 Dec 2020 16:27:57 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.3 MIME-Version: 1.0 In-Reply-To: <20201217192912.1981-1-ssbssa@yahoo.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 18 Dec 2020 21:27:57 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 2020-12-17 2:29 p.m., Hannes Domani via Gdb-patches wrote: > Comparing types of enum fields results in a crash, because they don't > have a type. > > It can be reproduced by comparing the types of 2 instances of the same > enum type in different objects: > > enum.h: > enum e > { > zero, > one, > }; > > enum-1.c: > int func(); > enum e e1; > int main() > { > return e1 + func(); > } > > enum-2.c: > enum e e2; > int func() > { > return e2; > } > > $ gcc -g -oenum enum-1.c enum-2.c > $ gdb -q enum.exe > Reading symbols from enum.exe... > (gdb) py print(gdb.parse_and_eval("e1").type==gdb.parse_and_eval("e2").type) > > Thread 1 received signal SIGSEGV, Segmentation fault. > [Switching to Thread 6184.0x1cc4] > check_typedef (type=0x0) at C:/src/repos/binutils-gdb.git/gdb/gdbtypes.c:2745 > 2745 while (type->code () == TYPE_CODE_TYPEDEF) > > gdb/ChangeLog: > > 2020-12-17 Hannes Domani > > PR exp/27070 > * gdbtypes.c (check_types_equal): Don't compare types of enum fields. > > gdb/testsuite/ChangeLog: > > 2020-12-17 Hannes Domani > > PR exp/27070 > * gdb.base/pr27070-a.c: New test. > * gdb.base/pr27070-b.c: New test. > * gdb.base/pr27070.exp: New file. > * gdb.base/pr27070.h: New test. > --- > v2: > - Add detailed problem description in commit message and test case. Same comment as Tom, please find a descriptive name for this test. When you write the test, it's nice to name it after the PR number because you don't have to think of a good name. But that's about it, after that it doesn't give a clue what the test is about. > --- > gdb/gdbtypes.c | 4 +++- > gdb/testsuite/gdb.base/pr27070-a.c | 27 +++++++++++++++++++++++++++ > gdb/testsuite/gdb.base/pr27070-b.c | 25 +++++++++++++++++++++++++ > gdb/testsuite/gdb.base/pr27070.exp | 29 +++++++++++++++++++++++++++++ > gdb/testsuite/gdb.base/pr27070.h | 22 ++++++++++++++++++++++ > 5 files changed, 106 insertions(+), 1 deletion(-) > create mode 100644 gdb/testsuite/gdb.base/pr27070-a.c > create mode 100644 gdb/testsuite/gdb.base/pr27070-b.c > create mode 100644 gdb/testsuite/gdb.base/pr27070.exp > create mode 100644 gdb/testsuite/gdb.base/pr27070.h > > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c > index a40ae5f30e..2207613eef 100644 > --- a/gdb/gdbtypes.c > +++ b/gdb/gdbtypes.c > @@ -4036,7 +4036,9 @@ check_types_equal (struct type *type1, struct type *type2, > case FIELD_LOC_KIND_ENUMVAL: > if (FIELD_ENUMVAL (*field1) != FIELD_ENUMVAL (*field2)) > return false; > - break; > + /* Don't compare types of enum fields, because they don't > + have a type. */ > + continue; > case FIELD_LOC_KIND_PHYSADDR: > if (FIELD_STATIC_PHYSADDR (*field1) > != FIELD_STATIC_PHYSADDR (*field2)) > diff --git a/gdb/testsuite/gdb.base/pr27070-a.c b/gdb/testsuite/gdb.base/pr27070-a.c > new file mode 100644 > index 0000000000..f3850803ea > --- /dev/null > +++ b/gdb/testsuite/gdb.base/pr27070-a.c > @@ -0,0 +1,27 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020 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 . */ > + > +#include "pr27070.h" > + > +int func(); > + > +enum e e1; > + > +int main() int main (void) > +{ > + return e1 + func(); > +} Space before parens everywhere. > diff --git a/gdb/testsuite/gdb.base/pr27070-b.c b/gdb/testsuite/gdb.base/pr27070-b.c > new file mode 100644 > index 0000000000..39962c671a > --- /dev/null > +++ b/gdb/testsuite/gdb.base/pr27070-b.c > @@ -0,0 +1,25 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020 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 . */ > + > +#include "pr27070.h" > + > +enum e e2; > + > +int func() int func (void) > +{ > + return e2; > +} > diff --git a/gdb/testsuite/gdb.base/pr27070.exp b/gdb/testsuite/gdb.base/pr27070.exp > new file mode 100644 > index 0000000000..3f565da661 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/pr27070.exp > @@ -0,0 +1,29 @@ > +# Copyright 2020 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 . > + Please add a short comment explaining what the test tests. It's useful to refer to the PR number in that comment, so readers can get more context if they want, but don't just use "Test for PR 27070.", the comment needs to be informative even if bugzilla disappears. > +set testname pr27070 > +set sources "pr27070-a.c pr27070-b.c" You can probably use: standard_testfile -a.c -b.c > + > +if {[build_executable ${testname}.exp $testname $sources {debug}] == -1} { > + return -1 > +} > + > +# Start with a fresh gdb. > + > +clean_restart ${testname} You can probably use "prepare_for_testing" which does the build_executable + clean_restart. > + > +if { [skip_python_tests] } { continue } I suppose this can go earlier, to avoid compiling the program and starting GDB if the test is going to be skipped anyway. > + > +gdb_test "py print(gdb.parse_and_eval('e1').type == gdb.parse_and_eval('e2').type)" "True" I think it would make sense to move this test to the gdb.python directory. Simon