[PATCH] Fix handling of #include files during prologue skipping

[PATCH] Fix handling of #include files during prologue skipping

[Tiago Stürmer Daitx]

In symtab.c:skip_prologue_using_sal, the code goes through the SAL markers 
starting from the beginning of the function, and checks for markers with the 
same (or increasing line numbers). The idea behind this is that if one gets 
decreasing line numbers, than one probably have the case where the optimizer 
scheduled code from later in the function into the prologue.

However, the problem was that this code was simply comparing the line
*numbers* -- even if they refered to a different file! That's why the
presence of the comment makes a difference, because it makes the line
number in t.c larger (or smaller) than the line number in t.h.

This patch simply considers a change in file equivalent to an
increasing line number.

Before the fix:

$ gdb prologue-include
[snip]
(gdb) break main
Breakpoint 1 at 0x10000594: file prologue-include.h, line 2.

After the fix:
$ gdb prologue-include
[snip]
(gdb) break main
Breakpoint 1 at 0x1000058c: file prologue-include.h, line 1.


No regressions detected on PPC32, PPC64, and AMD64.

Regards,
Tiago Daitx



gdb/ChangeLog

2012-11-01  Ulrich Weigand  <uweigand@de.ibm.com>

    * symtab.c (skip_prologue_using_sal): Consider a file
    change the same as an increased file number

gdb/testsuite/ChangeLog

2012-11-01  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>
   
    * gdb.base/prologue-include.c: New file.
    * gdb.base/prologue-include.exp: New file.
    * gdb.base/prologue-include.h: New file.


diff --git a/gdb/symtab.c b/gdb/symtab.c
index d40436a..b200b36 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4905,6 +4905,10 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
 	     line mark the prologue -> body transition.  */
 	  if (sal.line >= prologue_sal.line)
 	    break;
+	  /* Likewise if we are in a different symtab altogether
+	     (e.g. within a file included via #include).  */
+	  if (sal.symtab != prologue_sal.symtab)
+	    break;
 
 	  /* The line number is smaller.  Check that it's from the
 	     same function, not something inlined.  If it's inlined,
diff --git a/gdb/testsuite/gdb.base/prologue-include.c b/gdb/testsuite/gdb.base/prologue-include.c
new file mode 100644
index 0000000..b39e24d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2013 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/>.  */
+
+int
+main (void)
+{
+  int i, j;
+#include "prologue-include.h"
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/prologue-include.exp b/gdb/testsuite/gdb.base/prologue-include.exp
new file mode 100644
index 0000000..aa30439
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.exp
@@ -0,0 +1,31 @@
+# Copyright (C) 2012-2013 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 ${testfile}.exp ${testfile}] } {
+    return -1
+}
+
+#if ![runto callee] {
+#    return 0
+#}
+
+set test "break main"
+gdb_test_multiple $test $test {
+    -re "\r\nBreakpoint \[0-9\]+ at .*: file .*/$testfile.h, line 1\\.\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
diff --git a/gdb/testsuite/gdb.base/prologue-include.h b/gdb/testsuite/gdb.base/prologue-include.h
new file mode 100644
index 0000000..34b920c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.h
@@ -0,0 +1,2 @@
+  i = 2;
+  j = 2;

Re: [PATCH] Fix handling of #include files during prologue skipping

[Ulrich Weigand]

Tiago Daitx wrote:
> 
> gdb/ChangeLog
> 
> 2012-11-01  Ulrich Weigand  <uweigand@de.ibm.com>
> 
>     * symtab.c (skip_prologue_using_sal): Consider a file
>     change the same as an increased file number

"line number" instead of "file number".

> gdb/testsuite/ChangeLog
> 
> 2012-11-01  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>
>    
>     * gdb.base/prologue-include.c: New file.
>     * gdb.base/prologue-include.exp: New file.
>     * gdb.base/prologue-include.h: New file.

Otherwise, this is OK.

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tom Tromey]

>>>>> "Tiago" == Tiago Stürmer Daitx <tdaitx@linux.vnet.ibm.com> writes:

Tiago> +#if ![runto callee] {
Tiago> +#    return 0
Tiago> +#}

You might as well just drop commented-out code.

