Re: [PATCH, gdbserver] Further cleanup of FDPIC/DSBT divergences

[Pedro Alves <palves@redhat.com>]

On 08/08/2013 05:41 PM, Luis Machado wrote:

> Here is an updated patch that deals with solib-dsbt.c as well. The
> additional tic6x-specific fields from the loadmap were removed and
> dsbt_index is now fetched from the shared library file on disk instead
> of grabbing such information from the runtime loadmap. That information
> does not change anyway.

Is there no DT_DEBUG in auvx in FDPIC/DSBT we could consult first?

>
> +/* Scan for DYNTAG in .dynamic section of ABFD. If DYNTAG is found 1

Double space after period.

> +   is returned and the corresponding PTR is set.  We only search in
> +   the BFD, not in the target's memory.  */
> +
> +static int
> +scan_dyntag_in_bfd (int dyntag, bfd *abfd, CORE_ADDR *ptr)
> +{
> +  int step, sect_size;
> +  long dyn_tag;
> +  CORE_ADDR dyn_ptr, dyn_addr;
> +  gdb_byte *bufend, *bufstart, *buf;
> +  Elf64_External_Dyn *x_dynp_64;
> +  struct bfd_section *sect;
> +  struct target_section *target_section;
> +
> +  if (abfd == NULL)
> +    return 0;
> +
> +  if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
> +    return 0;
> +
> +  /* Make sure the size is sane.  */
> +  if (bfd_get_arch_size (abfd) == -1)
> +    return 0;
> +
> +  /* Find the start address of the .dynamic section.  */
> +  sect = bfd_get_section_by_name (abfd, ".dynamic");
> +  if (sect == NULL)
> +    return 0;
> +
> +  for (target_section = current_target_sections->sections;
> +       target_section < current_target_sections->sections_end;
> +       target_section++)
> +    if (sect == target_section->the_bfd_section)
> +      break;
> +  if (target_section < current_target_sections->sections_end)
> +    dyn_addr = target_section->addr;
> +  else
> +    {
> +      /* ABFD may come from OBJFILE acting only as a symbol file
> +	 without being loaded into the target
> +	 (see add_symbol_file_command).  This case is such fallback
> +	 to the file VMA address without the possibility of having
> +	 the section relocated to its actual in-memory address.  */
> +
> +      dyn_addr = bfd_section_vma (abfd, sect);
> +    }
> +
> +  /* Read in .dynamic from the BFD.  */
> +  sect_size = bfd_section_size (abfd, sect);
> +  buf = bufstart = alloca (sect_size);
> +  if (!bfd_get_section_contents (abfd, sect,
> +				 buf, 0, sect_size))
> +    return 0;
> +
> +  /* Iterate over BUF and scan for DYNTAG.  If found, set PTR and
> +     return.  */
> +  step = sizeof (Elf64_External_Dyn);
> +
> +  for (bufend = buf + sect_size;
> +       buf < bufend;
> +       buf += step)
> +  {
> +    x_dynp_64 = (Elf64_External_Dyn *) buf;
> +    dyn_tag = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_tag);
> +    dyn_ptr = bfd_h_get_64 (abfd, (bfd_byte *) x_dynp_64->d_un.d_ptr);
> +
> +    if (dyn_tag == DT_NULL)
> +      return 0;
> +    if (dyn_tag == dyntag)
> +      {
> +	if (ptr)

	if (ptr != NULL)

> +	  {
> +	    /* We don't want to read this information from memory.
> +	       If a relocation is needed, it should be done
> +	       elsewhere.  */
> +	    *ptr = dyn_ptr;
> +	  }
> +	return 1;
> +      }
> +  }
> +
> +  return 0;
> +}
> +

Would solib-svr4.c's scan_dyntag work pristine here?
This duplication is making me like Maciej's idea of a
shared-between-elf-ish-solib-backends solib-elf.c file more.

> +/* Given a shared library filename, load it up and find
> +   out what is its dsbt index.  */
> +
> +static int
> +fetch_solib_dsbt_index (char *filename)

could be const?

> +      target_read_string (addr, &name_buf, SO_NAME_MAX_PATH_SIZE - 1,
> +			  &errcode);
> +
> +      if (errcode != 0)
> +	dsbt_index = 0;

Is this an error path?  Should it still have the warning removed below?

> +      else
>  	{
> -	  warning (_("dsbt_current_sos: Unable to read dsbt index."
> -		     "  Shared object chain may be incomplete."));
> -	  break;
> +	  /* Fetch the DSBT index of this load module.  */
> +	  dsbt_index = fetch_solib_dsbt_index (name_buf);
>  	}
> -      dsbt_index = extract_unsigned_integer (indexword, sizeof indexword,
> -					     byte_order);
>

the bit where that above was moved from:

>  	  sop->lm_info->map = loadmap;
> -	  /* Fetch the name.  */
> -	  addr = extract_unsigned_integer (lm_buf.l_name,
> -					   sizeof (lm_buf.l_name),
> -					   byte_order);
> -	  target_read_string (addr, &name_buf, SO_NAME_MAX_PATH_SIZE - 1,
> -			      &errcode);
>
>  	  if (errcode != 0)
>  	    warning (_("Can't read pathname for link map entry: %s."),

had the warning, but it isn't reachable, due to

  if (dsbt_index != 0)

above.

Otherwise this is fine with me, if fine with Yao.

-- 
Pedro Alves