Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Cagney <ac131313@cygnus.com>
To: gdb-patches@sourceware.cygnus.com
Subject: [rfa] Don't refer to previous arch in rs6000_gdbarch_init()
Date: Tue, 25 Sep 2001 19:03:00 -0000	[thread overview]
Message-ID: <3BB13779.3060906@cygnus.com> (raw)

Hello,

The attatched patch removes a reference to the old architecture (via 
TDEP -> gdbarch_tdep (current_gdbarch)) in rs6000_gdbarch_init().

Given that the previous architecture may not be a member of the rs6000 
family, using a value from the previous architectures tdep makes for a 
high risk operation.  I think rs6000 should instead arrange to keep a 
local copy of any stuff it wants to refer to.

Is this change to address the immediate problem ok?

	Andrew
From ac131313@cygnus.com Tue Sep 25 20:43:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [patch] Don't refer to current architecture when allocating a new one
Date: Tue, 25 Sep 2001 20:43:00 -0000
Message-id: <3BB14EC1.7060704@cygnus.com>
X-SW-Source: 2001-09/msg00341.html
Content-length: 948

Hello,

The attached fixes a potentially nasty but some what theoretical bug. 
The code in gdbarch_alloc() was refering to macros such as 
``TARGET_LONG_BIT'' when initializing various fields vis:

	gdbarch->target_long_long_bit = 2*TARGET_LONG_BIT;

Unfortunatly such macros refer to the global ``current_gdbarch'' and 
hence the code picks up the value of TARGET_LONG_BIT from the previous 
and not this new architecture :-(  The obvious fix would be to just not 
use the macros.  Unfortunatly due to the way a non-multi-arch 
configuration works, that isn't possible.

The attached addresses the problem by naming local variable designating 
the new architecture ``curent_gdbarch''.  That way all macros refer to 
that local variable and not the global.

As an asside, people multi-arching targets should be aware of this 
potential problem in their code.  I've a follow on patch to ensure it 
doesn't happen but it is some what prutal.

Andrew
From ac131313@cygnus.com Tue Sep 25 22:14:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [rfc] Swap out current when creating a new architecture
Date: Tue, 25 Sep 2001 22:14:00 -0000
Message-id: <3BB16441.30805@cygnus.com>
X-SW-Source: 2001-09/msg00342.html
Content-length: 861

Hello,

The attached changes the run-time environment within which a new 
architectures are created.  Briefly the simplified sequence:

- call XXX_gdbarch_init()
- swap out old architecture
- install new architecture

is changed to:

- swap out old architecture
- call XX_gdbarch_init()
- install new architecture

This has the effect of making current_gdbarch invalid for the lifetime 
of the XXX_gdbarch_init() call.

The motivation behind this change is to stop XXXX_gdbarch_init() 
functions refering (unintentionally I suspect) to the previous 
architecture.  I think it is proving effective since it has so far 
flushed out two bugs.

I can think of one additional tweek: add a ``gdb_assert (gdbarch != 
NULL)'' to each architecture method.  Without it a XXX_gdbarch_init() 
function that tries to use current_gdbarch will dump core :-/

thoughts?
Andrew
From ac131313@cygnus.com Tue Sep 25 23:22:00 2001
From: Andrew Cagney <ac131313@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [patch] Sort ``maint print architecture'' output
Date: Tue, 25 Sep 2001 23:22:00 -0000
Message-id: <3BB1740E.70601@cygnus.com>
X-SW-Source: 2001-09/msg00343.html
Content-length: 179

Hello,

The attached tweek sorts the output from ``maint print architecture''. 
It makes finding the value of one of those macro's much easier!

I'll check it in shortly.

Andrew
From kevinb@cygnus.com Wed Sep 26 00:30:00 2001
From: Kevin Buettner <kevinb@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [PATCH] solib-svr4.[hc]: Use gdbarch data mechanism...
Date: Wed, 26 Sep 2001 00:30:00 -0000
Message-id: <1010926073025.ZM625@ocotillo.lan>
X-SW-Source: 2001-09/msg00344.html
Content-length: 11120

I've just committed the patch below.

It changes solib-svr4.c to use the gdbarch data mechanism instead of
the swap mechanism for managing architecture specific link_map_offset
fetchers.  I believe this patch addresses Andrew's concerns in

    http://sources.redhat.com/ml/gdb-patches/2001-09/msg00296.html

I noticed something a bit odd which I'd like to point out (mainly to
Andrew).  init_fetch_link_map_offsets() now looks like this:

    static void *
    init_fetch_link_map_offsets (struct gdbarch *gdbarch)
    {
      struct link_map_offsets *(*flmo) =
	gdbarch_data (fetch_link_map_offsets_gdbarch_data);

      if (flmo == NULL)
	return legacy_fetch_link_map_offsets;
      else
	return flmo;
    }