Tiago> +set test "break main"
Tiago> +gdb_test_multiple $test $test {
Tiago> +    -re "\r\nBreakpoint \[0-9\]+ at .*: file .*/$testfile.h, line 1\\.\r\n$gdb_prompt $" {
Tiago> +	pass $test
Tiago> +    }

Why gdb_test_multiple and not just gdb_test here?

Tom

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tiago Stürmer Daitx]

On Thu, 2013-01-24 at 08:12 -0700, Tom Tromey wrote:
> >>>>> "Tiago" == Tiago Stürmer Daitx <tdaitx@linux.vnet.ibm.com> writes:
> 
> Tiago> I got a tip from Edjunior to use gdb_get_line_number instead of that
> Tiago> hardcoded/magic line number. Works like a charm.
> 
> This is ok, thanks.
> 
> Tom
> 

Patch has been committed.

Thank you all for the reviews and discussion. =)

http://www.sourceware.org/ml/gdb-cvs/2013-01/msg00159.html

-- 
Tiago Stürmer Daitx
tdaitx@linux.vnet.ibm.com
IBM - Linux Technology Center

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tom Tromey]

>>>>> "Tiago" == Tiago Stürmer Daitx <tdaitx@linux.vnet.ibm.com> writes:

Tiago> I got a tip from Edjunior to use gdb_get_line_number instead of that
Tiago> hardcoded/magic line number. Works like a charm.

This is ok, thanks.

Tom

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tiago Stürmer Daitx]

I got a tip from Edjunior to use gdb_get_line_number instead of that
hardcoded/magic line number. Works like a charm.


Cheers

-tiago


gdb/ChangeLog

2013-01-24  Ulrich Weigand  <uweigand@de.ibm.com>

	* symtab.c (skip_prologue_using_sal): Consider a file
	change the same as an increased line number

gdb/testsuite/ChangeLog

2013-01-24  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>

	* gdb.base/prologue-include.c: New file.
	* gdb.base/prologue-include.exp: New file.
	* gdb.base/prologue-include.h: New file.



diff --git a/gdb/symtab.c b/gdb/symtab.c
index d40436a..b200b36 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4905,6 +4905,10 @@ skip_prologue_using_sal (struct gdbarch *gdbarch,
CORE_ADDR func_addr)
 	     line mark the prologue -> body transition.  */
 	  if (sal.line >= prologue_sal.line)
 	    break;
+	  /* Likewise if we are in a different symtab altogether
+	     (e.g. within a file included via #include).  */
+	  if (sal.symtab != prologue_sal.symtab)
+	    break;
 
 	  /* The line number is smaller.  Check that it's from the
 	     same function, not something inlined.  If it's inlined,
diff --git a/gdb/testsuite/gdb.base/prologue-include.c
b/gdb/testsuite/gdb.base/prologue-include.c
new file mode 100644
index 0000000..c393e75
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+int
+main (void)
+{
+  int i, j;
+#include "prologue-include.h"
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/prologue-include.exp
b/gdb/testsuite/gdb.base/prologue-include.exp
new file mode 100644
index 0000000..ed378b8
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.exp
@@ -0,0 +1,26 @@
+# Copyright (C) 2013 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 ${testfile}.exp ${testfile}] } {
+    return -1
+}
+
+set bp_main [gdb_get_line_number "break main" ${testfile}.h]
+
+gdb_test "break main" \
+    "Breakpoint.*at.* file .*$testfile.h, line $bp_main\\." \
+    "breakpoint main"
diff --git a/gdb/testsuite/gdb.base/prologue-include.h
b/gdb/testsuite/gdb.base/prologue-include.h
new file mode 100644
index 0000000..77843e0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.h
@@ -0,0 +1,19 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+  i = 2; /* break main should stop here */
+  j = 2;

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tiago Stürmer Daitx]

Just noticed that I forgot to hit sent earlier on.

I added the copyright header to prologue-include.h and updated the
testcase to reflect the line number change. I also took the time to get
the license date right on the 3 prologue-include.* files: it was
2012-2013 where is should read 2013 only.

Let me know if this is acceptable.

Without patch:
$ gdb prologue-include
(gdb) break main
Breakpoint 1 at 0x10000594: file prologue-include.h, line 19.

