Re: [PATCH] solib-svr4.c: Fix set_solib_svr4_fetch_link_map_offsets

Re: [PATCH] solib-svr4.c: Fix set_solib_svr4_fetch_link_map_offsets

[Kevin Buettner]

On Sep 21, 12:05am, Mark Kettenis wrote:

> I have been working on disentangling SVR4 (ELF) and SunOS (a.out)
> shared library support a few weeks ago.  It's not quite finished yet
> (ELF is basically done, but I still need to finish the a.out stuff).
> Perhaps we should coordinate our actions?

Definitely.  Can you send me a patch of your current state?  (My current
state is simply what's in CVS...)

Kevin

Re: [PATCH] solib-svr4.c: Fix set_solib_svr4_fetch_link_map_offsets

[Andrew Cagney]

> As a heads up though, one of the problems is that solib-svr4.c
> actually contains (ifdef'd) shared library support for both SVR4 and
> SunOS.  I will be disentangling these mechanisms and will place SunOS
> shared library support in its own file.  That way we'll be able to do
> away with the SVR4_SHARED_LIBS macro and we won't accidentally attempt
> to ever include <link.h> from solib-svr4.c any longer.

To be honest, I've been very tempted to solve this problem using a very 
brutal approach: take solib-svr4.c, copy it to solib-aout.c, strip 
out/in SVR4-SHARED_LIBS code, and then tweek the config files to use either.

I think this is one of those cases where the desire to re-use the code 
was taken a little to far - better to get the interfaces right and have 
a clear separation.  Who cares about a little duplication :-)

enjoy,
Andrew

Re: [PATCH] solib-svr4.c: Fix set_solib_svr4_fetch_link_map_offsets

[Kevin Buettner]

On Sep 21,  9:56pm, Andrew Cagney wrote:

> > As a heads up though, one of the problems is that solib-svr4.c
> > actually contains (ifdef'd) shared library support for both SVR4 and
> > SunOS.  I will be disentangling these mechanisms and will place SunOS
> > shared library support in its own file.  That way we'll be able to do
> > away with the SVR4_SHARED_LIBS macro and we won't accidentally attempt
> > to ever include <link.h> from solib-svr4.c any longer.
> 
> To be honest, I've been very tempted to solve this problem using a very 
> brutal approach: take solib-svr4.c, copy it to solib-aout.c, strip 
> out/in SVR4-SHARED_LIBS code, and then tweek the config files to use either.
> 
> I think this is one of those cases where the desire to re-use the code 
> was taken a little to far - better to get the interfaces right and have 
> a clear separation.  Who cares about a little duplication :-)

Actually, I think this is the right approach.  The duplication of code
is actually desirable because it is then possible to make bug fixes to
the (formerly) common code without having to worry about breaking the
other case.  In other words, over time, the code can safely diverge.

I actually went down this path once - and was nearly finished, but at
the time I thought it might be important for both solib-svr4.c and
solib-aout.c (or perhaps we'll name it solib-sunos.c) to share
current_sos() (now called svr4_current_sos()).  But I no longer think
it's worthwhile to attempt to share current_sos() between these two
shared lib implementations; in fact, I think we may be able to simplify
current_sos() once the split is made.  Looking at the comments, it
appears that there's some SVR4-specific code that the SunOS
implementation doesn't need.  I wouldn't be at all surprised if the
reverse was also true.

Kevin

Re: [PATCH] solib-svr4.c: Fix set_solib_svr4_fetch_link_map_offsets

[Andrew Cagney]

> I actually went down this path once - and was nearly finished, but at
> the time I thought it might be important for both solib-svr4.c and
> solib-aout.c (or perhaps we'll name it solib-sunos.c)

I suspect the code started out SunOS<5 specific (and hence a.out) but 
over time was re-used by other platforms that used a.out object files 
and sun's shared library techniques.

Andrew

Re: [PATCH] solib-svr4.c: Fix set_solib_svr4_fetch_link_map_offsets

[Andrew Cagney]

> I recently attempted to use set_solib_svr4_fetch_link_map_offsets() to
> establish a Linux/PPC specific link map fetcher, but discovered a
> number of problems.  One of them is as follows...  I attempted to call
> set_solib_svr4_fetch_link_map_offsets() from rs6000_gdbarch_init().
> The <arch>_gdbarch_init() functions (as they are commonly named) are
> used to allocate and initialize a gdbarch struct which is then installed
> to define a new "architecture" after this function returns.

First an fyi.  Elena found a cute bug in gdbarch's init sequence - it 
leaves the old architecture installed while it is creating the new 
architecture vis:

	-	create new architecture
	-	swap out old architecture
	-	install new architecture
	-	initialize remainder of new architecture

  This leads to all sorts of confusion such as what you encountered here.

I'm working on a patch that will change the initialization behavour so 
that it:

	-	swaps out the old architecture
	-	creates the new architecture
	-	installs new architecture
	-	initializes remainder of new architecture

I recall NickD encountering a similar situation to yours.  The 
gdbarch_data object's behavour was changed to better handle this situtation.

Instead of having a global variable, have you looked at registering a 
data pointer and then calling set_gdbarch_data(), from 
set_solib_svr4_fetch_link_map_offsets(), to set it?

The only potential problem I can see with this is with non- multi-arch 
but there, looking at the code I think it is already handled.

Andrew

Re: [PATCH] solib-svr4.c: Fix set_solib_svr4_fetch_link_map_offsets

[Kevin Buettner]

On Sep 21, 10:31pm, Andrew Cagney wrote:

> Instead of having a global variable, have you looked at registering a 
> data pointer and then calling set_gdbarch_data(), from 
> set_solib_svr4_fetch_link_map_offsets(), to set it?

Well, I thought I had, but now that I look at it again, it does look
like this'll work.  I'll give it a try...

> The only potential problem I can see with this is with non- multi-arch 
> but there, looking at the code I think it is already handled.

I think you're right.

Kevin