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

Pedro Alves
2013-08-09T15:08:26Z

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