[PATCH] Fix setting breakpoints or stepping on line 65535

[PATCH] Fix setting breakpoints or stepping on line 65535

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

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.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-setting-breakpoints-or-stepping-on-line-65535.patch --]
[-- Type: text/x-patch; name="0001-Fix-setting-breakpoints-or-stepping-on-line-65535.patch", Size: 742 bytes --]

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

---
 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Add-a-test-case-for-line-65535.patch --]
[-- Type: text/x-patch; name="0002-Add-a-test-case-for-line-65535.patch", Size: 2688 bytes --]

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/>.
+
+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
-- 
1.9.1

[PING] [PATCH] Fix setting breakpoints or stepping on line 65535

[Bernd Edlinger]

Ping...


On 11/24/19 12:54 PM, Bernd Edlinger wrote:
> 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.
> 

Re: [PATCH] Fix setting breakpoints or stepping on line 65535

[Andrew Burgess]

* 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.

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?

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.

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.

> +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.

> -- 
> 1.9.1
> 

[PATCHv2] Fix setting breakpoints or stepping on line 65535

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 5888 bytes --]

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
>>
> 

[-- Attachment #2: changelog.txt --]
[-- Type: text/plain, Size: 289 bytes --]

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Fix-setting-breakpoints-or-stepping-on-line-65535.patch --]
[-- Type: text/x-patch; name="0001-Fix-setting-breakpoints-or-stepping-on-line-65535.patch", Size: 1242 bytes --]

From 7cee9fbe408af0a2da8e7812c2f86d2b9e4a809e 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

This removes code that was 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.
---
 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-Add-a-test-case-for-line-65535.patch --]
[-- Type: text/x-patch; name="0002-Add-a-test-case-for-line-65535.patch", Size: 2927 bytes --]

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

[PING] [PATCHv2] Fix setting breakpoints or stepping on line 65535

[Bernd Edlinger]

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**2] [PATCHv2] Fix setting breakpoints or stepping on line 65535

[Bernd Edlinger]

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
>>>>
>>>

Re: [PATCHv2] Fix setting breakpoints or stepping on line 65535

[Simon Marchi]

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

Re: [PATCHv2] Fix setting breakpoints or stepping on line 65535

[Andrew Burgess]

* 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

Re: [PATCHv2] Fix setting breakpoints or stepping on line 65535

[Simon Marchi]

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

Re: [PATCHv2] Fix setting breakpoints or stepping on line 65535

[Bernd Edlinger]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 744 bytes --]

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!
\x16º&Öéj×!zÊÞ¶êç×­ºçÉb²Ö«r\x18\x1dn–­r\x17¬