[PATCH 24700]

[PATCH 24700]

[Jose Flavio Aguilar Paulino]

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

This patch modifies the <gdb/ppc-linux-tdep.c> file.
It was noticed in GDB for PPC64 that the command "set var" failed when
used to change a string value, the intent of this patch is to solve this
gdb64's problem. Ex:

char *ptr = NULL;
printf("ptr string is %s\n", ptr);
(gdb) set var ptr="def"
---Problem Description--- 
gdb64: failed to set a string variable's value.

I could not detect any regressions that could be caused by this patch in GDB's testsuite.

-----
José Flávio Aguilar Paulino
Software Engineer
LoP Toolchain Team
IBM

[-- Attachment #2: 24700.patch --]
[-- Type: text/x-patch, Size: 2025 bytes --]

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.81
diff -a -u -r1.81 ppc-linux-tdep.c
--- ppc-linux-tdep.c	9 Jan 2007 17:58:55 -0000	1.81
+++ ppc-linux-tdep.c	8 Feb 2007 01:11:15 -0000
@@ -753,11 +753,57 @@
 					CORE_ADDR addr,
 					struct target_ops *targ)
 {
+  CORE_ADDR addr2deref = 0;
   struct section_table *s = target_section_by_addr (targ, addr);
+  char buf[sizeof (ULONGEST)];
+  struct objfile *objfile;
+  struct obj_section *osect;
+  asection *sect;
+  CORE_ADDR sect_addr;
 
   /* Check if ADDR points to a function descriptor.  */
-  if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
-    return get_target_memory_unsigned (targ, addr, 8);
+
+
+  if (s)
+    {
+      if (strcmp (s->the_bfd_section->name, ".opd") != 0)
+        /* Found a the section, but it's not an .opd section. */
+        return addr;
+      addr2deref = addr;
+    }
+  else
+    /* The followin table search has been copied from printcmd.c */
+    ALL_OBJSECTIONS (objfile, osect)
+      {
+        /* Only process each object file once, even if there's a separate
+           debug file.  */
+        if (objfile->separate_debug_objfile_backlink)
+          continue;
+  
+        sect = osect->the_bfd_section;
+        sect_addr = overlay_mapped_address (addr, sect);
+    
+        if (osect->addr <= sect_addr && sect_addr < osect->endaddr)
+          {
+            if (strcmp (sect->name, ".opd") != 0)
+	      /* Found the section, but it's not an .opd section. */
+              return addr;
+            addr2deref = addr;
+	    break;
+          }
+      }
+
+  if (addr2deref) 
+    {
+      if (targ != &current_target)
+        return get_target_memory_unsigned (targ, addr2deref, 8);
+      else
+          {
+            gdb_assert (8 <= sizeof (buf));
+            target_read_memory(addr, buf, 8);
+            return extract_unsigned_integer (buf, 8);
+          }
+    }
 
   return addr;
 }

Re: [PATCH 24700]

[Daniel Jacobowitz]

On Thu, Mar 01, 2007 at 03:26:38PM -0400, Jose Flavio Aguilar Paulino wrote:
> This patch modifies the <gdb/ppc-linux-tdep.c> file.
> It was noticed in GDB for PPC64 that the command "set var" failed when
> used to change a string value, the intent of this patch is to solve this
> gdb64's problem. Ex:
> 
> char *ptr = NULL;
> printf("ptr string is %s\n", ptr);
> (gdb) set var ptr="def"
> ---Problem Description--- 
> gdb64: failed to set a string variable's value.
> 
> I could not detect any regressions that could be caused by this patch in GDB's testsuite.

What is the problem, and how did the patch fix it?  There are a number
of style issues with the patch, but no point at looking at them until
we understand the overall issue.

> +      if (targ != &current_target)
> +        return get_target_memory_unsigned (targ, addr2deref, 8);
> +      else
> +          {
> +            gdb_assert (8 <= sizeof (buf));
> +            target_read_memory(addr, buf, 8);
> +            return extract_unsigned_integer (buf, 8);
> +          }

What's the point of special casing based on the target?

-- 
Daniel Jacobowitz
CodeSourcery

[PATCH 24700]

[Jose Flavio Aguilar Paulino]

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

This patch modifies the <gdb/ppc-linux-tdep.c> file.
It was noticed in GDB for PPC64 that the command "set var" failed when
used to change a string value, the intent of this patch is to solve this
gdb64's problem.

I could not detect any regressions that could be caused by this patch in
GDB's testsuite.

