Re: [PATCH RFA] solib-svr4.c patch for dynamic executables

[Jim Blandy <jimb@zwingli.cygnus.com>]

The way this patch decides whether or not to relocate the main
executable seems questionable to me.  It looks like we're basing our
decision on conditions which are suggestive, but not definitive.

We should look at the ABI to see what rules `exec' follows to decide
whether to relocate the main executable, and then make GDB use the
same rules.

At the very least, the test for the ".interp" section should be
explained.



Kevin Buettner <kevinb@cygnus.com> writes:

> 
> The patch below is based on some patches from Ulrich Drepper that
> were previously posted to this and other forums.
> 
> These patches make it possible to debug executables (on Linux and
> other SVR4-like systems) in which the program is loaded at an address
> different than the one given by the executable's symbolic information.
> 
> As indicated in the comments in the patch, it is likely that this code
> will only be useful for debugging dynamic linkers since the code makes
> sure that there is no .interp section prior to attempting to do any
> relocations.  (Though I suppose it's possible that it would work on a
> statically linked binary which ended up being relocated for some
> reason also.)
> 
> I was hoping to arrive at a more generic solution, but after working
> on some similar functionality for AIX5, I've concluded that the
> mechanisms for determining which sections need to be relocated and by
> how much is very much target dependent.  When I compare the code
> below with the code that I wrote for AIX5, there seems to be very
> little in common aside from the bit about allocating an array
> of section_offset structs and then later calling objfile_relocate()
> with section_offsets filled in as appropriate.
> 
> Anyway... I've done some limited testing of the patch below on
> Linux/x86 (Red Hat 6.2 and 7.0) with /lib/ld-linux.so.2 and it
> works for me; it's even possible to rerun the program again.
> (I wish I could get the dynamic linker to load at a different
> locations on subsequent runs; I think this should work, but have
> been unable to test it.)  Also, I've asked Ulrich to test these
> changes (who better?) and he reports that they work fine for him.
> 
> Finally, I've run the gdb testsuite on Linux/x86 and see no
> regressions.  (However, someone's changes in the last couple of days
> has made the number of FAILs creep up to 16 from 14 on my machine.)
> 
> Okay to commit?
> 
> 	Changes based on patch from Ulrich Drepper:
> 	* solib-svr4.c (svr4_relocate_main_executable): New function.
> 	(svr4_solib_create_inferior_hook):  Call
> 	svr4_relocate_main_executable.
> 
> Index: solib-svr4.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-svr4.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 solib-svr4.c
> --- solib-svr4.c	2000/10/30 23:31:17	1.2
> +++ solib-svr4.c	2000/11/03 23:22:03
> @@ -1437,6 +1437,54 @@ svr4_special_symbol_handling (void)
>  #endif /* !SVR4_SHARED_LIBS */
>  }
>  
> +/* Relocate the main executable.  This function should be called
> +   upon stopping the inferior process at the entry point to the
> +   program.  The entry point from BFD is compared to the PC and
> +   if they are different, the main executable is relocated by
> +   the proper amount.  
> +   
> +   This code is somewhat naive in that it assumes that each
> +   section needs to be relocated by the same amount.
> +
> +   Also, as written it will only attempt to relocate executables
> +   which lack interpreter sections.   It seems likely that only
> +   dynamic linker executables will get relocated.  */
> +
> +static void
> +svr4_relocate_main_executable (void)
> +{
> +  asection *interp_sect;
> +  CORE_ADDR pc = read_pc ();
> +
> +  interp_sect = bfd_get_section_by_name (exec_bfd, ".interp");
> +  if (interp_sect == NULL && bfd_get_start_address (exec_bfd) != pc)
> +    {
> +      struct cleanup *old_chain;
> +      struct section_offsets *new_offsets;
> +      int i, changed;
> +      CORE_ADDR displacement;
> +
> +      displacement = pc - bfd_get_start_address (exec_bfd);
> +      changed = 0;
> +
> +      new_offsets = xcalloc (sizeof (struct section_offsets),
> +			     symfile_objfile->num_sections);
> +      old_chain = make_cleanup (free, new_offsets);
> +
> +      for (i = 0; i < symfile_objfile->num_sections; i++)
> +	{
> +	  if (displacement != ANOFFSET (symfile_objfile->section_offsets, i))
> +	    changed = 1;
> +	  new_offsets->offsets[i] = displacement;
> +	}
> +
> +      if (changed)
> +	objfile_relocate (symfile_objfile, new_offsets);
> +
> +      do_cleanups (old_chain);
> +    }
> +}
> +
>  /*
>  
>     GLOBAL FUNCTION
> @@ -1489,9 +1537,12 @@ svr4_special_symbol_handling (void)
>     Also, what if child has exit()ed?  Must exit loop somehow.
>   */
>  
> -void
> +static void
>  svr4_solib_create_inferior_hook (void)
>  {
> +  /* Relocate the main executable if necessary.  */
> +  svr4_relocate_main_executable ();
> +
>    /* If we are using the BKPT_AT_SYMBOL code, then we don't need the base
>       yet.  In fact, in the case of a SunOS4 executable being run on
>       Solaris, we can't get it yet.  current_sos will get it when it needs
> 
> 
From jimb@zwingli.cygnus.com Tue Nov 07 13:51:00 2000
From: Jim Blandy <jimb@zwingli.cygnus.com>
To: Andrei Petrov <and@genesyslab.com>
Cc: Nick Duffek <nsd@redhat.com>, gdb-patches@sourceware.cygnus.com, cagney@redhat.com
Subject: Re: gdb under solaris7
Date: Tue, 07 Nov 2000 13:51:00 -0000
Message-id: <npvgtz8n7y.fsf@zwingli.cygnus.com>
References: <Pine.GSO.4.10.10011071155420.507-100000@muppet>
X-SW-Source: 2000-11/msg00074.html
Content-length: 2589

We'd definitely like to use this patch.  We'll need you to assign your
copyright interest this patch to the Free Software Foundation.  I'll
send you the details in private mail.

(I've asked Stallman if we really need the assignment for a mostly
mechanical patch like this; it might not be necessary.  However, it
probably will, so let's get the paperwork rolling now.)


> uh, I didn't think it might get into.
> Glad because it might help somebody.
> 
> On Tue, 7 Nov 2000, Nick Duffek wrote:
> 
> > On 30-Aug-2000, Andrei Petrov wrote:
> > 
> > >The patch below lets to build gdb with SUNWspro cc compiler.
> > 
> > Thanks.  Speaking as a Solaris/x86 maintainer, I approve of the
> > sol-thread.c part for Solaris/x86.  I have some comments on the rest,
> > which you can feel free to ignore since I can't approve or disapprove
> > it. :-)
> > 
> > In various files:
> > >!   char raw_buffer[MAX_REGISTER_RAW_SIZE];
> > [...]
> > >!   char *raw_buffer = alloca(MAX_REGISTER_RAW_SIZE);
> > 
> > All of those changes look right to me.
> > 
> > remote.c:
> > >!   scan = (char *) ref;
> > [...]
> > >!   scan = (unsigned char *) ref;
> > 
> > Looks right.
> > 
> > remote.c:
> > >!       pkt = unpack_int (pkt, &tag);	/* tag */
> > [...]
> > >!       pkt = unpack_int (pkt, (int *)&tag);	/* tag */
> > 
> > How about declaring tag as an int instead?  Better not to use typecasts
> > when easily avoidable, I think.  Andrew?
> > 
> > remote.c:
> > >!       p = (unsigned char *) bfd_get_section_name (abfd, sect);
> > [...]
> > >!       p = (char *) bfd_get_section_name (abfd, sect);
> > 
> > Yup.
> > 
> > sparc-tdep.c:
> > >! sparc_software_single_step (enum target_signal ignore,	/* pid, but we don't need it */
> > [...]
> > >! sparc_software_single_step (/*enum target_signal*/ uint ignore,	/* pid, but we don't need it */
> > 
> > What's the reason for this change?
> 
> I couldn't get it compiled without this change as far as I remember,
> I don't think I tried to get proper thing for this one.
> It hard to get motivited when you see argument named ignore:-).
> 
> > 
> > sparc-tdep.c:
> > >!   static LONGEST call_dummy_64[] = 
> > [...]
> > >!   static unsigned LONGEST call_dummy_64[] = 
> > 
> > Is the compiler complaining about the values being too big for signed long
> > long?  I suggest ULONGEST instead of unsigned LONGEST along with a comment
> > explaining the reason for the change.
> Yes, I just didn't understand the reason for compiler's complain.
> I didn't find ULONGEST so the change.
> 
> > 
> > Nick Duffek
> > nsd@redhat.com
> > 
> Regards,
> 	Andrey
> 
> 
From cgf@redhat.com Tue Nov 07 13:56:00 2000
From: Christopher Faylor <cgf@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Re: [RFA] 'set step-mode' to control the step command.
Date: Tue, 07 Nov 2000 13:56:00 -0000
Message-id: <20001107165605.A12474@redhat.com>
References: <20001106144634.A4632@redhat.com> <14855.5477.538483.447944@kwikemart.cygnus.com> <20001106170332.A5533@redhat.com>
X-SW-Source: 2000-11/msg00075.html
Content-length: 8846

On Mon, Nov 06, 2000 at 05:03:32PM -0500, Christopher Faylor wrote:
>So, I will submit a revised patch with a enum'ized step_over_calls but keep
>the step_stop_if_no_debug, if that is ok.

And, here it is.  There were no regressions with this patch.

I haven't added a test suite entry for this but it is coming.

Is this ok?

cgf

2000-11-07  Christopher Faylor <cgf@cygnus.com>

        * inferior.h (step_over_calls_kind): New enum to clarify values in step_over_calls.
        * infcmd.c (step_over_calls): Change definition.
	(step_1): Use new enum values in relation to step_over_calls.
        (step_once): Ditto.
        (until_next_command): Ditto.
        * infrun.c (clear_proceed_status): Ditto.
        (handle_inferior_event): Ditto.

2000-11-06  Stephane Carrez  <Stephane.Carrez@sun.com>

        * inferior.h (step_stop_if_no_debug): New variable.
        * infrun.c (step_stop_if_no_debug): Declare.
        (handle_inferior_event): Stop the step command if we entered a function
        without line info.
        (_initialize_infrun): New command 'set step-mode' to control the step
        command.
        * infcmd.c (step_once): Switch to stepi mode if there is no line info
        (and switching is enabled).

Index: infcmd.c
===================================================================
RCS file: /cvs/uberbaum/gdb/infcmd.c,v
retrieving revision 1.12
diff -u -p -r1.12 infcmd.c
--- infcmd.c	2000/10/30 15:32:51	1.12
+++ infcmd.c	2000/11/07 06:28:06
@@ -178,12 +178,8 @@ CORE_ADDR step_frame_address;
 
 CORE_ADDR step_sp;
 
-/* 1 means step over all subroutine calls.
-   0 means don't step over calls (used by stepi).
-   -1 means step over calls to undebuggable functions.  */
+enum step_over_calls_kind step_over_calls;
 
-int step_over_calls;
-
 /* If stepping, nonzero means step count is > 1
    so don't print frame next time inferior stops
    if it stops due to stepping.  */
@@ -513,11 +509,11 @@ which has no line number information.\n"
 		/* It is stepi.
 		   Don't step over function calls, not even to functions lacking
 		   line numbers.  */
-		step_over_calls = 0;
+		step_over_calls = STEP_OVER_NONE;
 	    }
 
 	  if (skip_subroutines)
-	    step_over_calls = 1;
+	    step_over_calls = STEP_OVER_ALL;
 
 	  step_multi = (count > 1);
 	  proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 1);
@@ -607,7 +603,13 @@ step_once (int skip_subroutines, int sin
       if (!single_inst)
 	{
 	  find_pc_line_pc_range (stop_pc, &step_range_start, &step_range_end);
-	  if (step_range_end == 0)
+
+	  /* If we have no line info, switch to stepi mode.  */
+	  if (step_range_end == 0 && step_stop_if_no_debug)
+	    {
+	      step_range_start = step_range_end = 1;
+	    }
+	  else if (step_range_end == 0)
 	    {
 	      char *name;
 	      if (find_pc_partial_function (stop_pc, &name, &step_range_start,
@@ -628,11 +630,11 @@ which has no line number information.\n"
 	    /* It is stepi.
 	       Don't step over function calls, not even to functions lacking
 	       line numbers.  */
-	    step_over_calls = 0;
+	    step_over_calls = STEP_OVER_NONE;
 	}
 
       if (skip_subroutines)
-	step_over_calls = 1;
+	step_over_calls = STEP_OVER_ALL;
 
       step_multi = (count > 1);
       arg1 =
@@ -958,7 +960,7 @@ until_next_command (int from_tty)
       step_range_end = sal.end;
     }
 
-  step_over_calls = 1;
+  step_over_calls = STEP_OVER_ALL;
   step_frame_address = FRAME_FP (frame);
   step_sp = read_sp ();
 
Index: inferior.h
===================================================================
RCS file: /cvs/uberbaum/gdb/inferior.h,v
retrieving revision 1.11
diff -u -p -r1.11 inferior.h
--- inferior.h	2000/08/01 14:48:01	1.11
+++ inferior.h	2000/11/07 06:28:06
@@ -125,6 +125,11 @@ extern void clear_proceed_status (void);
 
 extern void proceed (CORE_ADDR, enum target_signal, int);
 
+/* When set, stop the 'step' command if we enter a function which has
+   no line number information.  The normal behavior is that we step
+   over such function.  */
+extern int step_stop_if_no_debug;
+
 extern void kill_inferior (void);
 
 extern void generic_mourn_inferior (void);
@@ -335,7 +340,12 @@ extern CORE_ADDR step_sp;
 /* 1 means step over all subroutine calls.
    -1 means step over calls to undebuggable functions.  */
 
-extern int step_over_calls;
+enum step_over_calls_kind
+  {
+    STEP_OVER_NONE,
+    STEP_OVER_ALL,
+    STEP_OVER_UNDEBUGGABLE,
+  } step_over_calls;
 
 /* If stepping, nonzero means step count is > 1
    so don't print frame next time inferior stops
Index: infrun.c
===================================================================
RCS file: /cvs/uberbaum/gdb/infrun.c,v
retrieving revision 1.20
diff -u -p -r1.20 infrun.c
--- infrun.c	2000/10/30 15:32:51	1.20
+++ infrun.c	2000/11/07 06:28:06
@@ -84,6 +84,11 @@ void _initialize_infrun (void);
 int inferior_ignoring_startup_exec_events = 0;
 int inferior_ignoring_leading_exec_events = 0;
 
+/* When set, stop the 'step' command if we enter a function which has
+   no line number information.  The normal behavior is that we step
+   over such function.  */
+int step_stop_if_no_debug = 0;
+
 /* In asynchronous mode, but simulating synchronous execution. */
 
 int sync_execution = 0;
@@ -940,7 +945,7 @@ clear_proceed_status (void)
   step_range_start = 0;
   step_range_end = 0;
   step_frame_address = 0;
-  step_over_calls = -1;
+  step_over_calls = STEP_OVER_UNDEBUGGABLE;
   stop_after_trap = 0;
   stop_soon_quietly = 0;
   proceed_to_finish = 0;
@@ -2612,7 +2617,7 @@ handle_inferior_event (struct execution_
        loader dynamic symbol resolution code, we keep on single stepping
        until we exit the run time loader code and reach the callee's
        address.  */
-    if (step_over_calls < 0 && IN_SOLIB_DYNSYM_RESOLVE_CODE (stop_pc))
+    if (step_over_calls == STEP_OVER_UNDEBUGGABLE && IN_SOLIB_DYNSYM_RESOLVE_CODE (stop_pc))
       {
 	CORE_ADDR pc_after_resolver = SKIP_SOLIB_RESOLVER (stop_pc);
 
@@ -2733,7 +2738,7 @@ handle_inferior_event (struct execution_
       {
 	/* It's a subroutine call.  */
 
-	if (step_over_calls == 0)
+	if (step_over_calls == STEP_OVER_NONE)
 	  {
 	    /* I presume that step_over_calls is only 0 when we're
 	       supposed to be stepping at the assembly language level
@@ -2744,7 +2749,7 @@ handle_inferior_event (struct execution_
 	    return;
 	  }
 
-	if (step_over_calls > 0 || IGNORE_HELPER_CALL (stop_pc))
+	if (step_over_calls == STEP_OVER_ALL || IGNORE_HELPER_CALL (stop_pc))
 	  {
 	    /* We're doing a "next".  */
 
@@ -2810,6 +2815,18 @@ handle_inferior_event (struct execution_
 	      return;
 	    }
 	}
+
+	/* If we have no line number and the step-stop-if-no-debug
+	   is set, we stop the step so that the user has a chance to
+	   switch in assembly mode.  */
+	if (step_over_calls == STEP_OVER_UNDEBUGGABLE && step_stop_if_no_debug)
+	  {
+	    stop_step = 1;
+	    print_stop_reason (END_STEPPING_RANGE, 0);
+	    stop_stepping (ecs);
+	    return;
+	  }
+
 	step_over_function (ecs);
 	keep_going (ecs);
 	return;
@@ -3934,7 +3951,7 @@ struct inferior_status
   CORE_ADDR step_range_start;
   CORE_ADDR step_range_end;
   CORE_ADDR step_frame_address;
-  int step_over_calls;
+  enum step_over_calls_kind step_over_calls;
   CORE_ADDR step_resume_break_address;
   int stop_after_trap;
   int stop_soon_quietly;
@@ -4308,5 +4325,14 @@ step == scheduler locked during every si
 			&setlist);
 
   c->function.sfunc = set_schedlock_func;	/* traps on target vector */
+  add_show_from_set (c, &showlist);
+
+  c = add_set_cmd ("step-mode", class_run,
+		   var_boolean, (char*) &step_stop_if_no_debug,
+"Set mode of the step operation. When set, doing a step over a\n\
+function without debug line information will stop at the first\n\
+instruction of that function. Otherwise, the function is skipped and\n\
+the step command stops at a different source line.",
+			&setlist);
   add_show_from_set (c, &showlist);
 }
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/uberbaum/gdb/doc/gdb.texinfo,v
retrieving revision 1.28
diff -u -p -r1.28 gdb.texinfo
--- gdb.texinfo	2000/10/16 07:34:02	1.28
+++ gdb.texinfo	2000/11/07 06:28:09
@@ -3323,6 +3323,16 @@ The @code{next} command only stops at th
 source line.  This prevents multiple stops that could otherwise occur in
 switch statements, for loops, etc.
 
+@kindex set step-mode
+@item set step-mode
+@itemx set step-mode on
+When stepping over functions, cause gdb to stop at the first instruction
+of a function which contains no debug line information.
+
+@itemx set step-mode off
+When stepping over functions, cause gdb to skip over any function which
+contains no debug information.  This is the default.
+
 @kindex finish
 @item finish
 Continue running until just after function in the selected stack frame
From drepper@redhat.com Tue Nov 07 14:29:00 2000
From: Ulrich Drepper <drepper@redhat.com>
To: Jim Blandy <jimb@cygnus.com>
Cc: Kevin Buettner <kevinb@cygnus.com>, gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFA] solib-svr4.c patch for dynamic executables
Date: Tue, 07 Nov 2000 14:29:00 -0000
Message-id: <m33dh34dry.fsf@otr.mynet.cygnus.com>
References: <1001104012305.ZM6714@ocotillo.lan> <np1ywna3f4.fsf@zwingli.cygnus.com>
X-SW-Source: 2000-11/msg00076.html
Content-length: 695

Jim Blandy <jimb@cygnus.com> writes:

> We should look at the ABI to see what rules `exec' follows to decide
> whether to relocate the main executable, and then make GDB use the
> same rules.

