arm_addr_bits_remove

arm_addr_bits_remove

[Pedro Alves]

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

Re: arm_addr_bits_remove

[Jim Blandy]

I'm not an ARM expert by any means, so I don't object to the patch,
but I wonder if allowing arm_pc_is_thumb to return the wrong answer
for the first address beyond the end of a function will cause other
problems elsewhere.

If it is valuable to make arm_pc_is_thumb accurate in this case, when
it can't find a minsym at memaddr, and memaddr > 0, would it make
sense to look for a minsym at memaddr - 1, and see if MSYMBOL_SIZE (m)
!= 0 && SYMBOL_VALUE_ADDRESS (m) + MSYMBOL_SIZE (m) == memaddr, and
use m if so?

Re: arm_addr_bits_remove

[Pedro Alves]

Jim Blandy wrote:
> I'm not an ARM expert by any means, so I don't object to the patch,
> but I wonder if allowing arm_pc_is_thumb to return the wrong answer
> for the first address beyond the end of a function will cause other
> problems elsewhere.
> 

[FWIW, I'm not an ARM expert either]

> If it is valuable to make arm_pc_is_thumb accurate in this case, when
> it can't find a minsym at memaddr, and memaddr > 0, would it make
> sense to look for a minsym at memaddr - 1, and see if MSYMBOL_SIZE (m)
> != 0 && SYMBOL_VALUE_ADDRESS (m) + MSYMBOL_SIZE (m) == memaddr, and
> use m if so?
> 

With that you'll be certain that there is a symbol *before* the
address you want to check, and you'll be sure about it's mode,
and I'm sure that most of the times that mode will be the same as
the mode of memaddr, but you can't be sure, can you?

I'd say that relying on the mode of first address beyond the end
of a function to infer something about the function itself is broken.
In those cases the '- 1' should be applied explicitly on the
call to arm_pc_is_thumb (or earlier on the call stack).

The case I stumbled on the bug is a bit different from that
case you mentioned, because the line info doesn't refer to a possible
function which includes memaddr or ends before memaddr.  There was no
real code at the address the lookup was being performed, because
it refers to the end of the object file, where padding is being
performed, but real code in a different mode could be there.  If
there was code there, then the correct mode for it could be
inferred, and it could be different from memaddr-1 -- at
least that's my understanding.  I could be wrong though.  :-)

An earlier version of the patch changed arm_addr_bits_remove to
call a new function arm_mode_at_pc that returned arm, thumb or
unknown.  Only if that returned arm, I'd strip bit 1.  But then,
if that bit is not used for any encoding (unlike bit 0), we can
just never strip it, and save us the min sym lookup to begin with.
That is, the patch removes one usage of arm_pc_is_thumb when
it can't always provide a correct answer.

-- 
Pedro Alves

Re: arm_addr_bits_remove

[Jim Blandy]

On Jan 23, 2008 6:44 AM, Pedro Alves <pedro_alves@portugalmail.pt> wrote:
> With that you'll be certain that there is a symbol *before* the
> address you want to check, and you'll be sure about it's mode,
> and I'm sure that most of the times that mode will be the same as
> the mode of memaddr, but you can't be sure, can you?

I guess the thought was that if there is no symbol covering an
address, but there is a symbol whose range ends at that address, it's
a safe bet the address is some sort of endpoint (as it is in the case
at hand), and should be treated like the other addresses within the
range.

But you're right, it's a heuristic.

> The case I stumbled on the bug is a bit different from that
> case you mentioned, because the line info doesn't refer to a possible
> function which includes memaddr or ends before memaddr.  There was no
> real code at the address the lookup was being performed, because
> it refers to the end of the object file, where padding is being
> performed, but real code in a different mode could be there.  If
> there was code there, then the correct mode for it could be
> inferred, and it could be different from memaddr-1 -- at
> least that's my understanding.  I could be wrong though.  :-)

So the heuristic I suggested will often be wrong.  :(  I guess you're
right that one should be subtracting one from the address somewhere
earlier in the call stack.

Looking into this a bit more, I got even more confused.  (I don't want
to get in the way of getting a bug fixed, so take this if it's helpful
and ignore it if it's not.)  Why are we calling
gdbarch_addr_bits_remove in record_line at all?  Who sets the thumb
bit in line number info?  The thumb bit is only meaningful in ELF
symbols, and values derived from those.

It seems like the use of gdbarch_addr_bits_remove originated in 2001:

2001-04-06  Fernando Nasser  <fnasser@redhat.com>

	* buildsym.c (record_line): Turn off unused addr bits.

Could we try simply removing this?

Re: arm_addr_bits_remove

[Daniel Jacobowitz]

On Wed, Jan 23, 2008 at 11:21:50AM -0800, Jim Blandy wrote:
> It seems like the use of gdbarch_addr_bits_remove originated in 2001:
> 
> 2001-04-06  Fernando Nasser  <fnasser@redhat.com>
> 
> 	* buildsym.c (record_line): Turn off unused addr bits.
> 
> Could we try simply removing this?

It was for Thumb in the first place.

http://sourceware.org/ml/gdb-patches/2001-03/msg00182.html
http://sourceware.org/ml/gdb-patches/2001-04/msg00012.html

That may not be true on new toolchains though.

-- 
Daniel Jacobowitz
CodeSourcery

Re: arm_addr_bits_remove

[Jim Blandy]

On Jan 23, 2008 11:28 AM, Daniel Jacobowitz <drow@false.org> wrote:
> It was for Thumb in the first place.
>
> http://sourceware.org/ml/gdb-patches/2001-03/msg00182.html
> http://sourceware.org/ml/gdb-patches/2001-04/msg00012.html
>
> That may not be true on new toolchains though.

Rats.  If it'd be quick for you to check current toolchains, could you?

Re: arm_addr_bits_remove

[Pedro Alves]

Jim Blandy wrote:
> On Jan 23, 2008 11:28 AM, Daniel Jacobowitz wrote:
>> It was for Thumb in the first place.
>>
>> http://sourceware.org/ml/gdb-patches/2001-03/msg00182.html
>> http://sourceware.org/ml/gdb-patches/2001-04/msg00012.html
>>
>> That may not be true on new toolchains though.
> 
> Rats.  If it'd be quick for you to check current toolchains, could you?
> 

Jim Blandy wrote:
 > 2001-04-06  Fernando Nasser  <fnasser@redhat.com>
 >
 > 	* buildsym.c (record_line): Turn off unused addr bits.
 >
 > Could we try simply removing this?
 >

Removing this shows no regressions on arm-eabi or arm-linux-gnueabi.

I've also ran the testsuite on arm-linux-gnueabi arm/thumb
replacing the stripping in record_line by logging the pc being
stripped, and no odd address was ever printed.

Would a patch removing this be more acceptable then?

-- 
Pedro Alves

Re: arm_addr_bits_remove

[Jim Blandy]

On Jan 23, 2008 8:53 PM, Pedro Alves <pedro_alves@portugalmail.pt> wrote:
> Would a patch removing this be more acceptable then?

I'd certainly be more enthusiastic about it.  :)

Re: arm_addr_bits_remove

[Jim Blandy]

On Jan 23, 2008 8:53 PM, Pedro Alves <pedro_alves@portugalmail.pt> wrote:
> Would a patch removing this be more acceptable then?

I'd certainly be more enthusiastic about it.  :)

Re: arm_addr_bits_remove

[Pedro Alves]

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

Jim Blandy wrote:
> On Jan 23, 2008 8:53 PM, Pedro Alves wrote:
>> Would a patch removing this be more acceptable then?
> 
> I'd certainly be more enthusiastic about it.  :)
> 

Well, then, here's one for the review-enthusiasts amongst us. :-)

Thanks Jim.

OK ?

-- 
Pedro Alves

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

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

	* buildsym.c (record_line): Don't try to strip unused addr bits.

---
 gdb/buildsym.c |    2 --
 1 files changed, 2 deletions(-)

Index: gdb-trunk/gdb/buildsym.c
===================================================================
--- gdb-trunk.orig/gdb/buildsym.c	2008-01-09 10:53:49.000000000 -0800
+++ gdb-trunk/gdb/buildsym.c	2008-01-24 04:30:26.000000000 -0800
@@ -794,8 +794,6 @@ record_line (struct subfile *subfile, in
 		      * sizeof (struct linetable_entry))));
     }
 
