From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cagney 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 Message-id: <3BB13779.3060906@cygnus.com> X-SW-Source: 2001-09/msg00340.html 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 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 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 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 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 _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 _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 _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 _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 _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