Re: [PATCH] solib-svr4: Pass down svr4_info as much as possible

[Simon Marchi <simark at simark dot ca>]

On 2019-04-22 9:57 a.m., Pedro Alves wrote:
> On 4/19/19 4:03 PM, Simon Marchi wrote:
>> On 2019-04-10 10:49 p.m., Simon Marchi wrote:
>>> I am not able to reproduce the problem, and the test doesn't fail here, without
>>> the rest of the patch applied.  I think it's because on my system gdb doesn't use
>>> the probe based interface.
>>>
>>> Anyhow, the fix makes sense.  Since the probe is deleted on objfile destruction, the
>>> corresponding probe_and_action structures should too.
>>>
>>> Simon
>>
>> While reviewing this patch, I had written the patch below to experiment, and
>> while it's not super important, I think it's a good cleanup.
>>
>>
>> From aedf5f7d846672ba6edc2780853baa43f35dd3c4 Mon Sep 17 00:00:00 2001
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Wed, 10 Apr 2019 22:02:33 -0400
>> Subject: [PATCH] solib-svr4: Pass down svr4_info as much as possible
>>
>> While reviewing
>>
>>   https://sourceware.org/ml/gdb-patches/2019-04/msg00141.html
>>
>> I noticed that we relied heavily on global state through the
>> get_svr4_info function, which uses current_program_space.  I thought we
>> could improve this (make things more explicit and easier to follow) by
>>
>> - Making get_svr4_info accept a program_space parameter, making it
>>   return the SVR4 info for that program space.
>> - Passing down the svr4_info object from callers as much as possible.
>>
>> This means looking up the svr4_info for the appropriate program space at
>> the entry points of the solib-svr4.c file and passing it down.  For now,
>> these entry points (most of them are "methods" of svr4_so_ops) rely on
>> current_program_space, but we can later try to change the target_so_ops
>> interface to pass down the program space.
> 
> Seems fine to me.  Please go ahead.

Pushed, thanks.

> Thinking a bit longer term we could end up passing down an inferior pointer
> around in functions in this file instead.  That's because we use target_gdbarch()
> in these routines, which is really inferior->gdbarch.  The program space can be
> found at inferior->pspace.  Etc.  Then again, the end up going target calls in
> a number of these routines, which implicitly refers to the current
> inferior/thread/pspace too...  Anyway, I'm happy with the patch as is, TBC.

Ok, thanks for the tip.

Simon