Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Luis Machado <lgustavo@codesourcery.com>
To: "'gdb-patches@sourceware.org'" <gdb-patches@sourceware.org>,
	 Mike Frysinger <vapier@gentoo.org>
Subject: [RFC, gdbserver] Avoid defining linux_read_offsets when the target does not need it
Date: Wed, 15 May 2013 11:25:00 -0000	[thread overview]
Message-ID: <519370AE.50908@codesourcery.com> (raw)

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

Hi,

uClibc-based targets can load their programs in an offset in memory, and 
this information has historically been communicated to gdbserver via 
ptrace with the following options: PT_TEXT_ADDR, PT_DATA_ADDR and 
PT_TEXT_END_ADDR.

We have a target that uses loadmaps as opposed to the above mechanism. 
It is just another ptrace request, but it doesn't use linux_read_offsets 
at all.

linux_read_offsets is always defined (for uClibc-based targets) though, 
so gdbserver eventually calls it and it obviously returns an error. This 
error is propagated all the way to GDB, displaying an alarming and 
cryptic warning on the host's side.

"warning: Remote failure reply: E01"

Though the warning is harmless, the handling of this scenario could be 
improved a little.

The following patch conditionally defines linux_read_offsets only for 
targets that are uClibc-based and that define PT_TEXT_ADDR, PT_DATA_ADDR 
and PT_TEXT_END_ADDR.

Targets using other mechanisms won't define this function then, making 
gdbserver return an empty response to GDB (meaning packet not 
supported). GDB is happy again.

Additionally, i see 3 different archs defining local constants. coldfire 
and c6x still seem to lack definitions in the kernel, but blackfin 
already has those.

I've asked Mike Frysinger whether these can be removed. If so, i'll 
update the patch.

Alternativelly, we could forward the burden of fetching offsets to the 
target backends, though the number of targets that would use this is 
quite limited.

Luis

[-- Attachment #2: 0001-qoffsets_uclinux.diff --]
[-- Type: text/x-patch, Size: 3672 bytes --]

2013-05-15  Luis Machado  <lgustavo@codesourcery.com>

	* linux-low.c: Move definition checks upwards for PT_TEXT_ADDR,
	PT_DATA_ADDR and PT_TEXT_END_ADDR. Update comments.
	(linux_read_offsets): Remove PT_TEXT_ADDR, PT_DATA_ADDR and
	PT_TEXT_END_ADDR guards. Update comments.
	(linux_target_op) <read_offsets): Conditionally define to
	linux_read_offsets if the target is UCLIBC and if it defines
	PT_TEXT_ADDR, PT_DATA_ADDR and PT_TEXT_END_ADDR.

Index: gdb-head/gdb/gdbserver/linux-low.c
===================================================================
--- gdb-head.orig/gdb/gdbserver/linux-low.c	2013-04-17 10:24:52.859499119 +0200
+++ gdb-head/gdb/gdbserver/linux-low.c	2013-05-15 12:22:04.141660168 +0200
@@ -84,6 +84,30 @@
 #endif
 #endif
 
+/* Some targets did not define these ptrace constants from the start,
+   so gdbserver defines them locally here.  In the future, these may
+   be removed after they are added to asm/ptrace.h.  */
+#if !(defined(PT_TEXT_ADDR) \
+      || defined(PT_DATA_ADDR) \
+      || defined(PT_TEXT_END_ADDR))
+#if defined(__mcoldfire__)
+/* These are still undefined in recent (3.10) kernels.  */
+#define PT_TEXT_ADDR 49*4
+#define PT_DATA_ADDR 50*4
+#define PT_TEXT_END_ADDR  51*4
+/* BFIN already defines these constants in recent (3.10) kernels.  */
+#elif defined(BFIN)
+#define PT_TEXT_ADDR 220
+#define PT_TEXT_END_ADDR 224
+#define PT_DATA_ADDR 228
+/* These are still undefined in recent (3.10) kernels.  */
+#elif defined(__TMS320C6X__)
+#define PT_TEXT_ADDR     (0x10000*4)
+#define PT_DATA_ADDR     (0x10004*4)
+#define PT_TEXT_END_ADDR (0x10008*4)
+#endif
+#endif
+
 #ifdef HAVE_LINUX_BTRACE
 # include "linux-btrace.h"
 #endif
@@ -4833,25 +4857,14 @@ linux_stopped_data_address (void)
   return lwp->stopped_data_address;
 }
 
-#if defined(__UCLIBC__) && defined(HAS_NOMMU)
-#if ! (defined(PT_TEXT_ADDR) \
-       || defined(PT_DATA_ADDR) \
-       || defined(PT_TEXT_END_ADDR))
-#if defined(__mcoldfire__)
-/* These should really be defined in the kernel's ptrace.h header.  */
-#define PT_TEXT_ADDR 49*4
-#define PT_DATA_ADDR 50*4
-#define PT_TEXT_END_ADDR  51*4
-#elif defined(BFIN)
-#define PT_TEXT_ADDR 220
-#define PT_TEXT_END_ADDR 224
-#define PT_DATA_ADDR 228
-#elif defined(__TMS320C6X__)
-#define PT_TEXT_ADDR     (0x10000*4)
-#define PT_DATA_ADDR     (0x10004*4)
-#define PT_TEXT_END_ADDR (0x10008*4)
-#endif
-#endif
+#if defined(__UCLIBC__) && defined(HAS_NOMMU)	      \
+    && defined(PT_TEXT_ADDR) && defined(PT_DATA_ADDR) \
+    && defined(PT_TEXT_END_ADDR)
+
+/* This is only used for targets that define PT_TEXT_ADDR,
+   PT_DATA_ADDR and PT_TEXT_END_ADDR.  If those are not defined, supposedly
+   the target has different ways of acquiring this information, like
+   loadmaps.  */
 
 /* Under uClinux, programs are loaded at non-zero offsets, which we need
    to tell gdb about.  */
@@ -4859,7 +4872,6 @@ linux_stopped_data_address (void)
 static int
 linux_read_offsets (CORE_ADDR *text_p, CORE_ADDR *data_p)
 {
-#if defined(PT_TEXT_ADDR) && defined(PT_DATA_ADDR) && defined(PT_TEXT_END_ADDR)
   unsigned long text, text_end, data;
   int pid = lwpid_of (get_thread_lwp (current_inferior));
 
@@ -4888,7 +4900,6 @@ linux_read_offsets (CORE_ADDR *text_p, C
 
       return 1;
     }
-#endif
  return 0;
 }
 #endif
@@ -5887,7 +5898,9 @@ static struct target_ops linux_target_op
   linux_remove_point,
   linux_stopped_by_watchpoint,
   linux_stopped_data_address,
-#if defined(__UCLIBC__) && defined(HAS_NOMMU)
+#if defined(__UCLIBC__) && defined(HAS_NOMMU)	      \
+    && defined(PT_TEXT_ADDR) && defined(PT_DATA_ADDR) \
+    && defined(PT_TEXT_END_ADDR)
   linux_read_offsets,
 #else
   NULL,

             reply	other threads:[~2013-05-15 11:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-15 11:25 Luis Machado [this message]
2013-05-15 14:14 ` Pedro Alves
2013-05-16 10:34   ` Luis Machado
2013-05-15 14:17 ` Yao Qi
2013-05-15 16:06 ` Mike Frysinger
2013-05-15 16:26   ` Luis Machado
2013-05-15 17:12     ` Mike Frysinger
2013-05-15 18:08       ` Luis Machado
2013-05-15 18:51         ` Mike Frysinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=519370AE.50908@codesourcery.com \
    --to=lgustavo@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=vapier@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox