Advice on fixing gdb/12528

Advice on fixing gdb/12528

[Paul Pluzhnikov]

Greetings,

Doug and myself just independently hit
http://sourceware.org/bugzilla/show_bug.cgi?id=12528 (and 12568).

I would like advice on fixing it.

I know that in general GDB can not ignore code at location 0,
but it appears inevitable that it must do so on platforms where
- it is known that no code can execute there, and
- linker relocates debug info to address 0 to indicate that
  the corresponding code has been discarded.

So would something like:

 case DW_LNE_set_address:
   address = read_address (abfd, line_ptr, cu, &bytes_read);
   if (!target->to_valid_code_address (address)) {
      // this debug line info corresponds to function that has
      // been GCd by the linker.  Skip to end_sequence.
   }

in dwarf_decode_lines() ?

Thanks,
-- 
Paul Pluzhnikov

Re: Advice on fixing gdb/12528

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> I know that in general GDB can not ignore code at location 0,
Paul> but it appears inevitable that it must do so on platforms where
Paul> - it is known that no code can execute there, and
Paul> - linker relocates debug info to address 0 to indicate that
Paul>   the corresponding code has been discarded.

Paul> So would something like:

Paul>  case DW_LNE_set_address:
Paul>    address = read_address (abfd, line_ptr, cu, &bytes_read);
Paul>    if (!target->to_valid_code_address (address)) {
Paul>       // this debug line info corresponds to function that has
Paul>       // been GCd by the linker.  Skip to end_sequence.
Paul>    }

Paul> in dwarf_decode_lines() ?

Can you check the has_section_at_zero flag on dwarf2_per_objfile?

Tom

[patch] Re: Advice on fixing gdb/12528

[Paul Pluzhnikov]

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

On Fri, Mar 11, 2011 at 9:14 AM, Tom Tromey <tromey@redhat.com> wrote:

> Can you check the has_section_at_zero flag on dwarf2_per_objfile?

Thanks!

Attached patch fixes the problem and adds a test case.
Tested on Linux/x86_64, no regressions.

-- 
Paul Pluzhnikov

ChangeLog:

2011-03-14  Paul Pluzhnikov  <ppluzhnikov@google.com>

	PR gdb/12528
	* dwarf2read.c (noop_record_line): New function.
	(dwarf_decode_lines): Ignore line tables for GCd functions.

testsuite/ChangeLog:

2011-03-14  Paul Pluzhnikov  <ppluzhnikov@google.com>

	PR gdb/12528
	* gdb.cp/pr12528.exp: New test.
	* gdb.cp/pr12528.cc: New file.

[-- Attachment #2: gdb-pr12528-20110314.txt --]
[-- Type: text/plain, Size: 6423 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.508
diff -u -p -p -u -r1.508 dwarf2read.c
--- dwarf2read.c	9 Mar 2011 07:07:55 -0000	1.508
+++ dwarf2read.c	14 Mar 2011 20:52:11 -0000
@@ -10341,6 +10341,14 @@ psymtab_include_file_name (const struct 
   return include_name;
 }
 
+/* Ignore this record_line request.  */
+
+static void
+noop_record_line (struct subfile *subfile, int line, CORE_ADDR pc)
+{
+  return;
+}
+
 /* Decode the Line Number Program (LNP) for the given line_header
    structure and CU.  The actual information extracted and the type
    of structures created from the LNP depends on the value of PST.
@@ -10376,6 +10384,8 @@ dwarf_decode_lines (struct line_header *
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   const int decode_for_pst_p = (pst != NULL);
   struct subfile *last_subfile = NULL, *first_subfile = current_subfile;
+  void (*p_record_line) (struct subfile *subfile, int line, CORE_ADDR pc)
+    = record_line;
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -10445,13 +10455,13 @@ dwarf_decode_lines (struct line_header *
 			{
 			  addr = gdbarch_addr_bits_remove (gdbarch, address);
 			  if (last_subfile)
-			    record_line (last_subfile, 0, addr);
+			    (*p_record_line) (last_subfile, 0, addr);
 			  last_subfile = current_subfile;
 			}
 		      /* Append row to matrix using current values.  */
 		      addr = check_cu_functions (address, cu);
 		      addr = gdbarch_addr_bits_remove (gdbarch, addr);
-		      record_line (current_subfile, line, addr);
+		      (*p_record_line) (current_subfile, line, addr);
 		    }
 		}
 	      basic_block = 0;