The rule is: binaries with e_type == ET_DYN have no fixed load address.
Don't know how to get this out of gdb data structures.

Note that this will include all DSOs but this is fine: if somebody
executes a DSO which is not prepared to do this it will crash.  Just
what you expect.

-- 
---------------.                          ,-.   1325 Chesapeake Terrace
Ulrich Drepper  \    ,-------------------'   \  Sunnyvale, CA 94089 USA
Red Hat          `--' drepper at redhat.com   `------------------------
From jimb@zwingli.cygnus.com Tue Nov 07 15:37:00 2000
From: Jim Blandy <jimb@zwingli.cygnus.com>
To: "Peter.Schauer" <Peter.Schauer@regent.e-technik.tu-muenchen.de>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [RFD] objfiles.h, symfile.c, mdebugread.c:  Fixes for Digital Unix.
Date: Tue, 07 Nov 2000 15:37:00 -0000
Message-id: <nplmuv8ihi.fsf@zwingli.cygnus.com>
References: <200011060732.IAA23824@reisser.regent.e-technik.tu-muenchen.de>
X-SW-Source: 2000-11/msg00077.html
Content-length: 722

It seems like we're adding a bunch of members to struct objfile, and a
bunch of macros to access them, and a bunch of code to
default_symfile_offsets to initialize them, for the benefit of
a new function `sect_index_for_sc' in mdebugread.c.

It seems like it would be cleaner to simply have some mdebug-specific
code look up the offsets and fill in a table indexed by mdebug SC
number, which sect_index_for_sc could then access directly.  That
would keep the entire change isolated in mdebugread.c, which is the
only reader that needs all that info.  Does that sound reasonable?

Peter, I think you're more familiar with the big picture regarding the
section offset handling than I am, so I'm interested in your comments.
From jtc@redback.com Tue Nov 07 16:22:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Nick Duffek <nsd@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] target.c: target_xfer_memory_partial() returns byte count
Date: Tue, 07 Nov 2000 16:22:00 -0000
Message-id: <5mhf5jwbvw.fsf@jtc.redback.com>
References: <200011070457.eA74vkf24088@rtl.cygnus.com> <5m7l6fk4gw.fsf@jtc.redback.com>
X-SW-Source: 2000-11/msg00078.html
Content-length: 625

>>>>> "jtc" == J T Conklin <jtc@redback.com> writes:
>>>>> "Nick" == Nick Duffek <nsd@redhat.com> writes:
Nick> It looks like target_xfer_memory_partial() got broken in a recent patch.
Nick> Okay to apply this fix?

jtc> My bad.  The fix looks correct.

But It's worse than I thought.

The change I committed didn't remove the code that transfer all the
memory when I cut-n-pasted to create do_xfer_memory() from the guts of
target_xfer_memory().  So at the moment, target_xfer_memory_partial()
transfers the entire region.  

I'll submit a patch to fix this botch tomorrow.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From kevinb@cygnus.com Tue Nov 07 17:34:00 2000
From: Kevin Buettner <kevinb@cygnus.com>
To: Jim Blandy <jimb@cygnus.com>
Cc: Ulrich Drepper <drepper@redhat.com>, gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH RFA] solib-svr4.c patch for dynamic executables
Date: Tue, 07 Nov 2000 17:34:00 -0000
Message-id: <1001108013427.ZM19910@ocotillo.lan>
References: <1001104012305.ZM6714@ocotillo.lan> <np1ywna3f4.fsf@zwingli.cygnus.com> <jimb@cygnus.com>
X-SW-Source: 2000-11/msg00079.html
Content-length: 7817

On Nov 7,  4:16pm, Jim Blandy wrote:

> The way this patch decides whether or not to relocate the main
> executable seems questionable to me.  It looks like we're basing our
> decision on conditions which are suggestive, but not definitive.
> 
> We should look at the ABI to see what rules `exec' follows to decide
> whether to relocate the main executable, and then make GDB use the
> same rules.