With patch applied:
$ gdb prologue-include
(gdb) break main
Breakpoint 1 at 0x1000058c: file prologue-include.h, line 18.

Thanks for all your help.

-tiago

---

gdb/ChangeLog

2013-01-24  Ulrich Weigand  <uweigand@de.ibm.com>

	* symtab.c (skip_prologue_using_sal): Consider a file
	change the same as an increased line number

gdb/testsuite/ChangeLog

2013-01-24  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>

	* gdb.base/prologue-include.c: New file.
	* gdb.base/prologue-include.exp: New file.
	* gdb.base/prologue-include.h: New file.


diff --git a/gdb/symtab.c b/gdb/symtab.c
index d40436a..b200b36 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4905,6 +4905,10 @@ skip_prologue_using_sal (struct gdbarch *gdbarch,
CORE_ADDR func_addr)
 	     line mark the prologue -> body transition.  */
 	  if (sal.line >= prologue_sal.line)
 	    break;
+	  /* Likewise if we are in a different symtab altogether
+	     (e.g. within a file included via #include).  */
+	  if (sal.symtab != prologue_sal.symtab)
+	    break;
 
 	  /* The line number is smaller.  Check that it's from the
 	     same function, not something inlined.  If it's inlined,
diff --git a/gdb/testsuite/gdb.base/prologue-include.c
b/gdb/testsuite/gdb.base/prologue-include.c
new file mode 100644
index 0000000..c393e75
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+int
+main (void)
+{
+  int i, j;
+#include "prologue-include.h"
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/prologue-include.exp
b/gdb/testsuite/gdb.base/prologue-include.exp
new file mode 100644
index 0000000..f18d55a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.exp
@@ -0,0 +1,24 @@
+# Copyright (C) 2013 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 ${testfile}.exp ${testfile}] } {
+    return -1
+}
+
+gdb_test "break main" \
+    "Breakpoint.*at.* file .*$testfile.h, line 18\\." \
+    "breakpoint main"
diff --git a/gdb/testsuite/gdb.base/prologue-include.h
b/gdb/testsuite/gdb.base/prologue-include.h
new file mode 100644
index 0000000..7da7d4a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.h
@@ -0,0 +1,19 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+  i = 2;
+  j = 2;


-- 
Tiago Stürmer Daitx
tdaitx@linux.vnet.ibm.com
IBM - Linux Technology Center


On Thu, 2013-01-24 at 11:11 -0200, Tiago Stürmer Daitx wrote:
> Yes, it still shows up. Sorry, should have added this information
> before.
> 
> Without patch:
> $ gdb prologue-include
> (gdb) break main
> Breakpoint 1 at 0x10000594: file prologue-include.h, line 19.
> 
> With patch applied:
> $ gdb prologue-include
> (gdb) break main
> Breakpoint 1 at 0x1000058c: file prologue-include.h, line 18.
> 
> 
> 

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tiago Stürmer Daitx]

Yes, it still shows up. Sorry, should have added this information
before.

Without patch:
$ gdb prologue-include
(gdb) break main
Breakpoint 1 at 0x10000594: file prologue-include.h, line 19.

With patch applied:
$ gdb prologue-include
(gdb) break main
Breakpoint 1 at 0x1000058c: file prologue-include.h, line 18.



-- 
Tiago Stürmer Daitx
tdaitx@linux.vnet.ibm.com
IBM - Linux Technology Center


On Thu, 2013-01-24 at 13:56 +0100, Ulrich Weigand wrote:
> Tiago Daitx wrote:
> 
> > I didn't include one since the header files in gdb.base that I looked at
> > the time didn't have it. I checked it again and noticed that from all 11
> > headers files in there only 1 has a copyright in it
> > (gdb.base/included.h).
> > 
> > If a copyright is actually needed I will be happy to include one, so
> > please let me know your answer.
> 
> If you do so, please keep in mind that the bug the test case triggered
> depends on (relative) line number values, so please make sure the bug
> actually still shows (with unpatched GDB) after you've added the
> comment lines ...
> 
> Bye,
> Ulrich
> 

Re: [PATCH] Fix handling of #include files during prologue skipping

[Ulrich Weigand]

Tiago Daitx wrote:

> I didn't include one since the header files in gdb.base that I looked at
> the time didn't have it. I checked it again and noticed that from all 11
> headers files in there only 1 has a copyright in it
> (gdb.base/included.h).
> 
> If a copyright is actually needed I will be happy to include one, so
> please let me know your answer.

If you do so, please keep in mind that the bug the test case triggered
depends on (relative) line number values, so please make sure the bug
actually still shows (with unpatched GDB) after you've added the
comment lines ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Fix handling of #include files during prologue skipping

[Pedro Alves]

On 01/24/2013 06:23 AM, Joel Brobecker wrote:
>> I didn't include one since the header files in gdb.base that I looked at
>> the time didn't have it. I checked it again and noticed that from all 11
>> headers files in there only 1 has a copyright in it
>> (gdb.base/included.h).
>>
>> If a copyright is actually needed I will be happy to include one, so
>> please let me know your answer.
> 
> Yes please. My understanding of the FSF procedures is that every file
> which contains significant material should have a copyright header,
> and my stance on this is that we should include one in every file.

Yeah.  My opinion is that its just easier to not go through
the trouble of applying some sort of "is there enough significant
material in copyright terms to warrant a header" process.  Someone
can always come in later and add more lines to a file that was
originally small enough to not have a header, and just reading the
patch may, and usually doesn't show whether a header is already
present or not.  So instead of placing a burden on review of
ensuring that detail is taken care of, and so whether a new header
should be added to an existing file, because the file is now big
enough, I much prefer a policy of "always a copyright header from
the start, even if the file is too small to begin with".  It's more
"mechanical" that way.

> Your observation is also correct, and it needs to be fixed eventually.

-- 
Pedro Alves

Re: [PATCH] Fix handling of #include files during prologue skipping

[Joel Brobecker]

> I didn't include one since the header files in gdb.base that I looked at
> the time didn't have it. I checked it again and noticed that from all 11
> headers files in there only 1 has a copyright in it
> (gdb.base/included.h).
> 
> If a copyright is actually needed I will be happy to include one, so
> please let me know your answer.

Yes please. My understanding of the FSF procedures is that every file
which contains significant material should have a copyright header,
and my stance on this is that we should include one in every file.
Your observation is also correct, and it needs to be fixed eventually.

-- 
Joel

Re: [PATCH] Fix handling of #include files during prologue skipping

[Yao Qi]

On 01/24/2013 11:19 AM, Tiago Stürmer Daitx wrote:
> I didn't include one since the header files in gdb.base that I looked at
> the time didn't have it. I checked it again and noticed that from all 11
> headers files in there only 1 has a copyright in it
> (gdb.base/included.h).

Yeah, that is true.  Joel applied some patches last month for the 
copyright headers, but I am not sure the rule applies to the headers in 
testsuite.

>
> If a copyright is actually needed I will be happy to include one, so
> please let me know your answer.

People who know more than me on this issue can give a comment.

-- 
Yao (齐尧)

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tiago Stürmer Daitx]

I didn't include one since the header files in gdb.base that I looked at
the time didn't have it. I checked it again and noticed that from all 11
headers files in there only 1 has a copyright in it
(gdb.base/included.h).

If a copyright is actually needed I will be happy to include one, so
please let me know your answer.

Cheers

-- 
Tiago Stürmer Daitx
tdaitx@linux.vnet.ibm.com
IBM - Linux Technology Center


On Thu, 2013-01-24 at 08:51 +0800, Yao Qi wrote:
> On 01/24/2013 04:33 AM, Tiago Stürmer Daitx wrote:
> > diff --git a/gdb/testsuite/gdb.base/prologue-include.h b/gdb/testsuite/gdb.base/prologue-include.h
> > new file mode 100644
> > index 0000000..34b920c
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/prologue-include.h
> > @@ -0,0 +1,2 @@
> > +  i = 2;
> > +  j = 2;
> 
> A copyright header is needed.
> 

Re: [PATCH] Fix handling of #include files during prologue skipping

[Yao Qi]

On 01/24/2013 04:33 AM, Tiago Stürmer Daitx wrote:
> diff --git a/gdb/testsuite/gdb.base/prologue-include.h b/gdb/testsuite/gdb.base/prologue-include.h
> new file mode 100644
> index 0000000..34b920c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/prologue-include.h
> @@ -0,0 +1,2 @@
> +  i = 2;
> +  j = 2;

A copyright header is needed.

-- 
Yao (齐尧)

Re: [PATCH] Fix handling of #include files during prologue skipping

[Tom Tromey]

>>>>> "Tiago" == Tiago Stürmer Daitx <tdaitx@linux.vnet.ibm.com> writes:

Tiago> 2013-01-23  Ulrich Weigand  <uweigand@de.ibm.com>

Tiago> 	* symtab.c (skip_prologue_using_sal): Consider a file
Tiago> 	change the same as an increased line number

Tiago> gdb/testsuite/ChangeLog

Tiago> 2013-01-23  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>

Tiago> 	* gdb.base/prologue-include.c: New file.
Tiago> 	* gdb.base/prologue-include.exp: New file.
Tiago> 	* gdb.base/prologue-include.h: New file.

This version looks good to me, thanks.

Tom

[PATCH] Fix handling of #include files during prologue skipping

[Tiago Stürmer Daitx]

In symtab.c:skip_prologue_using_sal, the code goes through the SAL markers 
starting from the beginning of the function, and checks for markers with the 
same (or increasing line numbers). The idea behind this is that if one gets 
decreasing line numbers, than one probably have the case where the optimizer 
scheduled code from later in the function into the prologue.

However, the problem was that this code was simply comparing the line
*numbers* -- even if they refered to a different file! That's why the
presence of the comment makes a difference, because it makes the line
number in t.c larger (or smaller) than the line number in t.h.

This patch simply considers a change in file equivalent to an
increasing line number.

Before the fix:

$ gdb prologue-include
[snip]
(gdb) break main
Breakpoint 1 at 0x10000594: file prologue-include.h, line 2.

After the fix:
$ gdb prologue-include
[snip]
(gdb) break main
Breakpoint 1 at 0x1000058c: file prologue-include.h, line 1.


No regressions detected on PPC32, PPC64, and AMD64.

Regards,
Tiago Daitx


---

gdb/ChangeLog

2013-01-23  Ulrich Weigand  <uweigand@de.ibm.com>

	* symtab.c (skip_prologue_using_sal): Consider a file
	change the same as an increased line number

gdb/testsuite/ChangeLog

2013-01-23  Tiago Stürmer Daitx  <tdaitx@linux.vnet.ibm.com>

	* gdb.base/prologue-include.c: New file.
	* gdb.base/prologue-include.exp: New file.
	* gdb.base/prologue-include.h: New file.


diff --git a/gdb/symtab.c b/gdb/symtab.c
index d40436a..b200b36 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4905,6 +4905,10 @@ skip_prologue_using_sal (struct gdbarch *gdbarch, CORE_ADDR func_addr)
 	     line mark the prologue -> body transition.  */
 	  if (sal.line >= prologue_sal.line)
 	    break;
