Re: GDB 5.1/Core files and ptids

[Kevin Buettner <kevinb at redhat dot com>]

On Jan 16, 11:16am, Takis Psarogiannakopoulos wrote:

> Trying to port DG/UX source files from gdb 5.0 to version
> 5.1 (and hopping to submit these at last to GNU) I have found
> that there is a serious inconsistency for the BFD core files.
> 
> Before one could overload a process pid with a thread id  
> using the macro:
> 
> #define MERGEPID(PID, TID) (((PID) & 0xffff) | ((TID) << 16))
> 
> Note that this will give us again an integer!
> In 5.1 someone change this to a stucture called ptid.

Yes.  That was me.

> However
> it seems to me that you forgot to implemnet something similar
> to BFD.

Okay...

> Problem:
> 
> Suppose that one has a line co code of the type
> 
> sprintf (section_name, "%s/%d", name, inferior_pid);
> 
> OR of the type
> 
> struct thread_info *tp;
> ...
> sprintf (section_name, "%s/%d", name, tp->pid);
> 
> inside  the gdb/core-dgux.c file. Note that this integer 
> (tp->pid or inferior_pid) it should contains/be overloaded 
> with  the tid too!
> 
> Clearly neven if in the 5.1 we have a tp->ptid one cannot 
> write 
> 
> sprintf (section_name, "%s/%d", name, tp->ptid);
> 
> because ptid is now a structure!

The context here is that we're trying to fetch the appropriate
.reg (.reg2, etc) section from bfd, right?

> Especially when, even in the new gdb-5.1/bfd we find:
> 
> ======
> static int
> elfcore_make_pid (abfd)
>      bfd *abfd;
> {
>   return ((elf_tdata (abfd)->core_lwpid << 16)
>           + (elf_tdata (abfd)->core_pid));
> }
> =======

This will need to change.  It is incorrect to attempt to represent
both the pid and lwp as a single integer whose size isn't large
enough to hold all of the bits.

> Any suggestions? Eg can the guy that introduced these new
> ptids how specicfically to rewrite the line:
> 
> sprintf (section_name, "%s/%d", name, tp->pid);
> 
> (pid is from 5.0 ie. a mixed pid but still integer!)
> 
> having given the tp->ptid and taking in account the 
> elfcore_make_pid that is used by bfd!!!
> 
> We want to rewrite the bit above so that gdb-5.1 will 
> understand! that this section has info about the pid= 
> PIDGET(tp->ptid) and the lwp=TIDGET(tp->ptid). And 
> reflect these when asked.

Here are my suggestions:

 1) In bfd, the parts requiring elfcore_make_pid() are all contained in
    elf.c.  I suggest that you rewrite elfcore_make_pid() to look something
    like this:

    static char *
    elfcore_make_ptid_str (abfd)
         bfd *abfd;
    {
      static char ptid_buf[40];
      
      if (elf_tdata (abfd)->core_lwpid == 0)
        {
          /* Non-threaded */
          sprintf (ptid_buf, "%d", elf_tdata (abfd)->core_pid);
        }
       else
        {
          /* Threaded */
          sprintf (ptid_buf, "%d+%d", elf_tdata (abfd->core_pid),
                                      elf_tdata (abfd->core_lwpid));
        }
      return ptid_buf;
    }

 2) Revise all callers in elf.c to use the above function.  E.g, in
    _bfd_elfcore_make_pseudosection(), the line:

      sprintf (buf, "%s/%d", name, elfcore_make_pid (abfd));

    will become

      sprintf (buf, "%s/%s", name, elfcore_make_ptid_str (abfd));

    So, to fetch a .reg section in a multithreaded core dump where
    the pid is 14 and the lwp is 42, GDB would need to ask BFD for
    the .reg/14+42 (pseudo) section.

 3) On the GDB side, specifically in corelow.c, as well as in the
    DG/UX specific corefile reader that you're working on, change
    the code which wants to fetch the bfd core section to understand
    these new section names.  E.g, in get_core_register_section()
    in corelow.c, change:

      if (PIDGET (inferior_ptid))
	sprintf (section_name, "%s/%d", name, PIDGET (inferior_ptid));
      else
	strcpy (section_name, name);

    to:

      else if (TIDGET (inferior_ptid))
	sprintf (section_name, "%s/%d+%d", name, PIDGET (inferior_ptid),
	         TIDGET (inferior_ptid));
      else if (PIDGET (inferior_ptid))
	sprintf (section_name, "%s/%d", name, PIDGET (inferior_ptid));
      else
	strcpy (section_name, name);

We ought to run this past the binutils folks too to make sure it's
okay with them.  (I've cc'd them on this correspondence.)

Kevin