The problem solved by this patch was noticied in SLES10.
Thats the first time I submit a Patch, so sorry the errors.

First I will give you a concrete example:
--------------------------------------------------------
~/bugz_24700> cat test.c
#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>

static void
fault_func (char *arg)
{
    *arg = 'b';
}

int main (int argc, char *argv[])
{
    int cnt = 0;
    char *ptr = NULL;

    printf("%s\n", "hello world");

    fault_func (ptr);

    printf("%s\n", ptr)

    return 0;
}
~/bugz_24700> gcc -m64 -g test.c
~/bugz_24700> gdb64 a.out
GNU gdb 6.4
Copyright 2005 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "ppc64-suse-linux"...Using host libthread_db library
"/lib64/power4/libthread_db.so.1".

(gdb) start
Breakpoint 1 at 0x10000604: file test.c, line 13.
Starting program: /home/pgilliam/bugz_24700/a.out
main (argc=1, argv=0xfffffb3f2b8) at test.c:13
13          int cnt = 0;
(gdb) n
14          char *ptr = NULL;
(gdb) n
16          printf("hello world\n");
(gdb) n
hello world
18          fault_func (ptr);
(gdb) set var ptr="foo"

Program received signal SIGSEGV, Segmentation fault.
0x000000000001cba0 in ?? ()
The program being debugged was signaled while in a function called from GDB.
GDB remains in the frame where the signal was received.
To change this behavior use "set unwindonsignal on"
Evaluation of the expression containing the function (at 0x1cba0) will be abando
ned.
(gdb) q
--------------------------------------------------------

At the begining of the investigation, it looks like
gdbarch_convert_from_func_ptr_addr is returning the wrong value,
and it is, but it's not its fault!
It calls ppc64_linux_convert_from_func_ptr_addr (correctly)
which needs to know the section the address is in.
So target_section_by_addr is called to get the section, but it returns null.
GDB finds one malloc (from /lib64/ld64.so.1 presumably) and the
program linked with another(presumably from libc)
This is from the shipping gdb64 from SLES10.

The problem was that 'ppc64_linux_convert_from_func_ptr_addr()' first wanted to
make sure that the function pointer for 'malloc()' pointed into an .opd section
before it was 'dereferenced'  It used 'target_section_by_addr()' to do the
search, and the section containing the PLT pointed to by the function pointer
was not in the section table being searched.  But there is another section
table.	The patch changes things so that if the search of the first section
table fails, the second table is searched.


--
José Flávio Aguilar Paulino
Software Engineer
LoP Toolchain Team
IBM


[-- Attachment #2: 24700.patch --]
[-- Type: text/x-patch, Size: 2025 bytes --]

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.81
diff -a -u -r1.81 ppc-linux-tdep.c
--- ppc-linux-tdep.c	9 Jan 2007 17:58:55 -0000	1.81
+++ ppc-linux-tdep.c	8 Feb 2007 01:11:15 -0000
@@ -753,11 +753,57 @@
 					CORE_ADDR addr,
 					struct target_ops *targ)
 {
+  CORE_ADDR addr2deref = 0;
   struct section_table *s = target_section_by_addr (targ, addr);
+  char buf[sizeof (ULONGEST)];
+  struct objfile *objfile;
+  struct obj_section *osect;
+  asection *sect;
+  CORE_ADDR sect_addr;
 
   /* Check if ADDR points to a function descriptor.  */
-  if (s && strcmp (s->the_bfd_section->name, ".opd") == 0)
-    return get_target_memory_unsigned (targ, addr, 8);
+
+
+  if (s)
+    {
+      if (strcmp (s->the_bfd_section->name, ".opd") != 0)
+        /* Found a the section, but it's not an .opd section. */
+        return addr;
+      addr2deref = addr;
+    }
+  else
+    /* The followin table search has been copied from printcmd.c */
+    ALL_OBJSECTIONS (objfile, osect)
+      {
+        /* Only process each object file once, even if there's a separate
+           debug file.  */
+        if (objfile->separate_debug_objfile_backlink)
+          continue;
+  
+        sect = osect->the_bfd_section;
+        sect_addr = overlay_mapped_address (addr, sect);
+    
+        if (osect->addr <= sect_addr && sect_addr < osect->endaddr)
+          {
+            if (strcmp (sect->name, ".opd") != 0)
+	      /* Found the section, but it's not an .opd section. */
+              return addr;
+            addr2deref = addr;
+	    break;
+          }
+      }
+
+  if (addr2deref) 
+    {
+      if (targ != &current_target)
+        return get_target_memory_unsigned (targ, addr2deref, 8);
+      else
+          {
+            gdb_assert (8 <= sizeof (buf));
+            target_read_memory(addr, buf, 8);
+            return extract_unsigned_integer (buf, 8);
+          }
+    }
 
   return addr;
 }

