Re: [RFA 2/5] Darwin: Handle unrelocated dyld.

[Xavier Roirand <roirand@adacore.com>]

Hello,

Thanks Tom and Simon for all the work you've done on this patch. I was 
quite busy last week and did not follow this thread.

Le 9/19/18 à 9:15 PM, Tom Tromey a écrit :
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> I would vote for only checking in the code you know is necessary for
> Simon> now, otherwise it will just be more confusing in the future, trying to
> Simon> figure out what is needed and what isn't.
> 
> Here is a more minimal version of the patch.  This one seems to work for
> me on High Sierra.  I tried running a "hello world" program -- this
> previously failed, but now works.  It's good enough that I could run
> gdb.cp/*.exp -- lots of fails but no crashes or mystery problems.
> 
> Tom
> 
> commit 114a1aae792443d72f1438dbc979b42a39c5b780
> Author: Xavier Roirand <roirand@adacore.com>
> Date:   Wed Aug 22 12:11:14 2018 +0200
> 
>      Darwin: Handle unrelocated dyld.
>      
>      On Darwin, debugging an helloworld program with GDB does
>      not work and ends with:
>      
>        (gdb) set startup-with-shell off
>        (gdb) start
>        Temporary breakpoint 1 at 0x100000fb4: file /tmp/helloworld.c, line 1.
>        Starting program: /private/tmp/helloworld
>        [New Thread 0x2703 of process 18906]
>        [New Thread 0x2603 of process 18906]
>      
>        [1]+  Stopped                 ./gdb/gdb /tmp/helloworld
>      
>      When debugging with lldb, instead of having the STOP signal, we can
>      see that a breakpoint is not set to a proper location:
>      
>        Warning:
>        Cannot insert breakpoint -1.
>        Cannot access memory at address 0xf726
>      
>        Command aborted.
>      
>      The inserted breakpoint is the one used when GDB has to stop the target
>      when a shared library is loaded or unloaded. The notifier address used
>      for adding the breakpoint is wrong thus the above failure.
>      This notifier address is an offset relative to dyld base address, so
>      the value calculation has to be updated to reflect this.
>      
>      This was tested on High Sierra by trying to run a simple "hello world"
>      program.
>      
>      gdb/ChangeLog:
>      
>              * solib-darwin.c (darwin_get_dyld_bfd): New function.
>              (darwin_solib_get_all_image_info_addr_at_init): Update call.
>              (darwin_handle_solib_event): New function.
>              (darwin_solib_create_inferior_hook): Handle unrelocated dyld.
>      
>      Change-Id: I7dde5008c9158f17b78dc89bd7f4bd8a12d4a6e1
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 328d48eeeb9..804aaf78e91 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-09-18  Xavier Roirand  <roirand@adacore.com>
> +
> +          * solib-darwin.c (darwin_get_dyld_bfd): New function.
> +          (darwin_solib_get_all_image_info_addr_at_init): Update call.
> +          (darwin_solib_create_inferior_hook): Handle unrelocated dyld.
> +
>   2018-09-18  Tom Tromey  <tom@tromey.com>
>   
>   	* compile/compile-object-load.c (struct
> diff --git a/gdb/solib-darwin.c b/gdb/solib-darwin.c
> index ed8e0c13365..1877ec0839d 100644
> --- a/gdb/solib-darwin.c
> +++ b/gdb/solib-darwin.c
> @@ -429,23 +429,21 @@ gdb_bfd_mach_o_fat_extract (bfd *abfd, bfd_format format,
>     return gdb_bfd_ref_ptr (result);
>   }
>   
> -/* Extract dyld_all_image_addr when the process was just created, assuming the
> -   current PC is at the entry of the dynamic linker.  */
> +/* Return the BFD for the program interpreter.  */
>   
> -static void
> -darwin_solib_get_all_image_info_addr_at_init (struct darwin_info *info)
> +static gdb_bfd_ref_ptr
> +darwin_get_dyld_bfd ()
>   {
>     char *interp_name;
> -  CORE_ADDR load_addr = 0;
>   
>     /* This method doesn't work with an attached process.  */
>     if (current_inferior ()->attach_flag)
> -    return;
> +    return NULL;
>   
>     /* Find the program interpreter.  */
>     interp_name = find_program_interpreter ();
>     if (!interp_name)
> -    return;
> +    return NULL;
>   
>     /* Create a bfd for the interpreter.  */
>     gdb_bfd_ref_ptr dyld_bfd (gdb_bfd_open (interp_name, gnutarget, -1));
> @@ -459,6 +457,18 @@ darwin_solib_get_all_image_info_addr_at_init (struct darwin_info *info)
>         else
>   	dyld_bfd.release ();
>       }
> +  return dyld_bfd;
> +}
> +
> +/* Extract dyld_all_image_addr when the process was just created, assuming the
> +   current PC is at the entry of the dynamic linker.  */
> +
> +static void
> +darwin_solib_get_all_image_info_addr_at_init (struct darwin_info *info)
> +{
> +  CORE_ADDR load_addr = 0;
> +  gdb_bfd_ref_ptr dyld_bfd (darwin_get_dyld_bfd ());
> +
>     if (dyld_bfd == NULL)
>       return;
>   
> @@ -528,10 +538,6 @@ darwin_solib_create_inferior_hook (int from_tty)
>         return;
>       }
>   
> -  /* Add the breakpoint which is hit by dyld when the list of solib is
> -     modified.  */
> -  create_solib_event_breakpoint (target_gdbarch (), info->all_image.notifier);
> -
>     if (info->all_image.count != 0)
>       {
>         /* Possible relocate the main executable (PIE).  */
> @@ -558,6 +564,49 @@ darwin_solib_create_inferior_hook (int from_tty)
>         if (vmaddr != load_addr)
>   	objfile_rebase (symfile_objfile, load_addr - vmaddr);
>       }
> +
> +  /* Set solib notifier (to reload list of shared libraries).  */
> +  CORE_ADDR notifier = info->all_image.notifier;
> +
> +  if (info->all_image.count == 0)
> +    {
> +      /* Dyld hasn't yet relocated itself, so the notifier address may
> +	 be incorrect (as it has to be relocated).  */
> +      CORE_ADDR start = bfd_get_start_address (exec_bfd);
> +      if (start == 0)
> +	notifier = 0;
> +      else
> +        {
> +          gdb_bfd_ref_ptr dyld_bfd (darwin_get_dyld_bfd ());
> +          if (dyld_bfd != NULL)
> +            {
> +              CORE_ADDR dyld_bfd_start_address;
> +              CORE_ADDR dyld_relocated_base_address;
> +              CORE_ADDR pc;
> +
> +              dyld_bfd_start_address = bfd_get_start_address (dyld_bfd.get());
> +
> +              /* We find the dynamic linker's base address by examining
> +                 the current pc (which should point at the entry point
> +                 for the dynamic linker) and subtracting the offset of
> +                 the entry point.  */
> +
> +              pc = regcache_read_pc (get_current_regcache ());
> +              dyld_relocated_base_address = pc - dyld_bfd_start_address;
> +
> +              /* We get the proper notifier relocated address by
> +                 adding the dyld relocated base address to the current
> +                 notifier offset value.  */
> +
> +              notifier += dyld_relocated_base_address;
> +            }
> +        }
> +    }
> +
> +  /* Add the breakpoint which is hit by dyld when the list of solib is
> +     modified.  */
> +  if (notifier != 0)
> +    create_solib_event_breakpoint (target_gdbarch (), notifier);
>   }
>   
>   static void
>