-  pc = gdbarch_addr_bits_remove (current_gdbarch, pc);
-
   /* Normally, we treat lines as unsorted.  But the end of sequence
      marker is special.  We sort line markers at the same PC by line
      number, so end of sequence markers (which have line == 0) appear

Re: arm_addr_bits_remove

[Daniel Jacobowitz]

On Thu, Jan 24, 2008 at 12:39:55PM +0000, Pedro Alves wrote:
> Jim Blandy wrote:
>> On Jan 23, 2008 8:53 PM, Pedro Alves wrote:
>>> Would a patch removing this be more acceptable then?
>>
>> I'd certainly be more enthusiastic about it.  :)
>>
>
> Well, then, here's one for the review-enthusiasts amongst us. :-)

If you're daring enough, this is OK - this could affect any target
defining gdbarch_addr_bits_remove so keep an eye out in case hppa,
m88k, mips, or s390 break.  Please wait another day before checking it
in, in case someone else knows more about it.

I think the original patch should be committed too.  Jim objected in
terms of "returning the wrong answer", but that's not really the case.
The 0x2 bit is never an extra piece of information about an address,
like the 0x1 bit is.  It's part of the address; just if it happens
to be part of an ARM address, executing code there is unpredictable.

-- 
Daniel Jacobowitz
CodeSourcery

Re: arm_addr_bits_remove

[Pedro Alves]

Daniel Jacobowitz wrote:
> 
> If you're daring enough, this is OK - this could affect any target
> defining gdbarch_addr_bits_remove so keep an eye out in case hppa,
> m88k, mips, or s390 break.  Please wait another day before checking it
> in, in case someone else knows more about it.
> 

It seems possible that the hppa port would be affected.  It has has a
comment mentioning symbol tables having special bits set and
other compilers other than gcc.  From the comments, I believe
the other ports would be safe -- the extra bits should be set
at runtime only.  Other than arm, only mips choses what to strip
based on some expression, but in that case, the I believe the
extra bits won't be set on the symbol tables/line info.

> I think the original patch should be committed too.  Jim objected in
> terms of "returning the wrong answer", but that's not really the case.
> The 0x2 bit is never an extra piece of information about an address,
> like the 0x1 bit is.  It's part of the address; just if it happens
> to be part of an ARM address, executing code there is unpredictable.

Since Jim didn't really object, I'll go with the original
arm-tdep.c patch.

-- 
Pedro Alves

Re: arm_addr_bits_remove

[Jim Blandy]

On Jan 24, 2008 6:45 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> Since Jim didn't really object, I'll go with the original
> arm-tdep.c patch.

Cool with me.

Re: arm_addr_bits_remove

[Pedro Alves]

Jim Blandy wrote:
> On Jan 24, 2008 6:45 AM, Pedro Alves <pedro@codesourcery.com> wrote:
>> Since Jim didn't really object, I'll go with the original
>> arm-tdep.c patch.
> 
> Cool with me.

Thanks Jim,

The patch is installed.

-- 
Pedro Alves

Re: arm_addr_bits_remove

[Mark Kettenis]

> Date: Wed, 23 Jan 2008 22:27:33 -0800
> From: "Jim Blandy" <jimb@red-bean.com>
> 
> On Jan 23, 2008 8:53 PM, Pedro Alves <pedro_alves@portugalmail.pt> wrote:
> > Would a patch removing this be more acceptable then?
> 
> I'd certainly be more enthusiastic about it.  :)

It would need some careful testing though, at least on RISCy targets.
2001 is quite a while ago and while the change might have been made
for arm in the first place, other targets might depend on it by now.

Re: arm_addr_bits_remove

[Joel Brobecker]

> It would need some careful testing though, at least on RISCy targets.
> 2001 is quite a while ago and while the change might have been made
> for arm in the first place, other targets might depend on it by now.

I am currently testing on IRIX and PA/HPUX; at least we'll know
about these targets.

-- 
Joel

Re: arm_addr_bits_remove

[Jim Blandy]

On Jan 24, 2008 9:17 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> > It would need some careful testing though, at least on RISCy targets.
> > 2001 is quite a while ago and while the change might have been made
> > for arm in the first place, other targets might depend on it by now.
>
> I am currently testing on IRIX and PA/HPUX; at least we'll know
> about these targets.

Let's keep in mind that this is a fixup for bad line number
information.  The DWARF spec says that those values are addresses of
instructions, not addresses with bits of type or state information
stuffed in the low bits.  Accomodation for this sort of lossage should
have a limited lifetime, because they complicate maintenance and
improvements.

Re: arm_addr_bits_remove

[Joel Brobecker]

> > 2008-01-24  Pedro Alves  <pedro@codesourcery.com>
> > 
> >         * buildsym.c (record_line): Don't try to strip unused addr bits.

> If you're daring enough, this is OK - this could affect any target
> defining gdbarch_addr_bits_remove so keep an eye out in case hppa,
> m88k, mips, or s390 break.  Please wait another day before checking it
> in, in case someone else knows more about it.

I just ran the testsuite on hppa and mips-irix, no regression.
It might not come as a surprise given that the patch is removing
something that handles what amounts to incorrect debugging info,
but it's better than nothing. In both case, we use GNU and the
system linker.

-- 
Joel

Re: arm_addr_bits_remove

[Pedro Alves]

Joel Brobecker wrote:
> I just ran the testsuite on hppa and mips-irix, no regression.
> It might not come as a surprise given that the patch is removing
> something that handles what amounts to incorrect debugging info,
> but it's better than nothing. In both case, we use GNU and the
> system linker.
> 

For the record:

hppa-tdep.c has this function registered as gdbarch_addr_bits_remove:

static CORE_ADDR
hppa_smash_text_address (CORE_ADDR addr)
{
   /* The low two bits of the PC on the PA contain the privilege level.
      Some genius implementing a (non-GCC) compiler apparently decided
      this means that "addresses" in a text section therefore include a
      privilege level, and thus symbol tables should contain these bits.
      This seems like a bonehead thing to do--anyway, it seems to work
      for our purposes to just ignore those bits.  */

   return (addr &= ~0x3);
}