+	  /* Likewise if we are in a different symtab altogether
+	     (e.g. within a file included via #include).  */
+	  if (sal.symtab != prologue_sal.symtab)
+	    break;
 
 	  /* The line number is smaller.  Check that it's from the
 	     same function, not something inlined.  If it's inlined,
diff --git a/gdb/testsuite/gdb.base/prologue-include.c b/gdb/testsuite/gdb.base/prologue-include.c
new file mode 100644
index 0000000..b39e24d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012-2013 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/>.  */
+
+int
+main (void)
+{
+  int i, j;
+#include "prologue-include.h"
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/prologue-include.exp b/gdb/testsuite/gdb.base/prologue-include.exp
new file mode 100644
index 0000000..89fdc1f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.exp
@@ -0,0 +1,24 @@
+# Copyright (C) 2012-2013 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 ${testfile}.exp ${testfile}] } {
+    return -1
+}
+
+gdb_test "break main" \
+    "Breakpoint.*at.* file .*$testfile.h, line 1\\." \
+    "breakpoint main"
diff --git a/gdb/testsuite/gdb.base/prologue-include.h b/gdb/testsuite/gdb.base/prologue-include.h
new file mode 100644
index 0000000..34b920c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/prologue-include.h
@@ -0,0 +1,2 @@
+  i = 2;
+  j = 2;