@@ -10468,12 +10478,19 @@ dwarf_decode_lines (struct line_header *
 	      switch (extended_op)
 		{
 		case DW_LNE_end_sequence:
+		  p_record_line = record_line;
 		  end_sequence = 1;
 		  break;
 		case DW_LNE_set_address:
 		  address = read_address (abfd, line_ptr, cu, &bytes_read);
 		  op_index = 0;
 		  line_ptr += bytes_read;
+
+		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
+		    /* This line table is for a function which has been
+		       GCd by the linker.  Ignore it.  PR gdb/12528 */
+		    p_record_line = noop_record_line;
+
 		  address += baseaddr;
 		  break;
 		case DW_LNE_define_file:
@@ -10528,12 +10545,12 @@ dwarf_decode_lines (struct line_header *
 			{
 			  addr = gdbarch_addr_bits_remove (gdbarch, address);
 			  if (last_subfile)
-			    record_line (last_subfile, 0, addr);
+			    (*p_record_line) (last_subfile, 0, addr);
 			  last_subfile = current_subfile;
 			}
 		      addr = check_cu_functions (address, cu);
 		      addr = gdbarch_addr_bits_remove (gdbarch, addr);
-		      record_line (current_subfile, line, addr);
+		      (*p_record_line) (current_subfile, line, addr);
 		    }
 		}
 	      basic_block = 0;
@@ -10632,7 +10649,7 @@ dwarf_decode_lines (struct line_header *
           if (!decode_for_pst_p)
 	    {
 	      addr = gdbarch_addr_bits_remove (gdbarch, address);
-	      record_line (current_subfile, 0, addr);
+	      (*p_record_line) (current_subfile, 0, addr);
 	    }
         }
     }
Index: testsuite/gdb.cp/pr12528.cc
===================================================================
RCS file: testsuite/gdb.cp/pr12528.cc
diff -N testsuite/gdb.cp/pr12528.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr12528.cc	14 Mar 2011 20:52:11 -0000
@@ -0,0 +1,41 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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 case for PR gdb/12528
+
+struct foo
+{
+  foo (int x);
+  int _x;
+};
+
+struct foo2 : public foo
+{
+  foo2 (int x);
+};
+
+foo::foo (int x) : _x (x) { }
+
+foo2::foo2 (int x) : foo (x) { }
+
+int
+main ()
+{
+  foo2 bar (3);
+  return 0;
+}
Index: testsuite/gdb.cp/pr12528.exp
===================================================================
RCS file: testsuite/gdb.cp/pr12528.exp
diff -N testsuite/gdb.cp/pr12528.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.cp/pr12528.exp	14 Mar 2011 20:52:11 -0000
@@ -0,0 +1,55 @@
+# Copyright 2011 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 file is part of the gdb testsuite
+
+# Test casting, especially between class types or pointer-to-class
+# types.
+
+# This file is part of the gdb testsuite
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+#
+# test running programs
+#
+if { [skip_cplus_tests] } { continue }
+
+set testfile "pr12528"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1;
+}
+
+set additional_flags {-ffunction-sections -Wl,--gc-sections}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+	   [list debug c++ additional_flags=$additional_flags]] != "" } {
+    untested $srcfile
+    return -1
+}
+
+clean_restart $testfile
+
+if { ![runto_main] } {
+    fail "Can't run to main"
+    return
+}
+
+gdb_breakpoint "foo::foo"
+gdb_test "continue" "Breakpoint .*foo::foo.*"

Re: [patch] Re: Advice on fixing gdb/12528

[Jan Kratochvil]

On Mon, 14 Mar 2011 22:01:44 +0100, Paul Pluzhnikov wrote:
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.cp/pr12528.exp	14 Mar 2011 20:52:11 -0000

I would prefer any non-numeric name of the testfile.


> +set additional_flags {-ffunction-sections -Wl,--gc-sections}
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
> +	   [list debug c++ additional_flags=$additional_flags]] != "" } {
> +    untested $srcfile
> +    return -1
> +}

This testfile correctly breaks without the fix applied with:
	g++ (GCC) 4.4.6 20110314 (prerelease)

but it gives false PASS even without the fix applied with:
	g++ (GCC) 4.5.3 20110314 (prerelease)
	g++ (GCC) 4.6.0 20110312 (experimental)

as recent GCCs no longer create multiple ctors in such case.

For a real testcase it should be in gdb.dwarf2/ (and then the special
compilation options are sure no longer needed).


> +if { ![runto_main] } {
> +    fail "Can't run to main"
> +    return
> +}