The idea behind this function is to simply return the value set by
a call to set_solib_svr4_fetch_link_map_offsets() if such a call
was made, or to return legacy_fetch_link_map_offsets otherwise.

The odd thing about it is that the gdbarch parameter is never used. 
Instead, it makes use of the fact that current_gdbarch has been set to
the new architecture (and that gdbarch_data queries this new
architecture).  Perhaps this is nothing to worry about, but I had
to stare at gdbarch.c for a while to convince myself that it would
work as I wanted it to.  I'd feel better about the whole thing if
there were a version of gdbarch_data() which would take a
struct gdbarch * as an argument.  Maybe something like the following?

    void *
    get_gdbarch_data (struct gdbarch *gdbarch, struct gdbarch_data *data)
    {
      gdb_assert (data->index < gdbarch->nr_data);
      return gdbarch->data[data->index];
    }

(It seems to me that this provides a nice complement to set_gdbarch_data().)

Anyway, with the above in place, I could then do

    static void *
    init_fetch_link_map_offsets (struct gdbarch *gdbarch)
    {
      struct link_map_offsets *(*flmo) =
	get_gdbarch_data (gdbarch, fetch_link_map_offsets_gdbarch_data);
      ...

and would no longer care so much about the initialization details
in gdbarch.c (specifically gdbarch_update_p()).

	* solib-svr4.h (set_solib_svr4_fetch_link_map_offsets): Add
	argument ``gdbarch''.
	* solib-svr4.c (SVR4_FETCH_LINK_MAP_OFFSETS): Change default value.
	(default_svr4_fetch_link_map_offsets): Rename to
	legacy_fetch_link_map_offsets().
	(svr4_fetch_link_map_offsets): New function.
	(fetch_link_map_offsets, fetch_link_map_offsets_init): Deleted.
	(fetch_link_map_offsets_gdbarch_data): New static global.
	(set_solib_svr4_fetch_link_map_offsets): Add argument ``gdbarch''.
	Revise to invoke set_gdbarch_data().
	(init_fetch_link_map_offsets): Change return type and add an
	argument so that it may be used as a gdbarch_data initializer.
	(_initialize_svr4_solib): Eliminate use of gdbarch swap mechanism.
	Use gdbarch data mechanism instead.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.18
diff -u -p -r1.18 solib-svr4.c
--- solib-svr4.c	2001/09/20 20:07:55	1.18
+++ solib-svr4.c	2001/09/26 06:59:17
@@ -58,28 +58,21 @@
 #include "solib-svr4.h"
 
 #ifndef SVR4_FETCH_LINK_MAP_OFFSETS
-#define SVR4_FETCH_LINK_MAP_OFFSETS() fetch_link_map_offsets ()
+#define SVR4_FETCH_LINK_MAP_OFFSETS() svr4_fetch_link_map_offsets ()
 #endif
 
-static struct link_map_offsets *default_svr4_fetch_link_map_offsets (void);
+static struct link_map_offsets *svr4_fetch_link_map_offsets (void);
+static struct link_map_offsets *legacy_fetch_link_map_offsets (void);
 
-/* fetch_link_map_offsets is the pointer to the architecture specific
-   link map offsets fetching function.  It uses the gdbarch_swap
-   mechanism to change its value when the architecture changes.  */
-static struct link_map_offsets *(*fetch_link_map_offsets)(void) = 
-  default_svr4_fetch_link_map_offsets;
-
-/* fetch_link_map_offsets_init is like the above, but obtains its
-   value from a call to set_solib_svr4_fetch_link_map_offsets().
-   This latter function is intended to be called from a *_gdbarch_init()
-   function.  The value of ``fetch_link_map_offsets_init'' is used
-   to actually set ``fetch_link_map_offsets'' when the architecture
-   is installed.  */
-static struct link_map_offsets *(*fetch_link_map_offsets_init)(void) = 0;
+/* fetch_link_map_offsets_gdbarch_data is a handle used to obtain the
+   architecture specific link map offsets fetching function.  */
 
+static struct gdbarch_data *fetch_link_map_offsets_gdbarch_data;
+
 /* legacy_svr4_fetch_link_map_offsets_hook is a pointer to a function
    which is used to fetch link map offsets.  It will only be set
    by solib-legacy.c, if at all. */
+
 struct link_map_offsets *(*legacy_svr4_fetch_link_map_offsets_hook)(void) = 0;
 
 /* Link map info to include in an allocated so_list entry */
@@ -144,27 +137,6 @@ static char *main_name_list[] =
 };
 
 