I guess that compiler would be HP's.  If the symbols had the bits set,
so could the line info.  Do we still support HP's object format and
debug info ?

I see that the hppa port of gas has this .export pseudo that enable
the setting of a privilege level (priv_lev in c-hppa.texi).  It doesn't
seem to be covered by gdb's testsuite in testsuite/gdb.hp.

-- 
Pedro Alves

Re: arm_addr_bits_remove

[Joel Brobecker]

Thanks Pedro.

>   /* The low two bits of the PC on the PA contain the privilege level.
>      Some genius implementing a (non-GCC) compiler apparently decided
>      this means that "addresses" in a text section therefore include a
>      privilege level, and thus symbol tables should contain these bits.
>      This seems like a bonehead thing to do--anyway, it seems to work
>      for our purposes to just ignore those bits.  */

I just love the comment :). I'm so happy I'm not the HP genius
in question :-). Seriously, I think we should be careful not to
add more sarcastic comments like this in the future, it's not
very serious nor very useful. That being said, I love sarcasm,
so I had a good laugh.

> I guess that compiler would be HP's.  If the symbols had the bits set,
> so could the line info.

Right. That's why I mentioned the fact that part of my toolchain
was from GNU tools.

> Do we still support HP's object format and debug info ?

Not sure.

-- 
Joel