It could check the `info break' output without running the testcase at all.
Nothing important.



Thanks,
Jan

Re: [patch] Re: Advice on fixing gdb/12528

[Paul Pluzhnikov]

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

On Mon, Mar 14, 2011 at 5:41 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Mon, 14 Mar 2011 22:01:44 +0100, Paul Pluzhnikov wrote:
>> --- /dev/null 1 Jan 1970 00:00:00 -0000
>> +++ testsuite/gdb.cp/pr12528.exp      14 Mar 2011 20:52:11 -0000
>
> I would prefer any non-numeric name of the testfile.

How does "break-on-linker-gcd-function.exp" sound?

>> +set additional_flags {-ffunction-sections -Wl,--gc-sections}
>> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
>> +        [list debug c++ additional_flags=$additional_flags]] != "" } {
>> +    untested $srcfile
>> +    return -1
>> +}
>
> This testfile correctly breaks without the fix applied with:
>        g++ (GCC) 4.4.6 20110314 (prerelease)
>
> but it gives false PASS even without the fix applied with:
>        g++ (GCC) 4.5.3 20110314 (prerelease)
>        g++ (GCC) 4.6.0 20110312 (experimental)
>
> as recent GCCs no longer create multiple ctors in such case.

Yes: I noted that in gdb/12568.

I am not sure how I can trigger the bug with newer GCC versions...
Ah yes, I can.

> For a real testcase it should be in gdb.dwarf2/ (and then the special
> compilation options are sure no longer needed).

Sorry, I didn't follow that part. Why special options wouldn't be needed?

>> +if { ![runto_main] } {
>> +    fail "Can't run to main"
>> +    return
>> +}
>
> It could check the `info break' output without running the testcase at all.

Done.

Thanks,
-- 
Paul Pluzhnikov


ChangeLog:

2011-03-14  Paul Pluzhnikov  <ppluzhnikov@google.com>

       PR gdb/12528
       * dwarf2read.c (noop_record_line): New function.
       (dwarf_decode_lines): Ignore line tables for GCd functions.

testsuite/ChangeLog:

2011-03-14  Paul Pluzhnikov  <ppluzhnikov@google.com>

       PR gdb/12528
       * gdb.dwarf2/break-on-linker-gcd-function.exp: New test.
       * gdb.dwarf2/break-on-linker-gcd-function.cc: New file.

[-- Attachment #2: gdb-pr12528-20110314a.txt --]
[-- Type: text/plain, Size: 6655 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.508
diff -u -p -p -u -r1.508 dwarf2read.c
--- dwarf2read.c	9 Mar 2011 07:07:55 -0000	1.508
+++ dwarf2read.c	15 Mar 2011 01:38:16 -0000
@@ -10341,6 +10341,14 @@ psymtab_include_file_name (const struct 
   return include_name;
 }
 
+/* Ignore this record_line request.  */
+
+static void
+noop_record_line (struct subfile *subfile, int line, CORE_ADDR pc)
+{
+  return;
+}
+
 /* Decode the Line Number Program (LNP) for the given line_header
    structure and CU.  The actual information extracted and the type
    of structures created from the LNP depends on the value of PST.
@@ -10376,6 +10384,8 @@ dwarf_decode_lines (struct line_header *
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   const int decode_for_pst_p = (pst != NULL);
   struct subfile *last_subfile = NULL, *first_subfile = current_subfile;
+  void (*p_record_line) (struct subfile *subfile, int line, CORE_ADDR pc)
+    = record_line;
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -10445,13 +10455,13 @@ dwarf_decode_lines (struct line_header *
 			{
 			  addr = gdbarch_addr_bits_remove (gdbarch, address);
 			  if (last_subfile)
-			    record_line (last_subfile, 0, addr);
+			    (*p_record_line) (last_subfile, 0, addr);
 			  last_subfile = current_subfile;
 			}
 		      /* Append row to matrix using current values.  */
 		      addr = check_cu_functions (address, cu);
 		      addr = gdbarch_addr_bits_remove (gdbarch, addr);
-		      record_line (current_subfile, line, addr);
+		      (*p_record_line) (current_subfile, line, addr);
 		    }
 		}
 	      basic_block = 0;
