[RFC] When can we remove Sun-specific stabs support? (when will it be ok to delete partial_symtab.section_offsets?)

[RFC] When can we remove Sun-specific stabs support? (when will it be ok to delete partial_symtab.section_offsets?)

[Doug Evans]

Hi.

While working on improving some symbol table code I stumbled on
partial_symtab.section_offsets and that led me to elfstab_offset_sections.

AFAICT, the only reason partial_symtab.section_offsets exists is for
Sun stabs.  There are no regressions if I comment out
elfstab_offset_sections on linux+stabs.
Grep for Ddata.data in doc/stabs.texinfo for further background.

elfread.c:

/* When handling an ELF file that contains Sun STABS debug info,
    some of the debug info is relative to the particular chunk of the
    section that was generated in its individual .o file.  E.g.
    offsets to static variables are relative to the start of the data
    segment *for that module before linking*.  This information is
    painfully squirreled away in the ELF symbol table as local symbols
    with wierd names.  Go get 'em when needed.  */

dbxread.c:

#ifdef HAVE_ELF
   /* If we're handling an ELF file, drag some section-relocation info
      for this source file out of the ELF symbol table, to compensate for
      Sun brain death.  This replaces the section_offsets in this psymtab,
      if successful.  */
   elfstab_offset_sections (objfile, result);
#endif

N.B. I'm not suggesting remove stabs support, just this hack for Sun stabs.

Deleting partial_symtab.section_offsets will simplify some code
(for non-sun-stabs it's just a pointer to objfile->section_offsets).
It'll also help the ObjfileSplitting project.
https://sourceware.org/gdb/wiki/ObjfileSplitting

Re: [RFC] When can we remove Sun-specific stabs support? (when will it be ok to delete partial_symtab.section_offsets?)

[Doug Evans]

On Thu, May 7, 2015 at 2:21 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> While working on improving some symbol table code I stumbled on
> partial_symtab.section_offsets and that led me to elfstab_offset_sections.
>
> AFAICT, the only reason partial_symtab.section_offsets exists is for
> Sun stabs.  There are no regressions if I comment out
> elfstab_offset_sections on linux+stabs.
> Grep for Ddata.data in doc/stabs.texinfo for further background.
>
> elfread.c:
>
> /* When handling an ELF file that contains Sun STABS debug info,
>    some of the debug info is relative to the particular chunk of the
>    section that was generated in its individual .o file.  E.g.
>    offsets to static variables are relative to the start of the data
>    segment *for that module before linking*.  This information is
>    painfully squirreled away in the ELF symbol table as local symbols
>    with wierd names.  Go get 'em when needed.  */
>
> dbxread.c:
>
> #ifdef HAVE_ELF
>   /* If we're handling an ELF file, drag some section-relocation info
>      for this source file out of the ELF symbol table, to compensate for
>      Sun brain death.  This replaces the section_offsets in this psymtab,
>      if successful.  */
>   elfstab_offset_sections (objfile, result);
> #endif
>
> N.B. I'm not suggesting remove stabs support, just this hack for Sun stabs.
>
> Deleting partial_symtab.section_offsets will simplify some code
> (for non-sun-stabs it's just a pointer to objfile->section_offsets).
> It'll also help the ObjfileSplitting project.
> https://sourceware.org/gdb/wiki/ObjfileSplitting

Heh.  Gotta love single stepping through code to see what's REALLY going on.

I found this testcase in gas and hacked it to compile on x86.
gas/testsuite/gas/sparc-solaris/sol-cc.s

I then walked through gdb to verify it was properly handling
this hack for Sun stabs.
But when I get to elfstab_offset_sections, sectinfo is NULL
so there is nothing to do. From the testcase clearly that's wrong.

  struct stab_section_info *maybe = dbx->stab_section_info;

(top-gdb) p maybe
$17 = (struct stab_section_info *) 0x0

It turns out the data is being freed
before elfstab_offset_sections is called!
elf_read_minimal_symbols has this:

  make_cleanup (free_elfinfo, (void *) objfile);

and free_elfinfo is this:

static void
free_elfinfo (void *objp)
{
  struct objfile *objfile = (struct objfile *) objp;
  struct dbx_symfile_info *dbxinfo = DBX_SYMFILE_INFO (objfile);
  struct stab_section_info *ssi, *nssi;

  ssi = dbxinfo->stab_section_info;
  while (ssi)
    {
      nssi = ssi->next;
      xfree (ssi);
      ssi = nssi;
    }

  dbxinfo->stab_section_info = 0;       /* Just say No mo info about this.  */
}

So this hack is currently a no-op: we collect the needed info here:

elfread.c:
                  if (strcmp ("Bbss.bss", sym->name) == 0)
                    special_local_sect = SECT_OFF_BSS (objfile);
                  else if (strcmp ("Ddata.data", sym->name) == 0)
                    special_local_sect = SECT_OFF_DATA (objfile);
                  else if (strcmp ("Drodata.rodata", sym->name) == 0)
                    special_local_sect = SECT_OFF_RODATA (objfile);

and then throw it away (in free_elfinfo) before we use it
(in elfstab_offset_sections).

IIUC, and assuming I'm not missing anything,
this has been broken since at least gdb 7.0 (I didn't
check back any further).
The code was a little different back then, but the free_elfinfo
cleanup was still done before building psymtabs.

Sufficient motivation for deleting this code now?

Re: [RFC] When can we remove Sun-specific stabs support? (when will it be ok to delete partial_symtab.section_offsets?)

[Doug Evans]

On Thu, May 7, 2015 at 3:52 PM, Doug Evans <dje@google.com> wrote:
> ...
> IIUC, and assuming I'm not missing anything,
> this has been broken since at least gdb 7.0 (I didn't
> check back any further).
> The code was a little different back then, but the free_elfinfo
> cleanup was still done before building psymtabs.
>
> Sufficient motivation for deleting this code now?

I'm guessing this is the patch, from 2003.

2003-02-20  Daniel Jacobowitz  <drow@mvista.com>

        * coffread.c (coff_symfile_read): Clean up minimal symbols earlier.
        * dbxread.c (elfstab_build_psymtabs): Don't call
        install_minimal_symbols.
        (stabsect_build_psymtabs): Likewise.
        * elfread.c (elf_symfile_read): Call install_minimal_symbols
        earlier.
        * somread.c (som_symfile_read): Call install_minimal_symbols
        and do_cleanups earlier.
        * nlmread.c (nlm_symfile_read): Likewise.
        * mdebugread.c (elfmdebug_build_psymtabs): Call
        install_minimal_symbols and make appropriate cleanups.

[there's a typo in the date of the c/l entry, which I fixed after cut-n-pasting]

So, assuming I'm not missing anything, we haven't supported
this hack for Sun stabs since 2003.

Patch to delete it completely to follow,
unless there's a REALLY good reason to keep it.

Re: [RFC] When can we remove Sun-specific stabs support? (when will it be ok to delete partial_symtab.section_offsets?)

[Pedro Alves]

On 05/08/2015 12:04 AM, Doug Evans wrote:

> So, assuming I'm not missing anything, we haven't supported
> this hack for Sun stabs since 2003.
> 
> Patch to delete it completely to follow,
> unless there's a REALLY good reason to keep it.

I don't know of a good reason to keep it.
Given the results of your investigation, I'd say go ahead.

Thanks,
Pedro Alves