See Ulrich's remark regarding the rules.

> At the very least, the test for the ".interp" section should be
> explained.

Ideally, our test for deciding whether to relocate the main executable
or not would look like this:

   if (bfd_get_start_address (exec_bfd) != pc)
      ... /* do the relocation(s) */

That is to say, if the address at which we are stopped (which we know
is the starting location of the program) is not the same as the start
address from the ELF file, then we need to relocate the sections.

Unfortunately, this won't work because *if* there's an interpreter,
the interpreter gets executed first and will branch to the ELF file
start address after the interpreter runs.  In other words, the start
address (from the executable) isn't the true start address when
there's an interepreter.

Let's explore an alternative.  Ulrich's patch upon which mine is
based had a slightly different test:

  if ((bfd_get_file_flags (exec_bfd) & DYNAMIC) != 0
      && bfd_get_start_address (exec_bfd) != stop_pc)

I have a hunch that this code is *practically* equivalent to my
proposed patch, but I could not convince myself of this fact.  (And I
did spend quite a while trying.)  I was concerned about the case where
the condition regarding the DYNAMIC flag (which is the same as
e_type == ET_DYN) being true in addition to a .interp section being
present.  If this were to happen, I have no idea what the right thing
to do would be, but it would almost certainly not be to relocate
symfile_objfile by the displacement given by the difference between
the address at which gdb is stopped and the start address from the
executable; it seemed safer just to leave it alone.