Re: arm_addr_bits_remove

[Carlos O'Donell]

Joel Brobecker wrote:
>>   /* The low two bits of the PC on the PA contain the privilege level.
>>      Some genius implementing a (non-GCC) compiler apparently decided
>>      this means that "addresses" in a text section therefore include a
>>      privilege level, and thus symbol tables should contain these bits.
>>      This seems like a bonehead thing to do--anyway, it seems to work
>>      for our purposes to just ignore those bits.  */
> 
> I just love the comment :). I'm so happy I'm not the HP genius
> in question :-). Seriously, I think we should be careful not to
> add more sarcastic comments like this in the future, it's not
> very serious nor very useful. That being said, I love sarcasm,
> so I had a good laugh.

What do they mean by `"addresses" in a text section'?

The only thing that should have a privilege level (PL) is the PC (IAOQ) 
at runtime, and it should match the PL of the executing instruction.

If you start talking about addresses, and symbol tables, you wander into 
the realm of function addresses and official procedure descriptors (OPD).

On 32-bit hppa-linux-gnu the address of a function may point to the 
function or the official procedure descriptor (On 64-bit you always have 
an OPD). The OPD address has bit 2 set, and bit 1 is reserved (See pg 31 
of the 32-bit SOM Runtime). The OPD is a struct { ELF32 ip; ELF32 gp}. 
If (<function symbol> & 3) is true you have a function address, 
otherwise an OPD. To read the OPD you strip the bottom 2 bits, cast to a 
struct, and use ip and gp.

Might the writer of the comment gotten confused?

>> I guess that compiler would be HP's.  If the symbols had the bits set,
>> so could the line info.
> 
> Right. That's why I mentioned the fact that part of my toolchain
> was from GNU tools.
> 
>> Do we still support HP's object format and debug info ?
> 
> Not sure.

HP's 32-bit HP-PARISC object format is SOM.

Dave, binutils still has SOM support?

We still generate some HP debug info, namely .PARISC.unwind sections, 
and AFAIK gdb uses them (readelf shows them correctly)?

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
carlos@codesourcery.com
(650) 331-3385 x716

Re: arm_addr_bits_remove

[John David Anglin]

> Joel Brobecker wrote:
> >>   /* The low two bits of the PC on the PA contain the privilege level.
> >>      Some genius implementing a (non-GCC) compiler apparently decided
> >>      this means that "addresses" in a text section therefore include a
> >>      privilege level, and thus symbol tables should contain these bits.
> >>      This seems like a bonehead thing to do--anyway, it seems to work
> >>      for our purposes to just ignore those bits.  */
> > 
> > I just love the comment :). I'm so happy I'm not the HP genius
> > in question :-). Seriously, I think we should be careful not to
> > add more sarcastic comments like this in the future, it's not
> > very serious nor very useful. That being said, I love sarcasm,
> > so I had a good laugh.
> 
> What do they mean by `"addresses" in a text section'?

