arm_addr_bits_remove

[Pedro Alves <pedro@codesourcery.com>]

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

[long speech, small patch]

Hi,

arm-eabi/-mthumb currently has a couple of failures on the break.exp test:

break marker4
Breakpoint 2 at 0x1e8
(gdb) FAIL: gdb.base/break.exp: breakpoint small function, optimized file

The test is failing because a filename and a line number is expected after
the breakpoint address.

What's happening is that when we're building the line table, we call
buildsym.c:record_line, which means we call gdbarch_addr_bits_remove
on every address of the line table, eventually calling it on the
end-of-sequence marker address, just past the last address of
the last function in the testcase's object file where function
"marker4" is defined.

arm uses an encoding in the minimal symbols to distinguish between
arm and thumb code.  A lookup_minimal_symbol_by_pc is needed
in arm_pc_is_thumb to get at the minimal symbol of the address in
question.  If this address we're looking up isn't part of any
real function, this lookup_minimal_symbol_by_pc returns NULL,
which has the effect of arm_pc_is_thumb returning false
(defaulting to non-thumb), which triggers arm_addr_bits_remove
to remove the 2 lower bits from the address -- it thinks this is
a 32-bit arm address).  This is thumb code we're testing, so bit
1 is important to keep.  The bug is then visible because when
we're handling the end of sequence marker in record_line, and
the address has bit 1 set:

          pc = gdbarch_addr_bits_remove (current_gdbarch, pc);  <<= trims more
                                                                <<= than it should

          if (line == 0 && subfile->line_vector->nitems > 0)
            {
              e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
              while (subfile->line_vector->nitems > 0 && e->pc == pc)
	{
	  e--;
	  subfile->line_vector->nitems--;
	}
            }

... the pc is trimmed back by 2 bytes.  If the last function in the
object happens to be empty, it will have two line entries with
the same address as this "broken" marker.  Now the "broken" marker
is no longer the last address in the line vector, so, the while
loop shown above will cut off the wrong lines -- effectively
removing all line info for this last function in the object file.

When we issue a break marker4, gdb will not be able to find its line
info, and will just print:

Breakpoint 2 at 0x1e8

The easy fix is to never strip the 0x2 from the address.  0x1
is used to mark thumb addresses, but 0x2 should never appear, and
should never need to be stripped.

Attached is a testcase, based on break.exp, and the fix for the bug.
I'm not sure if we should add the testcase or not.  On one hand, this was
already caught by a failure; on the other hand, this is not what the
break.exp test is testing -- any change to it may hide a similar
future bug; (on the other (third) hand, the test is sensible.)

Tested on arm-eabi, arm and thumb.

So... ,
OK ?

-- 
Pedro Alves




[-- Attachment #2: addr_bits_remove.diff --]
[-- Type: text/x-diff, Size: 722 bytes --]

2008-01-22  Pedro Alves  <pedro@codesourcery.com>

	* arm-tdep.c (arm_addr_bits_remove): In non 26-bit mode, don't
	strip bit 1 even if pc doesn't point to thumb code.

---
 gdb/arm-tdep.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: gdb-trunk/gdb/arm-tdep.c
===================================================================
--- gdb-trunk.orig/gdb/arm-tdep.c	2008-01-22 11:10:23.000000000 -0800
+++ gdb-trunk/gdb/arm-tdep.c	2008-01-22 11:12:04.000000000 -0800
@@ -244,7 +244,7 @@
 arm_addr_bits_remove (CORE_ADDR val)
 {
   if (arm_apcs_32)
-    return (val & (arm_pc_is_thumb (val) ? 0xfffffffe : 0xfffffffc));
+    return UNMAKE_THUMB_ADDR (val);
   else
     return (val & 0x03fffffc);
 }








[-- Attachment #3: addr_bits_rem_test.diff --]
[-- Type: text/x-diff, Size: 3417 bytes --]

2008-01-22  Pedro Alves  <pedro@codesourcery.com>

	* gdb.base/addbitsrem.c: New file.
	* gdb.base/addbitsrem.exp: New file.

---
 gdb/testsuite/gdb.base/addrbitsrem.c   |   29 ++++++++++++++++
 gdb/testsuite/gdb.base/addrbitsrem.exp |   57 +++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+)

Index: gdb-trunk/gdb/testsuite/gdb.base/addrbitsrem.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb-trunk/gdb/testsuite/gdb.base/addrbitsrem.c	2008-01-22 12:16:57.000000000 -0800
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 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 (int argc, char **argv)
+{
+    return argc;
+}
+
+/* This should be the last function in the file, and it shall have no
+   code in it.  */
+void
+last_function (void)
+{
+}
Index: gdb-trunk/gdb/testsuite/gdb.base/addrbitsrem.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gdb-trunk/gdb/testsuite/gdb.base/addrbitsrem.exp	2008-01-22 12:14:04.000000000 -0800
@@ -0,0 +1,57 @@
+# Copyright 2008 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/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+#
+# test running programs
+#
+set prms_id 0
+set bug_id 0
+
+set testfile "addrbitsrem"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}.o" object {debug nowarnings optimize=-O2}] != "" } {
+    untested addrbitsrem.exp
+    return -1
+}
+
+if  { [gdb_compile "${binfile}.o" "${binfile}" executable {debug nowarnings}] != "" } {
+    untested addrbitsrem.exp
+    return -1
+}
+
+if [get_compiler_info ${binfile}] {
+    untested addrbitsrem.exp
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+#
+# test break at last_function
+#
+gdb_test "break last_function" \
+    "Breakpoint.*at.* file .*$srcfile, line.*" \
+    "breakpoint last_function"