@@ -10468,12 +10478,19 @@ dwarf_decode_lines (struct line_header *
 	      switch (extended_op)
 		{
 		case DW_LNE_end_sequence:
+		  p_record_line = record_line;
 		  end_sequence = 1;
 		  break;
 		case DW_LNE_set_address:
 		  address = read_address (abfd, line_ptr, cu, &bytes_read);
 		  op_index = 0;
 		  line_ptr += bytes_read;
+
+		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
+		    /* This line table is for a function which has been
+		       GCd by the linker.  Ignore it.  PR gdb/12528 */
+		    p_record_line = noop_record_line;
+
 		  address += baseaddr;
 		  break;
 		case DW_LNE_define_file:
@@ -10528,12 +10545,12 @@ dwarf_decode_lines (struct line_header *
 			{
 			  addr = gdbarch_addr_bits_remove (gdbarch, address);
 			  if (last_subfile)
-			    record_line (last_subfile, 0, addr);
+			    (*p_record_line) (last_subfile, 0, addr);
 			  last_subfile = current_subfile;
 			}
 		      addr = check_cu_functions (address, cu);
 		      addr = gdbarch_addr_bits_remove (gdbarch, addr);
-		      record_line (current_subfile, line, addr);
+		      (*p_record_line) (current_subfile, line, addr);
 		    }
 		}
 	      basic_block = 0;
@@ -10632,7 +10649,7 @@ dwarf_decode_lines (struct line_header *
           if (!decode_for_pst_p)
 	    {
 	      addr = gdbarch_addr_bits_remove (gdbarch, address);
-	      record_line (current_subfile, 0, addr);
+	      (*p_record_line) (current_subfile, 0, addr);
 	    }
         }
     }
Index: testsuite/gdb.dwarf2/break-on-linker-gcd-function.exp
===================================================================
RCS file: testsuite/gdb.dwarf2/break-on-linker-gcd-function.exp
diff -N testsuite/gdb.dwarf2/break-on-linker-gcd-function.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/break-on-linker-gcd-function.exp	15 Mar 2011 01:38:16 -0000
@@ -0,0 +1,51 @@
+# Copyright 2011 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 file is part of the gdb testsuite
+
+# Test casting, especially between class types or pointer-to-class
+# types.
+
+# This file is part of the gdb testsuite
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+#
+# test running programs
+#
+if { [skip_cplus_tests] } { continue }
+
+set testfile "break-on-linker-gcd-function"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1;
+}
+
+set additional_flags {-ffunction-sections -Wl,--gc-sections}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+	   [list debug c++ additional_flags=$additional_flags]] != "" } {
+    untested $srcfile
+    return -1
+}
+
+clean_restart $testfile
+
+# This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
+# but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
+gdb_test "b [gdb_get_line_number "gdb break here"]" "Breakpoint \[0-9\] at 0x\[0-9a-f\]\[0-9a-f\]+: .*"
Index: testsuite/gdb.dwarf2/break-on-linker-gcd-function.cc
===================================================================
RCS file: testsuite/gdb.dwarf2/break-on-linker-gcd-function.cc
diff -N testsuite/gdb.dwarf2/break-on-linker-gcd-function.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/break-on-linker-gcd-function.cc	15 Mar 2011 01:38:16 -0000
@@ -0,0 +1,32 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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 case for PR gdb/12528
+
+void
+foo ()
+{
+  // This function is not referenced and should be GCd by the linker
+  return; // gdb break here
+}
+
+int
+main ()
+{
+  return 0;
+}

Re: [patch] Re: Advice on fixing gdb/12528

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> How does "break-on-linker-gcd-function.exp" sound?

Fine by me.

Paul> Yes: I noted that in gdb/12568.
Paul> I am not sure how I can trigger the bug with newer GCC versions...
Paul> Ah yes, I can.

Jan> For a real testcase it should be in gdb.dwarf2/ (and then the special
Jan> compilation options are sure no longer needed).

Paul> Sorry, I didn't follow that part. Why special options wouldn't be needed?

What Jan means is that you can put a .S file into gdb.dwarf2.
That way the test is independent of compiler version (but unfortunately
then dependent on arch).

Tom

Re: [patch] Re: Advice on fixing gdb/12528

[Jan Kratochvil]

On Tue, 15 Mar 2011 16:29:53 +0100, Tom Tromey wrote:
> >>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:
> Jan> For a real testcase it should be in gdb.dwarf2/ (and then the special
> Jan> compilation options are sure no longer needed).
> 
> Paul> Sorry, I didn't follow that part. Why special options wouldn't be needed?
> 
> What Jan means is that you can put a .S file into gdb.dwarf2.
> That way the test is independent of compiler version (but unfortunately
> then dependent on arch).

Or coded like
	gdb.dwarf2/dw2-ref-missing-frame*
with native code while still being arch-independent or the more simple case
	gdb.dwarf2/dw2-filename*
where no native code was needed (and it is also arch-independent).

But in both cases it is a pain to code it by hand.


Regards,
Jan

Re: [patch] Re: Advice on fixing gdb/12528

[Paul Pluzhnikov]

On Tue, Mar 15, 2011 at 8:43 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:

> On Tue, 15 Mar 2011 16:29:53 +0100, Tom Tromey wrote:

>> What Jan means is that you can put a .S file into gdb.dwarf2.
>> That way the test is independent of compiler version (but unfortunately
>> then dependent on arch).
>
> Or coded like
...
> But in both cases it is a pain to code it by hand.

