[PATCH] Change symbol::line to unsigned int

[PATCH] Change symbol::line to unsigned int

[Tom Tromey]

A user here at AdaCore noticed that, when debugging a certain program,
a stack frame reported line 34358, where it should have been line
99894.

After debugging a bit, I discovered:

(top) p (99894 & ~65536)
$60 = 34358

That line, symbol::line is too narrow.

This patch widens the member and changes all the uses that currently
use the narrower type.
---
 gdb/compile/compile-c-symbols.c          |  2 +-
 gdb/compile/compile-cplus-symbols.c      |  2 +-
 gdb/compile/compile-cplus-types.c        |  2 +-
 gdb/symtab.h                             | 12 ++----
 gdb/testsuite/gdb.python/lotsa-lines.exp | 51 ++++++++++++++++++++++++
 5 files changed, 58 insertions(+), 11 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/lotsa-lines.exp

diff --git a/gdb/compile/compile-c-symbols.c b/gdb/compile/compile-c-symbols.c
index 0449c37099e..e21689dc25b 100644
--- a/gdb/compile/compile-c-symbols.c
+++ b/gdb/compile/compile-c-symbols.c
@@ -58,7 +58,7 @@ convert_one_symbol (compile_c_instance *context,
 {
   gcc_type sym_type;
   const char *filename = sym.symbol->symtab ()->filename;
-  unsigned short line = sym.symbol->line ();
+  unsigned int line = sym.symbol->line ();
 
   context->error_symbol_once (sym.symbol);
 
diff --git a/gdb/compile/compile-cplus-symbols.c b/gdb/compile/compile-cplus-symbols.c
index 5e504ac839d..ab18e631247 100644
--- a/gdb/compile/compile-cplus-symbols.c
+++ b/gdb/compile/compile-cplus-symbols.c
@@ -49,7 +49,7 @@ convert_one_symbol (compile_cplus_instance *instance,
   /* Squash compiler warning.  */
   gcc_type sym_type = 0;
   const char *filename = sym.symbol->symtab ()->filename;
-  unsigned short line = sym.symbol->line ();
+  unsigned int line = sym.symbol->line ();
 
   instance->error_symbol_once (sym.symbol);
 
diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 47cb26785fc..02cd3810b5d 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -802,7 +802,7 @@ compile_cplus_convert_struct_or_union (compile_cplus_instance *instance,
 				       enum gcc_cp_symbol_kind nested_access)
 {
   const char *filename = nullptr;
-  unsigned short line = 0;
+  unsigned int line = 0;
 
   /* Get the decl name of this type.  */
   gdb::unique_xmalloc_ptr<char> name
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 2fd56ce21bd..5bcf374c8b2 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1293,12 +1293,12 @@ struct symbol : public general_symbol_info, public allocate_on_obstack
     m_type = type;
   }
 
-  unsigned short line () const
+  unsigned int line () const
   {
     return m_line;
   }
 
-  void set_line (unsigned short line)
+  void set_line (unsigned int line)
   {
     m_line = line;
   }
@@ -1461,13 +1461,9 @@ struct symbol : public general_symbol_info, public allocate_on_obstack
      SYMBOL_INLINED set) this is the line number of the function's call
      site.  Inlined function symbols are not definitions, and they are
      never found by symbol table lookup.
-     If this symbol is arch-owned, LINE shall be zero.
+     If this symbol is arch-owned, LINE shall be zero.  */
 
-     FIXME: Should we really make the assumption that nobody will try
-     to debug files longer than 64K lines?  What about machine
-     generated programs?  */
-
-  unsigned short m_line = 0;
+  unsigned int m_line = 0;
 
   /* An arbitrary data pointer, allowing symbol readers to record
      additional information on a per-symbol basis.  Note that this data
diff --git a/gdb/testsuite/gdb.python/lotsa-lines.exp b/gdb/testsuite/gdb.python/lotsa-lines.exp
new file mode 100644
index 00000000000..a3935db9737
--- /dev/null
+++ b/gdb/testsuite/gdb.python/lotsa-lines.exp
@@ -0,0 +1,51 @@
+# Copyright 2023 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/>.
+
+# Test that a very long file is ok.
+
+load_lib gdb-python.exp
+require allow_python_tests
+
+standard_testfile .c
+
+# Create a source file with many lines.
+set c_file [standard_output_file $srcfile]
+
+set chan [open $c_file w]
+puts $chan "int callee (int x) { return x; }"
+for {set i 2} {$i < 65538} {incr i} {
+    puts $chan "int call$i () { return callee ($i); }"
+}
+puts $chan "int main() { return call65537(); }"
+close $chan
+
+if {[prepare_for_testing "failed to prepare" ${testfile} \
+	 [list $c_file] {debug}]} {
+    return
+}
+
+if {![runto callee]} {
+    return
+}
+
+gdb_test "print x" " = 65537"
+
+# This doesn't actually demonstrate the bug, because it takes a code
+# path not checking symbol::line.
+gdb_test "frame 1" "call65537.*lotsa-lines.c:65537.*"
+
+# This is the simplest way to see the problem.
+gdb_test "python print(gdb.selected_frame().function().line)" "65537" \
+    "print function line number"
-- 
2.39.1

Re: [PATCH] Change symbol::line to unsigned int

[Simon Marchi]

On 3/27/23 09:45, Tom Tromey via Gdb-patches wrote:
> A user here at AdaCore noticed that, when debugging a certain program,
> a stack frame reported line 34358, where it should have been line
> 99894.
> 
> After debugging a bit, I discovered:
> 
> (top) p (99894 & ~65536)
> $60 = 34358
> 
> That line, symbol::line is too narrow.
> 
> This patch widens the member and changes all the uses that currently
> use the narrower type.

Looks fine to me:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon