Re: [PATCH 0/3] Fix issues with writing Linux core PRSTATUS note on MIPS o32, n32 and n64 into core file

["Maciej W. Rozycki" <macro@mips.com>]

On Wed, 8 Nov 2017, Pedro Alves wrote:

> >  So I have looked into it now and tracked down `libthread_db' rather 
> > than GDB to be the component requiring PID retrieval from a core file 
> > for TLS access to work, and then only before glibc commit c579f48edba8 
> > ("Remove cached PID/TID in clone"), which was first included in 2.25 
> > glibc release.
> > 
> >  Given I have been using recent glibc checkouts for MIPS verification I 
> > avoided the requirement, but I was indeed able to reproduce it natively 
> > with x86-64 and the system-supplied `libthread_db', and then with my 
> > MIPS environment as well, once I went with my glibc build back to commit 
> > c579f48edba8^.
> > 
> >  I will be adding a reference to said glibc commit when pushing your 2/3 
> > change.
> 
> Interesting, hadn't realized libthread_db stopped requiring this.
> I wonder whether it'd be a good idea to mention it in the code itself 
> too, say, in proc-service.:ps_getpid.

 This might not be the best place.  Calls to `ps_getpid' are still made 
from places throughout `libthread_db', and it's only one from 
`td_ta_thr_iter' (which matters here) whose result is discarded, by only 
passing it down to `iterate_thread_list' to become its ignored 
`match_pid' argument.  This looks like an oversight of some kind to me, 
which Adhemerval might be able to shed some light on.

 Adhemerval, in your commit referred above, corresponding to the change 
held at: <https://sourceware.org/ml/libc-alpha/2016-11/msg00282.html>, 
you made the `match_pid' argument of `iterate_thread_list' unused.  The 
function is static, called from `td_ta_thr_iter' only (and I actually 
wonder why the build does not trip on an unused argument here).

 Is it OK then to remove the argument then along with the `ps_getpid' 
call in `td_ta_thr_iter' used to set the corresponding parameter, or is 
there something missing from there you intended to do and `match_pid' 
was meant to remain used?

 NB I would post a clean-up proposal, but regrettably my company FSF 
copyright assignment status is in a limbo state right now, after the 
transition from Imagination to the new MIPS company, and I'd rather not 
block the cleanup by posting a patch that cannot move forward.  Or would 
it be small enough to qualify as not needing an assignment?  Hmm...

 As to choosing the right place to comment on this `libthread_db' 
semantics change, I think either the call to `bfd_core_file_pid' made 
from `add_to_thread_list' in corelow.c or the heading of the latter 
function might be better, as this is where the consumer of BFD's core 
file data PID is.  Adding a comment like this might be small/trivial 
enough for me to sneak in while the FSF paperwork is still pending.

  Maciej