-/* Fetch (and possibly build) an appropriate link_map_offsets structure
-   for native targets using struct definitions from link.h.  
-   
-   Note: For non-native targets (i.e. cross-debugging situations),
-   you need to define a target specific fetch_link_map_offsets()
-   function and call set_solib_svr4_fetch_link_map_offsets () to
-   register this function.  */
-
-static struct link_map_offsets *
-default_svr4_fetch_link_map_offsets (void)
-{
-  if (legacy_svr4_fetch_link_map_offsets_hook)
-    return legacy_svr4_fetch_link_map_offsets_hook ();
-  else
-    {
-      internal_error (__FILE__, __LINE__,
-"default_svr4_fetch_link_map_offsets called without legacy link_map support enabled.");
-      return 0;
-    }
-}
-
 /* Macro to extract an address from a solib structure.
    When GDB is configured for some 32-bit targets (e.g. Solaris 2.7
    sparc), BFD is configured to handle 64-bit targets, so CORE_ADDR is
@@ -1654,35 +1626,78 @@ svr4_relocate_section_addresses (struct 
   sec->endaddr += LM_ADDR (so);
 }
 
+/* Fetch a link_map_offsets structure for native targets using struct
+   definitions from link.h.  See solib-legacy.c for the function
+   which does the actual work.
+   
+   Note: For non-native targets (i.e. cross-debugging situations),
+   a target specific fetch_link_map_offsets() function should be
+   defined and registered via set_solib_svr4_fetch_link_map_offsets().  */
+
+static struct link_map_offsets *
+legacy_fetch_link_map_offsets (void)
+{
+  if (legacy_svr4_fetch_link_map_offsets_hook)
+    return legacy_svr4_fetch_link_map_offsets_hook ();
+  else
+    {
+      internal_error (__FILE__, __LINE__,
+                      "legacy_fetch_link_map_offsets called without legacy "
+		      "link_map support enabled.");
+      return 0;
+    }
+}
+
+/* Fetch a link_map_offsets structure using the method registered in the
+   architecture vector.  */
+
+static struct link_map_offsets *
+svr4_fetch_link_map_offsets (void)
+{
+  struct link_map_offsets *(*flmo)(void) =
+    gdbarch_data (fetch_link_map_offsets_gdbarch_data);
+
+  if (flmo == NULL)
+    {
+      internal_error (__FILE__, __LINE__, 
+                      "svr4_fetch_link_map_offsets: fetch_link_map_offsets "
+		      "method not defined for this architecture.");
+      return 0;
+    }
+  else
+    return (flmo ());
+}
+
 /* set_solib_svr4_fetch_link_map_offsets() is intended to be called by
-   a <arch>_gdbarch_init() function.  It uses ``fetch_link_map_offsets_init''
-   to temporarily hold a pointer to the link map offsets fetcher for
-   a particular architecture.  Once the architecture is actually installed,
-   init_fetch_link_map_offsets(), below, will be called to install this
-   value in ``fetch_link_map_offsets''.  After that, the gdbarch_swap
-   machinery will manage the contents of this variable whenever the
-   architecture changes.  */
+   a <arch>_gdbarch_init() function.  It is used to establish an
+   architecture specific link_map_offsets fetcher for the architecture
+   being defined.  */
 
 void