Memory addresses are the same everywhere.  In the PA 1.x architecture,
there are four qudrants (i.e., the architecture is segmented) each which
can address 4 GB of memory.  In the hpux and linux runtimes, the addressing
is restricted to 1 GB for efficiency.  Linux has a flat addressess space
(the upper four segment registers for a process are all equivalent).
The segments in hpux aren't flat.  As a result, calling stubs are more
elaborate.

The hardware has no concept of "text".  Text may be placed anywhere
in physical memory.  HP-UX typically places text in one quadrant,
data in another and shared text in another.  Howver, code can execute
from anywhere.  The text quadrant in HP-UX is readonly.  Code can
read it and we do this for jump tables in GCC.

You have to be aware that instructions can only execute from a
four byte aligned location and mask the two least least significant
bits when doing arithmetic involving the pc.  Other than that, user
code isn't really aware of the priority bits and the associated
access restrictions.

Regarding symbol tables, I would have to check what the two bits are
used for.  What struct/bits?

> The only thing that should have a privilege level (PL) is the PC (IAOQ) 
> at runtime, and it should match the PL of the executing instruction.
> 
> HP's 32-bit HP-PARISC object format is SOM.
> 
> Dave, binutils still has SOM support?

Yes.

> We still generate some HP debug info, namely .PARISC.unwind sections, 
> and AFAIK gdb uses them (readelf shows them correctly)?

GCC uses the .stabs format.  The .unwind sections are used by gdb,
Ada and probably other code that has unwind support.  Recent versions
of GCC use dwarf2 unwind info in the data section for exception
support.  I posted a patch a year or so ago to add dwarf2 debug support
but nobody commented on it.  There is a bit of support that needs
to be added to binutils that's not done yet.

Other than the unwind sections, HP debug info is only useful for
debugging applications compiled with HP tools.  HP has provided
wdb for years to do this.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: arm_addr_bits_remove

[Jim Blandy]

On Jan 25, 2008 10:49 AM, John David Anglin <dave@hiauly1.hia.nrc.ca> wrote:
> Regarding symbol tables, I would have to check what the two bits are
> used for.  What struct/bits?

Within GDB, the debug info readers call record_line to add each source
location / pc mapping they find to GDB's internal tables.  The issue
at hand is that record_line currently calls gdbarch_addr_bits_remove
on the pc values it's passed before recording them; apparently some
debug info had extra bits set in the addresses in the line number
info.  Since this doesn't meet the spec (of either DWARF or STABS),
it's a bug in the producer; the call to gdbarch_addr_bits_remove is
meant to work around that bug.  We've been discussing removing that
call, since it has caused problems recently.

Re: arm_addr_bits_remove

[John David Anglin]

> On Jan 25, 2008 10:49 AM, John David Anglin <dave@hiauly1.hia.nrc.ca> wrote:
> > Regarding symbol tables, I would have to check what the two bits are
> > used for.  What struct/bits?
> 
> Within GDB, the debug info readers call record_line to add each source
> location / pc mapping they find to GDB's internal tables.  The issue
> at hand is that record_line currently calls gdbarch_addr_bits_remove
> on the pc values it's passed before recording them; apparently some
> debug info had extra bits set in the addresses in the line number
> info.  Since this doesn't meet the spec (of either DWARF or STABS),
> it's a bug in the producer; the call to gdbarch_addr_bits_remove is
> meant to work around that bug.  We've been discussing removing that
> call, since it has caused problems recently.

I think the comment is likely wrong.  I can see using the bits in
dwarf or stabs information might cause problems.  Hopefully, this
isn't the case (ie., binutils doesn't need fixing).

