Message ID | VI1PR08MB5325F950A716B0FFDF3809EFE4430@VI1PR08MB5325.eurprd08.prod.outlook.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 41805 invoked by alias); 2 Dec 2019 18:04:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 41797 invoked by uid 89); 2 Dec 2019 18:04:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LOTSOFHASH, KAM_NUMSUBJECT, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: EUR03-AM5-obe.outbound.protection.outlook.com Received: from mail-oln040092070095.outbound.protection.outlook.com (HELO EUR03-AM5-obe.outbound.protection.outlook.com) (40.92.70.95) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Dec 2019 18:04:15 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=crM1tQ7fk/rsWas0ESOJZTyLacNe1+vAcXdZ7mSGtox2/icmBoYNs8+ZO/2mI3nLFq7w1kZ8hBddf5Cd90CQ6tbJ+tROWw/P5tVdDrUiEbTAqYUJt9cPdVMWqnB4sRxfWHHDebPM+GNnNRSVp9aKFuvIFm6B0I4DexphsV9OKg+xMAcSwYsHOut86vNuatQrJ1nC1+268pjUTiMs+10gHDcBXeFNcOFsrRTwwJYzUkVofmqiLhyeyFJX4uSVyByTsfDrX2cKAe7+dkF1eDTQThzsH2jvp9OBIv2/QPOVUeWMA2ocGeadZ3IQqJ9ClZE2nYHUPRoicTwf5TpAahmTwA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=XcVqQF+eKltUVSymJkoKVptqxNINkyJhCIP44jen+BI=; b=a0lrOOQ27Z0aPmonXudMOVm1zxnkB6EyHbzUZdq1eMo7MbMpqo9zrsecov7P4PEN+6KGxw90vX7o/G0M7Bio24X5XZU84q2lMeMVgeibKf25LS7xXBepocTYSwy3jB62xlpQHKmnWCB9ImqRevstgxp33rBL8JXU5OEy1Zw5xo9AAJkNw9HBqXR5UgqdbCpjT+h/mZCgi6NRyh3ms1UyVEogm+Ma5Kv4toLDWJI603FDA2laL90Sn9FMQrfxp+OwjXv4a/FmADItMe/twqsEI6J9Ci/c+BZRO5KKpe7gBRTvHb4mTg7DQ9tdK2qWm7oeE4KhjPxDAd4GzdG2aPingw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from AM5EUR03FT017.eop-EUR03.prod.protection.outlook.com (10.152.16.57) by AM5EUR03HT232.eop-EUR03.prod.protection.outlook.com (10.152.17.55) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2495.18; Mon, 2 Dec 2019 18:04:12 +0000 Received: from VI1PR08MB5325.eurprd08.prod.outlook.com (10.152.16.53) by AM5EUR03FT017.mail.protection.outlook.com (10.152.16.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2495.18 via Frontend Transport; Mon, 2 Dec 2019 18:04:12 +0000 Received: from VI1PR08MB5325.eurprd08.prod.outlook.com ([fe80::5861:779d:3997:2e70]) by VI1PR08MB5325.eurprd08.prod.outlook.com ([fe80::5861:779d:3997:2e70%7]) with mapi id 15.20.2495.014; Mon, 2 Dec 2019 18:04:12 +0000 From: Bernd Edlinger <bernd.edlinger@hotmail.de> To: Andrew Burgess <andrew.burgess@embecosm.com> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> Subject: [PATCHv2] Fix setting breakpoints or stepping on line 65535 Date: Mon, 2 Dec 2019 18:04:12 +0000 Message-ID: <VI1PR08MB5325F950A716B0FFDF3809EFE4430@VI1PR08MB5325.eurprd08.prod.outlook.com> References: <VI1PR03MB45286F546846FDB9CA9C2100E44B0@VI1PR03MB4528.eurprd03.prod.outlook.com> <20191201220818.GI3410@embecosm.com> In-Reply-To: <20191201220818.GI3410@embecosm.com> x-microsoft-original-message-id: <d3497d26-400b-2405-2d6d-c5b6f3208c62@hotmail.de> x-ms-exchange-transport-forked: True Content-Type: multipart/mixed; boundary="_004_VI1PR08MB5325F950A716B0FFDF3809EFE4430VI1PR08MB5325eurp_" MIME-Version: 1.0 |
Commit Message
Bernd Edlinger
Dec. 2, 2019, 6:04 p.m. UTC
Hi Andrew, On 12/1/19 11:08 PM, Andrew Burgess wrote: > * Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-11-24 11:54:23 +0000]: > >> Hi, >> >> this removes code that is present from the very first git revisison >> 7b4ac7e1ed2c4616bce56d1760807798be87ac9e from 1988. It was in the >> gdb/dbxread.c at the time (and makes more sense for dbx line info format >> since line numbers are 16-bit entities in that debug format and debugging >> files with more than 65535 lines would not work anyway) but moved from >> there to gdb/buildsym.c which is used for dwarf line info as well, and >> excluding an arbitrary line number does certainly not make sense nowadays. >> >> >> Thanks >> Bernd. > >> From f202ae765b72ad6d17600eb661993a63191309f7 Mon Sep 17 00:00:00 2001 >> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >> Date: Sat, 23 Nov 2019 07:37:26 +0100 >> Subject: [PATCH 1/2] Fix setting breakpoints or stepping on line 65535 >> > > Bernd, > > Thanks for looking into this, and especially thanks for adding a test! > > Normally you should include the git commit message and ChangeLog along > with your patch submission so that these can be reviewed too. 'git > format-patch' and 'git send-email' can be useful for this, if you can > get them setup. > Okay, I added changelog messages for gdb/ChangeLog and gdb/testsuite/ChangeLog, and improved the commit messages. I hope you can handle the format-patch files as attachments. > Given the age of the code you're removing I think this change sounds > reasonable. I assume there's no test that covers why this code should > be there, so you see no regressions with this code removed? > No, there were no regressions (last full test on 24-11-19). > I have a couple of minor issues with the test. If you address those > and repost with commit message and ChangeLog this can be approved. > Fixed the test. Thanks a lot for your review. Is it OK for trunk? Thanks Bernd. > Thanks, > Andrew > > > > >> --- >> gdb/buildsym.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/gdb/buildsym.c b/gdb/buildsym.c >> index 79f8305..6c14f3f 100644 >> --- a/gdb/buildsym.c >> +++ b/gdb/buildsym.c >> @@ -670,12 +670,6 @@ buildsym_compunit::record_line (struct subfile *subfile, int line, >> { >> struct linetable_entry *e; >> >> - /* Ignore the dummy line number in libg.o */ >> - if (line == 0xffff) >> - { >> - return; >> - } >> - >> /* Make sure line vector exists and is big enough. */ >> if (!subfile->line_vector) >> { >> -- >> 1.9.1 >> > >> From 54a7631206a2cf54573fb4cc94474cb2f6f99245 Mon Sep 17 00:00:00 2001 >> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >> Date: Sun, 24 Nov 2019 09:37:22 +0100 >> Subject: [PATCH 2/2] Add a test case for line 65535 >> >> --- >> gdb/testsuite/gdb.base/line65535.c | 19 +++++++++++++++++++ >> gdb/testsuite/gdb.base/line65535.exp | 26 ++++++++++++++++++++++++++ >> 2 files changed, 45 insertions(+) >> create mode 100644 gdb/testsuite/gdb.base/line65535.c >> create mode 100644 gdb/testsuite/gdb.base/line65535.exp >> >> diff --git a/gdb/testsuite/gdb.base/line65535.c b/gdb/testsuite/gdb.base/line65535.c >> new file mode 100644 >> index 0000000..d80a294 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/line65535.c >> @@ -0,0 +1,19 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2019 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 <http://www.gnu.org/licenses/>. */ >> + >> +#line 65535 "line65535.c" >> +int main() { return 0; } >> diff --git a/gdb/testsuite/gdb.base/line65535.exp b/gdb/testsuite/gdb.base/line65535.exp >> new file mode 100644 >> index 0000000..2535ba5 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/line65535.exp >> @@ -0,0 +1,26 @@ >> +# Copyright 2019 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 <http://www.gnu.org/licenses/>. >> + > > Normally we like to add a short description of what the test is doing > here - for new tests. Many old ones don't have this thought. > Okay, done. >> +standard_testfile >> + >> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { >> + return -1 >> +} >> + >> +gdb_test "break $srcfile:65535" \ >> + ".*Breakpoint 1 at .*: file $srcfile, line 65535\\..*" \ >> + "break at line 65535" >> + >> +return 0 > > I don't think this return is needed at the end of a test. > Okay, done. Works without. >> -- >> 1.9.1 >> > gdb: 2019-12-02 Bernd Edlinger <bernd.edlinger@hotmail.de> * buildsym.c (buildsym_compunit::record_line): Do no longer ignore line 65535. gdb/testsuite: 2019-12-02 Bernd Edlinger <bernd.edlinger@hotmail.de> * gdb.base/line65535.exp: New file. * gdb.base/line65535.c: New file.
Comments
Ping... I'd like to ping for this patch here: https://sourceware.org/ml/gdb-patches/2019-12/msg00049.html Thanks Bernd. On 12/2/19 7:04 PM, Bernd Edlinger wrote: > Hi Andrew, > > On 12/1/19 11:08 PM, Andrew Burgess wrote: >> * Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-11-24 11:54:23 +0000]: >> >>> Hi, >>> >>> this removes code that is present from the very first git revisison >>> 7b4ac7e1ed2c4616bce56d1760807798be87ac9e from 1988. It was in the >>> gdb/dbxread.c at the time (and makes more sense for dbx line info format >>> since line numbers are 16-bit entities in that debug format and debugging >>> files with more than 65535 lines would not work anyway) but moved from >>> there to gdb/buildsym.c which is used for dwarf line info as well, and >>> excluding an arbitrary line number does certainly not make sense nowadays. >>> >>> >>> Thanks >>> Bernd. >> >>> From f202ae765b72ad6d17600eb661993a63191309f7 Mon Sep 17 00:00:00 2001 >>> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >>> Date: Sat, 23 Nov 2019 07:37:26 +0100 >>> Subject: [PATCH 1/2] Fix setting breakpoints or stepping on line 65535 >>> >> >> Bernd, >> >> Thanks for looking into this, and especially thanks for adding a test! >> >> Normally you should include the git commit message and ChangeLog along >> with your patch submission so that these can be reviewed too. 'git >> format-patch' and 'git send-email' can be useful for this, if you can >> get them setup. >> > > Okay, I added changelog messages for gdb/ChangeLog and gdb/testsuite/ChangeLog, > and improved the commit messages. > I hope you can handle the format-patch files as attachments. > >> Given the age of the code you're removing I think this change sounds >> reasonable. I assume there's no test that covers why this code should >> be there, so you see no regressions with this code removed? >> > > No, there were no regressions (last full test on 24-11-19). > >> I have a couple of minor issues with the test. If you address those >> and repost with commit message and ChangeLog this can be approved. >> > > Fixed the test. > > Thanks a lot for your review. > Is it OK for trunk? > > > Thanks > Bernd. > >> Thanks, >> Andrew >> >> >> >> >>> --- >>> gdb/buildsym.c | 6 ------ >>> 1 file changed, 6 deletions(-) >>> >>> diff --git a/gdb/buildsym.c b/gdb/buildsym.c >>> index 79f8305..6c14f3f 100644 >>> --- a/gdb/buildsym.c >>> +++ b/gdb/buildsym.c >>> @@ -670,12 +670,6 @@ buildsym_compunit::record_line (struct subfile *subfile, int line, >>> { >>> struct linetable_entry *e; >>> >>> - /* Ignore the dummy line number in libg.o */ >>> - if (line == 0xffff) >>> - { >>> - return; >>> - } >>> - >>> /* Make sure line vector exists and is big enough. */ >>> if (!subfile->line_vector) >>> { >>> -- >>> 1.9.1 >>> >> >>> From 54a7631206a2cf54573fb4cc94474cb2f6f99245 Mon Sep 17 00:00:00 2001 >>> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >>> Date: Sun, 24 Nov 2019 09:37:22 +0100 >>> Subject: [PATCH 2/2] Add a test case for line 65535 >>> >>> --- >>> gdb/testsuite/gdb.base/line65535.c | 19 +++++++++++++++++++ >>> gdb/testsuite/gdb.base/line65535.exp | 26 ++++++++++++++++++++++++++ >>> 2 files changed, 45 insertions(+) >>> create mode 100644 gdb/testsuite/gdb.base/line65535.c >>> create mode 100644 gdb/testsuite/gdb.base/line65535.exp >>> >>> diff --git a/gdb/testsuite/gdb.base/line65535.c b/gdb/testsuite/gdb.base/line65535.c >>> new file mode 100644 >>> index 0000000..d80a294 >>> --- /dev/null >>> +++ b/gdb/testsuite/gdb.base/line65535.c >>> @@ -0,0 +1,19 @@ >>> +/* This testcase is part of GDB, the GNU debugger. >>> + >>> + Copyright 2019 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 <http://www.gnu.org/licenses/>. */ >>> + >>> +#line 65535 "line65535.c" >>> +int main() { return 0; } >>> diff --git a/gdb/testsuite/gdb.base/line65535.exp b/gdb/testsuite/gdb.base/line65535.exp >>> new file mode 100644 >>> index 0000000..2535ba5 >>> --- /dev/null >>> +++ b/gdb/testsuite/gdb.base/line65535.exp >>> @@ -0,0 +1,26 @@ >>> +# Copyright 2019 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 <http://www.gnu.org/licenses/>. >>> + >> >> Normally we like to add a short description of what the test is doing >> here - for new tests. Many old ones don't have this thought. >> > > Okay, done. > >>> +standard_testfile >>> + >>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { >>> + return -1 >>> +} >>> + >>> +gdb_test "break $srcfile:65535" \ >>> + ".*Breakpoint 1 at .*: file $srcfile, line 65535\\..*" \ >>> + "break at line 65535" >>> + >>> +return 0 >> >> I don't think this return is needed at the end of a test. >> > > Okay, done. Works without. > >>> -- >>> 1.9.1 >>> >>
Ping... On 12/14/19 2:57 PM, Bernd Edlinger wrote: > Ping... > > I'd like to ping for this patch here: > https://sourceware.org/ml/gdb-patches/2019-12/msg00049.html > > Thanks > Bernd. > > On 12/2/19 7:04 PM, Bernd Edlinger wrote: >> Hi Andrew, >> >> On 12/1/19 11:08 PM, Andrew Burgess wrote: >>> * Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-11-24 11:54:23 +0000]: >>> >>>> Hi, >>>> >>>> this removes code that is present from the very first git revisison >>>> 7b4ac7e1ed2c4616bce56d1760807798be87ac9e from 1988. It was in the >>>> gdb/dbxread.c at the time (and makes more sense for dbx line info format >>>> since line numbers are 16-bit entities in that debug format and debugging >>>> files with more than 65535 lines would not work anyway) but moved from >>>> there to gdb/buildsym.c which is used for dwarf line info as well, and >>>> excluding an arbitrary line number does certainly not make sense nowadays. >>>> >>>> >>>> Thanks >>>> Bernd. >>> >>>> From f202ae765b72ad6d17600eb661993a63191309f7 Mon Sep 17 00:00:00 2001 >>>> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >>>> Date: Sat, 23 Nov 2019 07:37:26 +0100 >>>> Subject: [PATCH 1/2] Fix setting breakpoints or stepping on line 65535 >>>> >>> >>> Bernd, >>> >>> Thanks for looking into this, and especially thanks for adding a test! >>> >>> Normally you should include the git commit message and ChangeLog along >>> with your patch submission so that these can be reviewed too. 'git >>> format-patch' and 'git send-email' can be useful for this, if you can >>> get them setup. >>> >> >> Okay, I added changelog messages for gdb/ChangeLog and gdb/testsuite/ChangeLog, >> and improved the commit messages. >> I hope you can handle the format-patch files as attachments. >> >>> Given the age of the code you're removing I think this change sounds >>> reasonable. I assume there's no test that covers why this code should >>> be there, so you see no regressions with this code removed? >>> >> >> No, there were no regressions (last full test on 24-11-19). >> >>> I have a couple of minor issues with the test. If you address those >>> and repost with commit message and ChangeLog this can be approved. >>> >> >> Fixed the test. >> >> Thanks a lot for your review. >> Is it OK for trunk? >> >> >> Thanks >> Bernd. >> >>> Thanks, >>> Andrew >>> >>> >>> >>> >>>> --- >>>> gdb/buildsym.c | 6 ------ >>>> 1 file changed, 6 deletions(-) >>>> >>>> diff --git a/gdb/buildsym.c b/gdb/buildsym.c >>>> index 79f8305..6c14f3f 100644 >>>> --- a/gdb/buildsym.c >>>> +++ b/gdb/buildsym.c >>>> @@ -670,12 +670,6 @@ buildsym_compunit::record_line (struct subfile *subfile, int line, >>>> { >>>> struct linetable_entry *e; >>>> >>>> - /* Ignore the dummy line number in libg.o */ >>>> - if (line == 0xffff) >>>> - { >>>> - return; >>>> - } >>>> - >>>> /* Make sure line vector exists and is big enough. */ >>>> if (!subfile->line_vector) >>>> { >>>> -- >>>> 1.9.1 >>>> >>> >>>> From 54a7631206a2cf54573fb4cc94474cb2f6f99245 Mon Sep 17 00:00:00 2001 >>>> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >>>> Date: Sun, 24 Nov 2019 09:37:22 +0100 >>>> Subject: [PATCH 2/2] Add a test case for line 65535 >>>> >>>> --- >>>> gdb/testsuite/gdb.base/line65535.c | 19 +++++++++++++++++++ >>>> gdb/testsuite/gdb.base/line65535.exp | 26 ++++++++++++++++++++++++++ >>>> 2 files changed, 45 insertions(+) >>>> create mode 100644 gdb/testsuite/gdb.base/line65535.c >>>> create mode 100644 gdb/testsuite/gdb.base/line65535.exp >>>> >>>> diff --git a/gdb/testsuite/gdb.base/line65535.c b/gdb/testsuite/gdb.base/line65535.c >>>> new file mode 100644 >>>> index 0000000..d80a294 >>>> --- /dev/null >>>> +++ b/gdb/testsuite/gdb.base/line65535.c >>>> @@ -0,0 +1,19 @@ >>>> +/* This testcase is part of GDB, the GNU debugger. >>>> + >>>> + Copyright 2019 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 <http://www.gnu.org/licenses/>. */ >>>> + >>>> +#line 65535 "line65535.c" >>>> +int main() { return 0; } >>>> diff --git a/gdb/testsuite/gdb.base/line65535.exp b/gdb/testsuite/gdb.base/line65535.exp >>>> new file mode 100644 >>>> index 0000000..2535ba5 >>>> --- /dev/null >>>> +++ b/gdb/testsuite/gdb.base/line65535.exp >>>> @@ -0,0 +1,26 @@ >>>> +# Copyright 2019 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 <http://www.gnu.org/licenses/>. >>>> + >>> >>> Normally we like to add a short description of what the test is doing >>> here - for new tests. Many old ones don't have this thought. >>> >> >> Okay, done. >> >>>> +standard_testfile >>>> + >>>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { >>>> + return -1 >>>> +} >>>> + >>>> +gdb_test "break $srcfile:65535" \ >>>> + ".*Breakpoint 1 at .*: file $srcfile, line 65535\\..*" \ >>>> + "break at line 65535" >>>> + >>>> +return 0 >>> >>> I don't think this return is needed at the end of a test. >>> >> >> Okay, done. Works without. >> >>>> -- >>>> 1.9.1 >>>> >>>
On 2019-12-02 1:04 p.m., Bernd Edlinger wrote: > Hi Andrew, > > On 12/1/19 11:08 PM, Andrew Burgess wrote: >> * Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-11-24 11:54:23 +0000]: >> >>> Hi, >>> >>> this removes code that is present from the very first git revisison >>> 7b4ac7e1ed2c4616bce56d1760807798be87ac9e from 1988. It was in the >>> gdb/dbxread.c at the time (and makes more sense for dbx line info format >>> since line numbers are 16-bit entities in that debug format and debugging >>> files with more than 65535 lines would not work anyway) but moved from >>> there to gdb/buildsym.c which is used for dwarf line info as well, and >>> excluding an arbitrary line number does certainly not make sense nowadays. >>> >>> >>> Thanks >>> Bernd. >> >>> From f202ae765b72ad6d17600eb661993a63191309f7 Mon Sep 17 00:00:00 2001 >>> From: Bernd Edlinger <bernd.edlinger@hotmail.de> >>> Date: Sat, 23 Nov 2019 07:37:26 +0100 >>> Subject: [PATCH 1/2] Fix setting breakpoints or stepping on line 65535 >>> >> >> Bernd, >> >> Thanks for looking into this, and especially thanks for adding a test! >> >> Normally you should include the git commit message and ChangeLog along >> with your patch submission so that these can be reviewed too. 'git >> format-patch' and 'git send-email' can be useful for this, if you can >> get them setup. >> > > Okay, I added changelog messages for gdb/ChangeLog and gdb/testsuite/ChangeLog, > and improved the commit messages. > I hope you can handle the format-patch files as attachments. > >> Given the age of the code you're removing I think this change sounds >> reasonable. I assume there's no test that covers why this code should >> be there, so you see no regressions with this code removed? >> > > No, there were no regressions (last full test on 24-11-19). > >> I have a couple of minor issues with the test. If you address those >> and repost with commit message and ChangeLog this can be approved. >> > > Fixed the test. > > Thanks a lot for your review. > Is it OK for trunk? Thanks Bernd, this is OK. For the future, I think it would be reasonable to put the fix and the test in the same patch. It would also help somebody wondering why we have such a weird test trace it back to the fix, and the explanation you have put in the first patch. But it's also fine like this. I don't remember, do you have an account to push patches now? Simon
* Simon Marchi <simark@simark.ca> [2019-12-29 14:45:13 -0500]: > On 2019-12-02 1:04 p.m., Bernd Edlinger wrote: > > Hi Andrew, > > > > On 12/1/19 11:08 PM, Andrew Burgess wrote: > >> * Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-11-24 11:54:23 +0000]: > >> > >>> Hi, > >>> > >>> this removes code that is present from the very first git revisison > >>> 7b4ac7e1ed2c4616bce56d1760807798be87ac9e from 1988. It was in the > >>> gdb/dbxread.c at the time (and makes more sense for dbx line info format > >>> since line numbers are 16-bit entities in that debug format and debugging > >>> files with more than 65535 lines would not work anyway) but moved from > >>> there to gdb/buildsym.c which is used for dwarf line info as well, and > >>> excluding an arbitrary line number does certainly not make sense nowadays. > >>> > >>> > >>> Thanks > >>> Bernd. > >> > >>> From f202ae765b72ad6d17600eb661993a63191309f7 Mon Sep 17 00:00:00 2001 > >>> From: Bernd Edlinger <bernd.edlinger@hotmail.de> > >>> Date: Sat, 23 Nov 2019 07:37:26 +0100 > >>> Subject: [PATCH 1/2] Fix setting breakpoints or stepping on line 65535 > >>> > >> > >> Bernd, > >> > >> Thanks for looking into this, and especially thanks for adding a test! > >> > >> Normally you should include the git commit message and ChangeLog along > >> with your patch submission so that these can be reviewed too. 'git > >> format-patch' and 'git send-email' can be useful for this, if you can > >> get them setup. > >> > > > > Okay, I added changelog messages for gdb/ChangeLog and gdb/testsuite/ChangeLog, > > and improved the commit messages. > > I hope you can handle the format-patch files as attachments. > > > >> Given the age of the code you're removing I think this change sounds > >> reasonable. I assume there's no test that covers why this code should > >> be there, so you see no regressions with this code removed? > >> > > > > No, there were no regressions (last full test on 24-11-19). > > > >> I have a couple of minor issues with the test. If you address those > >> and repost with commit message and ChangeLog this can be approved. > >> > > > > Fixed the test. > > > > Thanks a lot for your review. > > Is it OK for trunk? > > Thanks Bernd, this is OK. For the future, I think it would be reasonable to put the > fix and the test in the same patch. It would also help somebody wondering why we have > such a weird test trace it back to the fix, and the explanation you have put in the first > patch. But it's also fine like this. I place more value on having tests and changes in the same commit - I find it more useful the other way around, when I find some code I don't understand and I figure out which commit it came from, having the tests in the same commit means it is easy to know "this test" should check "this code". If the test and changes are separate I have to do more work. Boo! For this reason, if you've not already pushed this patch, could you collapse the test and change into a single commit please before pushing. Thanks, Andrew
On 2019-12-29 3:32 p.m., Andrew Burgess wrote: > I place more value on having tests and changes in the same commit - I > find it more useful the other way around, when I find some code I > don't understand and I figure out which commit it came from, having > the tests in the same commit means it is easy to know "this test" > should check "this code". > > If the test and changes are separate I have to do more work. Boo! > > For this reason, if you've not already pushed this patch, could you > collapse the test and change into a single commit please before > pushing. FWIW, that's my preference too. Simon
On 12/29/19 9:41 PM, Simon Marchi wrote: > On 2019-12-29 3:32 p.m., Andrew Burgess wrote: >> I place more value on having tests and changes in the same commit - I >> find it more useful the other way around, when I find some code I >> don't understand and I figure out which commit it came from, having >> the tests in the same commit means it is easy to know "this test" >> should check "this code". >> >> If the test and changes are separate I have to do more work. Boo! >> >> For this reason, if you've not already pushed this patch, could you >> collapse the test and change into a single commit please before >> pushing. > > FWIW, that's my preference too. > Okay, will do. Thanks!
From 46fe1ccfb2a1df701cb3bfeb6f882ca49f11ec0f Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Sun, 24 Nov 2019 09:37:22 +0100 Subject: [PATCH 2/2] Add a test case for line 65535 This adds a test case for setting a breakpoint on line 65535. --- gdb/testsuite/gdb.base/line65535.c | 19 +++++++++++++++++++ gdb/testsuite/gdb.base/line65535.exp | 28 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 gdb/testsuite/gdb.base/line65535.c create mode 100644 gdb/testsuite/gdb.base/line65535.exp diff --git a/gdb/testsuite/gdb.base/line65535.c b/gdb/testsuite/gdb.base/line65535.c new file mode 100644 index 0000000..d80a294 --- /dev/null +++ b/gdb/testsuite/gdb.base/line65535.c @@ -0,0 +1,19 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2019 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 <http://www.gnu.org/licenses/>. */ + +#line 65535 "line65535.c" +int main() { return 0; } diff --git a/gdb/testsuite/gdb.base/line65535.exp b/gdb/testsuite/gdb.base/line65535.exp new file mode 100644 index 0000000..095f151 --- /dev/null +++ b/gdb/testsuite/gdb.base/line65535.exp @@ -0,0 +1,28 @@ +# Copyright 2019 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 <http://www.gnu.org/licenses/>. + +# This test tries to place a breakpoint on line 65535. +# For the purpose of this test we are satisfied if the break +# command succeeds, we will not try to actually run to that line. + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return -1 +} + +gdb_test "break $srcfile:65535" \ + ".*Breakpoint 1 at .*: file $srcfile, line 65535\\..*" \ + "break at line 65535" -- 1.9.1