I really do think that testing for the presence of an interpreter
is the correct way to make the decision about whether to relocate
or not.  Think of the condition as being worded this way:

    If it makes sense to compare the start addresses AND
    the start addresses are different, then ...

The "makes sense to compare the start addresses" condition is
simply the test for the interpreter section being absent.

I agree that I need to explain this better in the code though.  Below
is a new patch which adds a comment explaining the test for the
interpreter section.  I also added a comment which explains that
relocation by a single constant is in fact mandated by the System V
ABI.  I removed my former remark which said that relocating by a
single constant is naive.

I withdraw my former patch in favor of the one below.

	Changes based on a patch from Ulrich Drepper:
	* solib-svr4.c (svr4_relocate_main_executable): New function.
	(svr4_solib_create_inferior_hook):  Call
	svr4_relocate_main_executable.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.2
diff -u -p -r1.2 solib-svr4.c
--- solib-svr4.c	2000/10/30 23:31:17	1.2
+++ solib-svr4.c	2000/11/08 00:57:01
@@ -1437,6 +1437,96 @@ svr4_special_symbol_handling (void)
 #endif /* !SVR4_SHARED_LIBS */
 }
 
+/* Relocate the main executable.  This function should be called upon
+   stopping the inferior process at the entry point to the program. 
+   The entry point from BFD is compared to the PC and if they are
+   different, the main executable is relocated by the proper amount. 
+   
+   As written it will only attempt to relocate executables which
+   lack interpreter sections.  It seems likely that only dynamic
+   linker executables will get relocated, though it should work
+   properly for a position-independent static executable as well.  */
+
+static void
+svr4_relocate_main_executable (void)
+{
+  asection *interp_sect;
+  CORE_ADDR pc = read_pc ();
+
+  /* Decide if the objfile needs to be relocated.  As indicated above,
+     we will only be here when execution is stopped at the beginning
+     of the program.  Relocation is necessary if the address at which
+     we are presently stopped differs from the start address stored in
+     the executable AND there's no interpreter section.  The condition
+     regarding the interpreter section is very important because if
+     there *is* an interpreter section, execution will begin there
+     instead.  When there is an interpreter section, the start address
+     is (presumably) used by the interpreter at some point to start
+     execution of the program.
+
+     If there is an interpreter, it is normal for it to be set to an
+     arbitrary address at the outset.  The job of finding it is
+     handled in enable_break().
+
+     So, to summarize, relocations are necessary when there is no
+     interpreter section and the start address obtained from the
+     executable is different from the address at which GDB is
+     currently stopped.  */
+
+  interp_sect = bfd_get_section_by_name (exec_bfd, ".interp");
+  if (interp_sect == NULL && bfd_get_start_address (exec_bfd) != pc)
+    {
+      struct cleanup *old_chain;
+      struct section_offsets *new_offsets;
+      int i, changed;
+      CORE_ADDR displacement;
+      
+      /* It is necessary to relocate the objfile.  The amount to
+	 relocate by is simply the address at which we are stopped
+	 minus the starting address from the executable.
+
+	 We relocate all of the sections by the same amount.  This
+	 behavior is mandated by recent editions of the System V ABI. 
+	 According to the System V Application Binary Interface,
+	 Edition 4.1, page 5-5:
+
+	   ...  Though the system chooses virtual addresses for
+	   individual processes, it maintains the segments' relative
+	   positions.  Because position-independent code uses relative
+	   addressesing between segments, the difference between
+	   virtual addresses in memory must match the difference
+	   between virtual addresses in the file.  The difference
+	   between the virtual address of any segment in memory and
+	   the corresponding virtual address in the file is thus a
+	   single constant value for any one executable or shared
+	   object in a given process.  This difference is the base
+	   address.  One use of the base address is to relocate the
+	   memory image of the program during dynamic linking.
+
+	 The same language also appears in Edition 4.0 of the System V
+	 ABI and is left unspecified in some of the earlier editions.  */
+
+      displacement = pc - bfd_get_start_address (exec_bfd);
+      changed = 0;
+
+      new_offsets = xcalloc (sizeof (struct section_offsets),
+			     symfile_objfile->num_sections);
+      old_chain = make_cleanup (free, new_offsets);
+
+      for (i = 0; i < symfile_objfile->num_sections; i++)
+	{
+	  if (displacement != ANOFFSET (symfile_objfile->section_offsets, i))
+	    changed = 1;
+	  new_offsets->offsets[i] = displacement;
+	}
+
+      if (changed)
+	objfile_relocate (symfile_objfile, new_offsets);
+
+      do_cleanups (old_chain);
+    }
+}
+
 /*
 
    GLOBAL FUNCTION
@@ -1489,9 +1579,12 @@ svr4_special_symbol_handling (void)
    Also, what if child has exit()ed?  Must exit loop somehow.
  */
 
-void
+static void
 svr4_solib_create_inferior_hook (void)
 {
+  /* Relocate the main executable if necessary.  */
+  svr4_relocate_main_executable ();
+
   /* If we are using the BKPT_AT_SYMBOL code, then we don't need the base
      yet.  In fact, in the case of a SunOS4 executable being run on
      Solaris, we can't get it yet.  current_sos will get it when it needs