[PATCH, gdbserver] Further cleanup of FDPIC/DSBT divergences

Luis Machado
2013-06-20T18:51:14Z

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

Hi,

At some point, c6x used different data structures for its DSBT-based
loadmap.

DSBT-based

struct target_loadmap
{
/* Protocol version number, must be zero. */
Elf32_Word version;
/* Pointer to the DSBT table, its size, and the DSBT index. */
unsigned *dsbt_table;
unsigned dsbt_size, dsbt_index;
/* Number of segments in this map. */
Elf32_Word nsegs;
/* The actual memory map. */
struct target_loadseg segs[/*nsegs*/];
};

FDPIC-based

struct target_loadmap
{
/* Protocol version number, must be zero. */
Elf32_Half version;
/* Number of segments in this map. */
Elf32_Half nsegs;
/* The actual memory map. */
struct target_loadseg segs[/*nsegs*/];
};

We shared a little bit of code with FDPIC-based targets though...

struct target_loadseg
{
/* Core address to which the segment is mapped. */
Elf32_Addr addr;
/* VMA recorded in the program header. */
Elf32_Addr p_vaddr;
/* Size of this segment in memory. */
Elf32_Word p_memsz;
};

Things have changed, and c6x is now using the exact same data structures
as FDPIC-based targets in uClibc. Please refer to
http://lists.uclibc.org/pipermail/uclibc/2013-May/047789.html for the
uClibc changes that led to this.

Mark Salter, the author of the uClibc change, has agreed with the
solution i proposed:
http://lists.uclibc.org/pipermail/uclibc/2013-May/047790.html.

It is all good, but we've been conditionalizing the c6x-specific
target_loadmap data structure based on the presence of PT_GETDSBT. This
has always been defined in uClibc and, since Mark's change, it doesn't
work as a hint of whether to use the new or the old target_loadmap data
structure anymore. Therefore we will/already have a potential problem
with backwards compatibility.

Bernhard has stated that backwards compatibility on uClibc's side is not
a problem: http://lists.uclibc.org/pipermail/uclibc/2013-June/047801.html.

With all that exposed, my proposed change to gdbserver is to drop all
the DSBT-specific bits, remove their definitions and explicitly use
FDPIC definitions instead, making things a little bit cleaner.

In the following patch i also changed the code slightly to stop defining
linux_read_loadmap to NULL and i switched to explicitly setting the
target hook to NULL in the absence of the required definition.

What do you think? Yao? Mike?

Luis

[-- Attachment #2: dsbt_fdpic.diff --]
[-- Type: text/x-patch, Size: 3290 bytes --]

2013-06-20 Luis Machado <lgustavo@codesourcery.com>

* linux-low.c: Remove check for PT_GETDSBT.
(target_loadmap): Remove data structure conditionalized by
the presence of PT_GETDSBT.
(LINUX_LOADMAP, LINUX_LOADMAP_EXEC,
LINUX_LOADMAP_INTERP): Remove definitions.
(linux_read_loadmap): Replace LINUX_LOADMAP_EXEC with
PTRACE_GETFDPIC_EXEC, LINUX_LOADMAP_INTERP with
PTRACE_GETFDPIC_INTERP and LINUX_LOADMAP with PTRACE_GETFDPIC.
Do not set linux_read_loadmap to NULL in the absence of
PTRACE_GETFDPIC.
(linux_target_ops) <read_loadmap>: Only set to linux_read_loadmap
in the presence of PTRACE_GETFDPIC.

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index bb7298a..df53775 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -5263,7 +5263,7 @@ linux_qxfer_spu (const char *annex, unsigned char *readbuf,
return ret;
}

-#if defined PT_GETDSBT || defined PTRACE_GETFDPIC
+#if defined PTRACE_GETFDPIC
struct target_loadseg
{
/* Core address to which the segment is mapped. */
@@ -5274,23 +5274,6 @@ struct target_loadseg
Elf32_Word p_memsz;
};

-# if defined PT_GETDSBT
-struct target_loadmap
-{
- /* Protocol version number, must be zero. */
- Elf32_Word version;
- /* Pointer to the DSBT table, its size, and the DSBT index. */
- unsigned *dsbt_table;
- unsigned dsbt_size, dsbt_index;
- /* Number of segments in this map. */
- Elf32_Word nsegs;
- /* The actual memory map. */
- struct target_loadseg segs[/*nsegs*/];
-};
-# define LINUX_LOADMAP PT_GETDSBT
-# define LINUX_LOADMAP_EXEC PTRACE_GETDSBT_EXEC
-# define LINUX_LOADMAP_INTERP PTRACE_GETDSBT_INTERP
-# else
struct target_loadmap
{
/* Protocol version number, must be zero. */
@@ -5300,10 +5283,6 @@ struct target_loadmap
/* The actual memory map. */
struct target_loadseg segs[/*nsegs*/];
};
-# define LINUX_LOADMAP PTRACE_GETFDPIC
-# define LINUX_LOADMAP_EXEC PTRACE_GETFDPIC_EXEC
-# define LINUX_LOADMAP_INTERP PTRACE_GETFDPIC_INTERP
-# endif

static int
linux_read_loadmap (const char *annex, CORE_ADDR offset,
@@ -5315,13 +5294,13 @@ linux_read_loadmap (const char *annex, CORE_ADDR offset,
unsigned int actual_length, copy_length;

if (strcmp (annex, "exec") == 0)
- addr = (int) LINUX_LOADMAP_EXEC;
+ addr = (int) PTRACE_GETFDPIC_EXEC;
else if (strcmp (annex, "interp") == 0)
- addr = (int) LINUX_LOADMAP_INTERP;
+ addr = (int) PTRACE_GETFDPIC_INTERP;
else
return -1;

- if (ptrace (LINUX_LOADMAP, pid, addr, &data) != 0)
+ if (ptrace (PTRACE_GETFDPIC, pid, addr, &data) != 0)
return -1;

if (data == NULL)
@@ -5337,9 +5316,7 @@ linux_read_loadmap (const char *annex, CORE_ADDR offset,
memcpy (myaddr, (char *) data + offset, copy_length);
return copy_length;
}
-#else
-# define linux_read_loadmap NULL
-#endif /* defined PT_GETDSBT || defined PTRACE_GETFDPIC */
+#endif /* defined PTRACE_GETFDPIC */

static void
linux_process_qsupported (const char *query)
@@ -6037,7 +6014,11 @@ static struct target_ops linux_target_ops = {
NULL,
#endif
linux_common_core_of_thread,
+#if defined PTRACE_GETFDPIC
linux_read_loadmap,
+#else
+ NULL,
+#endif /* defined PTRACE_GETFDPIC */
linux_process_qsupported,
linux_supports_tracepoints,
linux_read_pc,