It is possible the bits need masking from the relocated unwind or
the HP debug info in the $DEBUG$ space.  The best source of information
for HP debug info is the HP-UX 10.20 run-time architecure document.
I believe this information was dropped from the public HP-UX 11
document.

The bits do arise whenever the program counter is saved on the stack
or copied to a register.  In that case, the bits need to masked.
This probably should be done arlier.

Priority can occur on calls.  This normally only used for system
calls.  There is no support priority changes in GCC.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: arm_addr_bits_remove

[Maciej W. Rozycki]

On Thu, 24 Jan 2008, Joel Brobecker wrote:

> > > 2008-01-24  Pedro Alves  <pedro@codesourcery.com>
> > > 
> > >         * buildsym.c (record_line): Don't try to strip unused addr bits.
> 
> > If you're daring enough, this is OK - this could affect any target
> > defining gdbarch_addr_bits_remove so keep an eye out in case hppa,
> > m88k, mips, or s390 break.  Please wait another day before checking it
> > in, in case someone else knows more about it.
> 
> I just ran the testsuite on hppa and mips-irix, no regression.
> It might not come as a surprise given that the patch is removing
> something that handles what amounts to incorrect debugging info,
> but it's better than nothing. In both case, we use GNU and the
> system linker.

 For the record -- to ever hit this case on MIPS you would have to run the 
test suite in the 32-bit kernel mode, so IRIX certainly does not apply 
here.  One of the embedded targets might fit, e.g. using newlib and the 
GNU sim.  Still this would be a bug if it mattered these days as proper 
sign-extension of addresses is meant to be done for MIPS now.

  Maciej

Re: arm_addr_bits_remove

[Daniel Jacobowitz]

On Mon, Feb 04, 2008 at 10:15:57AM +0000, Maciej W. Rozycki wrote:
>  For the record -- to ever hit this case on MIPS you would have to run the 
> test suite in the 32-bit kernel mode, so IRIX certainly does not apply 
> here.  One of the embedded targets might fit, e.g. using newlib and the 
> GNU sim.  Still this would be a bug if it mattered these days as proper 
> sign-extension of addresses is meant to be done for MIPS now.

Really?  This is for removing low bits, not high bits - I'd be more
worried about MIPS16.

-- 
Daniel Jacobowitz
CodeSourcery

Re: arm_addr_bits_remove

[Maciej W. Rozycki]

On Mon, 4 Feb 2008, Daniel Jacobowitz wrote:

> >  For the record -- to ever hit this case on MIPS you would have to run the 
> > test suite in the 32-bit kernel mode, so IRIX certainly does not apply 
> > here.  One of the embedded targets might fit, e.g. using newlib and the 
> > GNU sim.  Still this would be a bug if it mattered these days as proper 
> > sign-extension of addresses is meant to be done for MIPS now.
> 
> Really?  This is for removing low bits, not high bits - I'd be more
> worried about MIPS16.

 Well, for MIPS the function only removes high bits if any at all:

static CORE_ADDR
mips_addr_bits_remove (CORE_ADDR addr)
{
  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
  if (mips_mask_address_p (tdep) && (((ULONGEST) addr) >> 32 == 0xffffffffUL))
    return addr &= 0xffffffffUL; /* sic! */
  else
    return addr;
}

See the comment in the sources for some background which I can understand 
based on my experience with the issues over the years and which I skipped 
here due to its length.

  Maciej

Re: arm_addr_bits_remove

[Daniel Jacobowitz]

On Mon, Feb 04, 2008 at 02:44:32PM +0000, Maciej W. Rozycki wrote:
>  Well, for MIPS the function only removes high bits if any at all:

Oh.  Huh.  I would have expected, since you said that the .debug_info
section sometimes has the low MIPS16 bit, that the .debug_line section
would too and this would have to remove it.  But I guess not!

-- 
Daniel Jacobowitz
CodeSourcery

Re: arm_addr_bits_remove

[Maciej W. Rozycki]

On Mon, 4 Feb 2008, Daniel Jacobowitz wrote:

> >  Well, for MIPS the function only removes high bits if any at all:
> 
> Oh.  Huh.  I would have expected, since you said that the .debug_info
> section sometimes has the low MIPS16 bit, that the .debug_line section
> would too and this would have to remove it.  But I guess not!

 Oh, it's the other way round.  The .debug_info section sometimes fails to 
have the low MIPS16 bit set even though GDB code has been overall written 
to expect it for MIPS16 functions.

  Maciej