Re: [PATCH 24700]

[Daniel Jacobowitz]

On Thu, Mar 01, 2007 at 05:05:36PM -0400, Jose Flavio Aguilar Paulino wrote:
> The problem was that 'ppc64_linux_convert_from_func_ptr_addr()' first wanted to
> make sure that the function pointer for 'malloc()' pointed into an .opd section
> before it was 'dereferenced'  It used 'target_section_by_addr()' to do the
> search, and the section containing the PLT pointed to by the function pointer
> was not in the section table being searched.  But there is another section
> table.

Actually, there's a ton of section tables - this is an organizational
problem in GDB.  Originally they all meant different things.

Does using find_pc_section instead work in all cases?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH 24700]

[Thiago Jung Bauermann]

Hi,

I'm working with Flávio on this issue...

On Thu, 2007-03-01 at 15:40 -0500, Daniel Jacobowitz wrote:
> On Thu, Mar 01, 2007 at 05:05:36PM -0400, Jose Flavio Aguilar Paulino wrote:
> > The problem was that 'ppc64_linux_convert_from_func_ptr_addr()' first wanted to
> > make sure that the function pointer for 'malloc()' pointed into an .opd section
> > before it was 'dereferenced'  It used 'target_section_by_addr()' to do the
> > search, and the section containing the PLT pointed to by the function pointer
> > was not in the section table being searched.  But there is another section
> > table.
> 
> Actually, there's a ton of section tables - this is an organizational
> problem in GDB.  Originally they all meant different things.
> 
> Does using find_pc_section instead work in all cases?

In the testcase mentioned here, GDB wants to allocate memory in the
inferior so that it can set the string's value. It's GDB that's calling
malloc and not the inferior, so the inferior's PC is not anywhere near
malloc (unless we're lucky), as far as I know...
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [PATCH 24700]

[Daniel Jacobowitz]

On Thu, Mar 01, 2007 at 06:56:40PM -0300, Thiago Jung Bauermann wrote:
> In the testcase mentioned here, GDB wants to allocate memory in the
> inferior so that it can set the string's value. It's GDB that's calling
> malloc and not the inferior, so the inferior's PC is not anywhere near
> malloc (unless we're lucky), as far as I know...

That doesn't have anything to do with my question :-)  find_pc_section
does the PC -> section mapping pretty much the way the new code you've
duplicated does it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH 24700]

[Thiago Jung Bauermann]

On Thu, 2007-03-01 at 17:03 -0500, Daniel Jacobowitz wrote:
> On Thu, Mar 01, 2007 at 06:56:40PM -0300, Thiago Jung Bauermann wrote:
> > In the testcase mentioned here, GDB wants to allocate memory in the
> > inferior so that it can set the string's value. It's GDB that's calling
> > malloc and not the inferior, so the inferior's PC is not anywhere near
> > malloc (unless we're lucky), as far as I know...
> 
> That doesn't have anything to do with my question :-)  find_pc_section
> does the PC -> section mapping pretty much the way the new code you've
> duplicated does it.

Whooops, mislead by the function name. :-)
Now I see what you mean. We'll try this alternative then.
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: [PATCH 24700]

[Andreas Schwab]

Daniel Jacobowitz <drow@false.org> writes:

> That doesn't have anything to do with my question :-)  find_pc_section
> does the PC -> section mapping pretty much the way the new code you've
> duplicated does it.

The original implementation of ppc64_linux_convert_from_func_ptr_addr
actually used find_pc_section.  It was changed to use
target_section_by_addr a few years ago:

2003-10-24  Andrew Cagney  <cagney@redhat.com>

	* target.c: Include "gdbcore.h".
	(get_target_memory, get_target_memory_unsigned): New functions.
	* target.h (get_target_memory, get_target_memory_unsigned): Declare.
	* ppc-linux-tdep.c (ppc64_linux_convert_from_func_ptr_addr):
	Use get_target_memory_unsigned.
	* Makefile.in (target.o): Update dependencies.

None of these implementations can handle all cases satisfactory.  I also
tried a combination of both, but it wasn't any better either.  There are
basically two problems: one is that gdb is often trying to convert a
function pointer that was already converted, the other one is that the
conversion is done both on unrelocated and relocated objects.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."