Thanks for clarification.

Since it doesn't appear that I actually have to do it the painful way
(at least not on platforms which support linker --gc-sections),
is it ok to keep the test as is?

It probably belongs to gdb.base then.

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Re: Advice on fixing gdb/12528

[Jan Kratochvil]

On Tue, 15 Mar 2011 02:39:39 +0100, Paul Pluzhnikov wrote:
> ChangeLog:
> 
> 2011-03-14  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
>        PR gdb/12528
>        * dwarf2read.c (noop_record_line): New function.
>        (dwarf_decode_lines): Ignore line tables for GCd functions.
> 
> testsuite/ChangeLog:
> 
> 2011-03-14  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
>        PR gdb/12528
>        * gdb.dwarf2/break-on-linker-gcd-function.exp: New test.
>        * gdb.dwarf2/break-on-linker-gcd-function.cc: New file.

It seems correct to me, please check it in.


>  		case DW_LNE_set_address:
>  		  address = read_address (abfd, line_ptr, cu, &bytes_read);
>  		  op_index = 0;
>  		  line_ptr += bytes_read;
> +
> +		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
> +		    /* This line table is for a function which has been
> +		       GCd by the linker.  Ignore it.  PR gdb/12528 */
> +		    p_record_line = noop_record_line;

There could be a `complaint' call here, correct linker/postprocessor would
discard the .debug_line part along.


> +
>  		  address += baseaddr;
>  		  break;

This opcode is the only one settings ADDRESS to absolute value (using
BASEADDR) so I believe it is correct.


> +# This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
> +# but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
> +gdb_test "b [gdb_get_line_number "gdb break here"]" "Breakpoint \[0-9\] at 0x\[0-9a-f\]\[0-9a-f\]+: .*"

I was thinking if some prologue cannot be >=0x10 due to some alignments etc.
But neither ia64 (0x1 - still in the first bundle) nor s390x (0xa) exceed the
0x10 limit so it is probably OK.  A regression would be found at least on x86*
boxes anyway.


Thanks,
Jan

Re: [patch] Re: Advice on fixing gdb/12528

[Paul Pluzhnikov]

On Tue, Mar 15, 2011 at 11:36 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:

>>        PR gdb/12528
>>        * gdb.dwarf2/break-on-linker-gcd-function.exp: New test.
>>        * gdb.dwarf2/break-on-linker-gcd-function.cc: New file.
>
> It seems correct to me, please check it in.

Is location of the test case ok?

The same problem could well exist on non-dwarf platforms.

I also noticed that gdb.dwarf2/Makefile.in would need to have
break-on-linker-gcd-function added to EXECUTABLES (or the executable
renamed to break-on-linker-gcd-function.x).

>> +# This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
>> +# but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
>> +gdb_test "b [gdb_get_line_number "gdb break here"]" "Breakpoint \[0-9\] at 0x\[0-9a-f\]\[0-9a-f\]+: .*"
>
> I was thinking if some prologue cannot be >=0x10 due to some alignments etc.

Perhaps "[$hex][$hex][$hex]+" to weed out up to 0xff ?

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Re: Advice on fixing gdb/12528

[Jan Kratochvil]

On Tue, 15 Mar 2011 19:49:47 +0100, Paul Pluzhnikov wrote:
> On Tue, Mar 15, 2011 at 11:36 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> >>        PR gdb/12528
> >>        * gdb.dwarf2/break-on-linker-gcd-function.exp: New test.
> >>        * gdb.dwarf2/break-on-linker-gcd-function.cc: New file.
> >
> > It seems correct to me, please check it in.
> 
> Is location of the test case ok?

It is not - it is not the explicit DWARF code, it should be gdb.base/ as you
write, thanks.


> I also noticed that gdb.dwarf2/Makefile.in would need to have
> break-on-linker-gcd-function added to EXECUTABLES (or the executable
> renamed to break-on-linker-gcd-function.x).

It has been recently fixed by Michael Snyder so it should be maintained now.
EXECUTABLES should be updated, I agree.


> >> +# This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
> >> +# but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
> >> +gdb_test "b [gdb_get_line_number "gdb break here"]" "Breakpoint \[0-9\] at 0x\[0-9a-f\]\[0-9a-f\]+: .*"
> >
> > I was thinking if some prologue cannot be >=0x10 due to some alignments etc.
> 
> Perhaps "[$hex][$hex][$hex]+" to weed out up to 0xff ?

But some embedded targets can fit the whole program / main under 0x100?

I would keep it as is, there is only a risk of false PASS, not false FAIL.
And the real FAIL would be caught by x86*.


Thanks,
Jan

Re: [patch] Re: Advice on fixing gdb/12528

[Paul Pluzhnikov]

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

On Tue, Mar 15, 2011 at 12:03 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:

> I would keep it as is, there is only a risk of false PASS, not false FAIL.
> And the real FAIL would be caught by x86*.

Thanks.

I'll commit attached patch tomorrow if there are no further comments.

-- 
Paul Pluzhnikov

ChangeLog:

2011-03-15  Paul Pluzhnikov  <ppluzhnikov@google.com>

      PR gdb/12528
      * dwarf2read.c (noop_record_line): New function.
      (dwarf_decode_lines): Ignore line tables for GCd functions.

testsuite/ChangeLog:

2011-03-15  Paul Pluzhnikov  <ppluzhnikov@google.com>

      PR gdb/12528
      * gdb.base/Makefile.in: Adjust EXECUTABLES.
      * gdb.base/break-on-linker-gcd-function.exp: New test.
      * gdb.base/break-on-linker-gcd-function.cc: New file.

[-- Attachment #2: gdb-pr12528-20110315.txt --]
[-- Type: text/plain, Size: 7727 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.509
diff -u -p -p -u -r1.509 dwarf2read.c
--- dwarf2read.c	15 Mar 2011 15:57:11 -0000	1.509
+++ dwarf2read.c	15 Mar 2011 19:18:30 -0000
@@ -10364,6 +10364,14 @@ psymtab_include_file_name (const struct 
   return include_name;
 }
 
+/* Ignore this record_line request.  */
+
+static void
+noop_record_line (struct subfile *subfile, int line, CORE_ADDR pc)
+{
+  return;
+}
+
 /* Decode the Line Number Program (LNP) for the given line_header
    structure and CU.  The actual information extracted and the type
    of structures created from the LNP depends on the value of PST.
@@ -10399,6 +10407,8 @@ dwarf_decode_lines (struct line_header *
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   const int decode_for_pst_p = (pst != NULL);
   struct subfile *last_subfile = NULL, *first_subfile = current_subfile;
+  void (*p_record_line) (struct subfile *subfile, int line, CORE_ADDR pc)
+    = record_line;
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -10468,13 +10478,13 @@ dwarf_decode_lines (struct line_header *
 			{
 			  addr = gdbarch_addr_bits_remove (gdbarch, address);
 			  if (last_subfile)
-			    record_line (last_subfile, 0, addr);
+			    (*p_record_line) (last_subfile, 0, addr);
 			  last_subfile = current_subfile;
 			}
 		      /* Append row to matrix using current values.  */
 		      addr = check_cu_functions (address, cu);
 		      addr = gdbarch_addr_bits_remove (gdbarch, addr);
-		      record_line (current_subfile, line, addr);
+		      (*p_record_line) (current_subfile, line, addr);
 		    }
 		}
 	      basic_block = 0;
@@ -10491,12 +10501,24 @@ dwarf_decode_lines (struct line_header *
 	      switch (extended_op)
 		{
 		case DW_LNE_end_sequence:
+		  p_record_line = record_line;
 		  end_sequence = 1;
 		  break;
 		case DW_LNE_set_address:
 		  address = read_address (abfd, line_ptr, cu, &bytes_read);
 		  op_index = 0;
 		  line_ptr += bytes_read;
+
+		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
+		    {
+		      /* This line table is for a function which has been
+			 GCd by the linker.  Ignore it.  PR gdb/12528 */
+
+		      complaint (&symfile_complaints,
+				 _(".debug_line section at address 0"));
+		      p_record_line = noop_record_line;
+		    }
+
 		  address += baseaddr;
 		  break;
 		case DW_LNE_define_file:
@@ -10551,12 +10573,12 @@ dwarf_decode_lines (struct line_header *
 			{
 			  addr = gdbarch_addr_bits_remove (gdbarch, address);
 			  if (last_subfile)
-			    record_line (last_subfile, 0, addr);
+			    (*p_record_line) (last_subfile, 0, addr);
 			  last_subfile = current_subfile;
 			}
 		      addr = check_cu_functions (address, cu);
 		      addr = gdbarch_addr_bits_remove (gdbarch, addr);
-		      record_line (current_subfile, line, addr);
+		      (*p_record_line) (current_subfile, line, addr);
 		    }
 		}
 	      basic_block = 0;
@@ -10655,7 +10677,7 @@ dwarf_decode_lines (struct line_header *
           if (!decode_for_pst_p)
 	    {
 	      addr = gdbarch_addr_bits_remove (gdbarch, address);
-	      record_line (current_subfile, 0, addr);
+	      (*p_record_line) (current_subfile, 0, addr);
 	    }
         }
     }
Index: testsuite/gdb.base/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/Makefile.in,v
retrieving revision 1.9
diff -u -p -p -u -r1.9 Makefile.in
--- testsuite/gdb.base/Makefile.in	22 Feb 2011 20:52:46 -0000	1.9
+++ testsuite/gdb.base/Makefile.in	15 Mar 2011 19:18:30 -0000
@@ -5,7 +5,8 @@ EXECUTABLES = a2-run advance all-types a
 	annota3 anon args arrayidx async attach attach-pie-misread \
 	attach2 auxv bang\! bfp-test bigcore bitfields bitfields2 \
 	break break-always break-entry break-interp-test breako2 \
-	breakpoint-shadow call-ar-st call-rt-st call-sc-t* call-signals \
+	breakpoint-shadow break-on-linker-gcd-function \
+	call-ar-st call-rt-st call-sc-t* call-signals \
 	call-strs callexit callfuncs callfwmall charset checkpoint \
 	chng-syms code_elim1 code_elim2 commands compiler complex \
 	condbreak consecutive constvars coremaker cursal cvexpr \
Index: testsuite/gdb.base/break-on-linker-gcd-function.cc
===================================================================
RCS file: testsuite/gdb.base/break-on-linker-gcd-function.cc
diff -N testsuite/gdb.base/break-on-linker-gcd-function.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/break-on-linker-gcd-function.cc	15 Mar 2011 19:18:30 -0000
@@ -0,0 +1,32 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 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 case for PR gdb/12528
+
+void
+foo ()
+{
+  // This function is not referenced and should be GCd by the linker
+  return; // gdb break here
+}
+
+int
+main ()
+{
+  return 0;
+}
Index: testsuite/gdb.base/break-on-linker-gcd-function.exp
===================================================================
RCS file: testsuite/gdb.base/break-on-linker-gcd-function.exp
diff -N testsuite/gdb.base/break-on-linker-gcd-function.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/break-on-linker-gcd-function.exp	15 Mar 2011 19:18:30 -0000
@@ -0,0 +1,54 @@
+# Copyright 2011 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 file is part of the gdb testsuite
+
+# Test casting, especially between class types or pointer-to-class
+# types.
+
+# This file is part of the gdb testsuite
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+#
+# test running programs
+#
+if { [skip_cplus_tests] } { continue }
+
+set testfile "break-on-linker-gcd-function"
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+
+if [get_compiler_info ${binfile} "c++"] {
+    return -1;
+}
+
+set additional_flags {-ffunction-sections -Wl,--gc-sections}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+	   [list debug c++ additional_flags=$additional_flags]] != "" } {
+    untested $srcfile
+    return -1
+}
+
+clean_restart $testfile
+
+# Single hex digit
+set xd {[0-9a-f]}
+
+# This accepts e.g. "Breakpoint 1 at 0x40968a" (fixed GDB)
+# but rejects e.g. "Breakpoint 1 at 0x4" (broken GDB).
+gdb_test "b [gdb_get_line_number "gdb break here"]" "Breakpoint \[0-9\] at 0x${xd}${xd}+: .*"

Re: [patch] Re: Advice on fixing gdb/12528

[Jan Kratochvil]

On Tue, 15 Mar 2011 20:27:56 +0100, Paul Pluzhnikov wrote:
> +		      /* This line table is for a function which has been
> +			 GCd by the linker.  Ignore it.  PR gdb/12528 */
> +
> +		      complaint (&symfile_complaints,
> +				 _(".debug_line section at address 0"));

		      complaint (&symfile_complaints,
				 _(".debug_line offset 0x%lx uses address 0 "
				   "[in module %s]"),
				 (long) (line_ptr
					 - dwarf2_per_objfile->line.buffer),
				 cu->objfile->name);

(the offset is not right but better than nothing)


Thanks,
Jan

Re: [patch] Re: Advice on fixing gdb/12528

[Paul Pluzhnikov]

On Tue, Mar 15, 2011 at 2:24 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 15 Mar 2011 20:27:56 +0100, Paul Pluzhnikov wrote:
>> +                   /* This line table is for a function which has been
>> +                      GCd by the linker.  Ignore it.  PR gdb/12528 */
>> +
>> +                   complaint (&symfile_complaints,
>> +                              _(".debug_line section at address 0"));
>
>                      complaint (&symfile_complaints,
>                                 _(".debug_line offset 0x%lx uses address 0 "
>                                   "[in module %s]"),
>                                 (long) (line_ptr
>                                         - dwarf2_per_objfile->line.buffer),
>                                 cu->objfile->name);
>
> (the offset is not right but better than nothing)

Maybe like this:

		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
		    {
		      /* This line table is for a function which has been
			 GCd by the linker.  Ignore it.  PR gdb/12528 */

		      long line_offset
			= line_ptr - bytes_read - dwarf2_per_objfile->line.buffer;

		      complaint (&symfile_complaints,
				 _(".debug_line offset 0x%lx uses address 0 "
				   "[in module %s]"),
				 line_offset, cu->objfile->name);
		      p_record_line = noop_record_line;
		    }


Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Re: Advice on fixing gdb/12528

[Jan Kratochvil]

On Tue, 15 Mar 2011 22:45:02 +0100, Paul Pluzhnikov wrote:
> On Tue, Mar 15, 2011 at 2:24 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> >                      complaint (&symfile_complaints,
> >                                 _(".debug_line offset 0x%lx uses address 0 "
> >                                   "[in module %s]"),
> >                                 (long) (line_ptr
> >                                         - dwarf2_per_objfile->line.buffer),
> >                                 cu->objfile->name);
> >
> > (the offset is not right but better than nothing)
> 
> Maybe like this:
> 
> 		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
> 		    {
> 		      /* This line table is for a function which has been
> 			 GCd by the linker.  Ignore it.  PR gdb/12528 */
> 
> 		      long line_offset
> 			= line_ptr - bytes_read - dwarf2_per_objfile->line.buffer;
> 
> 		      complaint (&symfile_complaints,
> 				 _(".debug_line offset 0x%lx uses address 0 "
> 				   "[in module %s]"),
> 				 line_offset, cu->objfile->name);
> 		      p_record_line = noop_record_line;
> 		    }

BYTES_READ is here just for the ADDRESS size.  One can also subtract 1 for
EXTENDED_OP read in, subtract already overwritten BYTES_READ for EXTENDED_LEN
read in and subtract 1 for OP_CODE read in.  This way we get the offset where
this operation starts.

But neither binutils nor elfutils readelf display offsets of the .debug_line
operations so one cannot debug the resulting offset so much which is why
I thought some approx. offset is good enough.

I do not think the displayed offset is so importatnt but at least the module
gets shown.


Thanks,
Jan

Re: [patch] Re: Advice on fixing gdb/12528

[Paul Pluzhnikov]

On Tue, Mar 15, 2011 at 11:06 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:

>> Maybe like this:
>>
>>                 if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
>>                   {
>>                     /* This line table is for a function which has been
>>                        GCd by the linker.  Ignore it.  PR gdb/12528 */
>>
>>                     long line_offset
>>                       = line_ptr - bytes_read - dwarf2_per_objfile->line.buffer;
>>
>>                     complaint (&symfile_complaints,
>>                                _(".debug_line offset 0x%lx uses address 0 "
>>                                  "[in module %s]"),
>>                                line_offset, cu->objfile->name);
>>                     p_record_line = noop_record_line;
>>                   }
>
> BYTES_READ is here just for the ADDRESS size.

Right. So the offset printed above is the offset of the address itself.

> I do not think the displayed offset is so importatnt but at least the module
> gets shown.

I think it's still (slightly) better to print offset to the start of address,
rather than just after it.

How about this then:

		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
		    {
		      /* This line table is for a function which has been
			 GCd by the linker.  Ignore it.  PR gdb/12528 */

		      long line_offset
			= line_ptr - bytes_read - dwarf2_per_objfile->line.buffer;

		      complaint (&symfile_complaints,
				 _(".debug_line address at offset 0x%lx is 0 "
				   "[in module %s]"),
				 line_offset, cu->objfile->name);
		      p_record_line = noop_record_line;
		    }

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Re: Advice on fixing gdb/12528

[Jan Kratochvil]

On Wed, 16 Mar 2011 18:31:50 +0100, Paul Pluzhnikov wrote:
> On Tue, Mar 15, 2011 at 11:06 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> I think it's still (slightly) better to print offset to the start of address,
> rather than just after it.

I agree.


> How about this then:
> 
> 		  if (address == 0 && !dwarf2_per_objfile->has_section_at_zero)
> 		    {
> 		      /* This line table is for a function which has been
> 			 GCd by the linker.  Ignore it.  PR gdb/12528 */
> 
> 		      long line_offset
> 			= line_ptr - bytes_read - dwarf2_per_objfile->line.buffer;

In thise case you can move the line
                  line_ptr += bytes_read;
below this block without having to undo the computation here.

> 
> 		      complaint (&symfile_complaints,
> 				 _(".debug_line address at offset 0x%lx is 0 "
> 				   "[in module %s]"),
> 				 line_offset, cu->objfile->name);
> 		      p_record_line = noop_record_line;
> 		    }

But I do not think it matters much to discuss this `complaint' more.


Thanks,
Jan