-set_solib_svr4_fetch_link_map_offsets (struct link_map_offsets *(*flmo) (void))
+set_solib_svr4_fetch_link_map_offsets (struct gdbarch *gdbarch,
+                                       struct link_map_offsets *(*flmo) (void))
 {
-  fetch_link_map_offsets_init = flmo;
+  set_gdbarch_data (gdbarch, fetch_link_map_offsets_gdbarch_data, flmo);
 }
 
-/* Initialize the value of ``fetch_link_map_offsets'' when a new
-   architecture is created.  set_solib_svr4_fetch_link_map_offsets()
-   is used to set the value that ``fetch_link_map_offsets'' should
-   be initialized to.  */
+/* Initialize the architecture specific link_map_offsets fetcher. 
+   This is called after <arch>_gdbarch_init() has set up its struct
+   gdbarch for the new architecture, so care must be taken to use the
+   value set by set_solib_svr4_fetch_link_map_offsets(), above.  We
+   do, however, attempt to provide a reasonable alternative (for
+   native targets anyway) if the <arch>_gdbarch_init() fails to call
+   set_solib_svr4_fetch_link_map_offsets().  */
 
-static void
-init_fetch_link_map_offsets (void)
+static void *
+init_fetch_link_map_offsets (struct gdbarch *gdbarch)
 {
-  if (fetch_link_map_offsets_init != NULL)
-    fetch_link_map_offsets = fetch_link_map_offsets_init;
-  else
-    fetch_link_map_offsets = default_svr4_fetch_link_map_offsets;
+  struct link_map_offsets *(*flmo) =
+    gdbarch_data (fetch_link_map_offsets_gdbarch_data);
 
-  fetch_link_map_offsets_init = NULL;
+  if (flmo == NULL)
+    return legacy_fetch_link_map_offsets;
+  else
+    return flmo;
 }
 
 static struct target_so_ops svr4_so_ops;
@@ -1690,9 +1705,8 @@ static struct target_so_ops svr4_so_ops;
 void
 _initialize_svr4_solib (void)
 {
-  register_gdbarch_swap (&fetch_link_map_offsets,
-                         sizeof (fetch_link_map_offsets),
-			 init_fetch_link_map_offsets);
+  fetch_link_map_offsets_gdbarch_data =
+    register_gdbarch_data (init_fetch_link_map_offsets, 0);
 
   svr4_so_ops.relocate_section_addresses = svr4_relocate_section_addresses;
   svr4_so_ops.free_so = svr4_free_so;
@@ -1706,4 +1720,3 @@ _initialize_svr4_solib (void)
   /* FIXME: Don't do this here.  *_gdbarch_init() should set so_ops. */
   current_target_so_ops = &svr4_so_ops;
 }
-
Index: solib-svr4.h
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.h,v
retrieving revision 1.2
diff -u -p -r1.2 solib-svr4.h
--- solib-svr4.h	2001/03/10 06:17:20	1.2
+++ solib-svr4.h	2001/09/26 06:59:17
@@ -64,8 +64,13 @@ struct link_map_offsets
     int l_name_size;
   };
 
-extern void set_solib_svr4_fetch_link_map_offsets (
-  struct link_map_offsets *(*func) (void));
+/* set_solib_svr4_fetch_link_map_offsets() is intended to be called by
+   a <arch>_gdbarch_init() function.  It is used to establish an
+   architecture specific link_map_offsets fetcher for the architecture
+   being defined.  */
+
+extern void set_solib_svr4_fetch_link_map_offsets
+  (struct gdbarch *gdbarch, struct link_map_offsets *(*func) (void));
 
 /* legacy_svr4_fetch_link_map_offsets_hook is a pointer to a function
    which is used to fetch link map offsets.  It will only be set


             reply	other threads:[~2001-09-25 19:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-09-25 19:03 Andrew Cagney [this message]
2001-09-26  8:05 ` Kevin Buettner
2001-09-29 14:12   ` Andrew Cagney

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=3BB13779.3060906@cygnus.com \
    --to=ac131313@cygnus.com \
    --cc=gdb-patches@sourceware.cygnus.com \
    /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