Re: [PATCH v1 07/10] gdb/linux-tdep: migrate linux_find_memory_regions_full to file_reader_t

[Matthieu Longo <matthieu.longo@arm.com>]

On 09/07/2026 07:36, Thiago Jung Bauermann wrote:
> Matthieu Longo <matthieu.longo@arm.com> writes:
> 
>> The previous implementation of linux_find_memory_regions_full could
>> still return success even when none of the /proc/PID/[s]maps files
>> existed, or all reads returned 0 bytes (this last case can happen on
>> Linux when the thread-group leader has exited).
>> As a result, the function could incorrectly succeed, allowing core
>> dump generation via the gcore command.
>> This logical defect was allowing, by chance, the function to return
>> success and hence, allowing fortuitously the codedump generation via
>> gcore command (see gcore-stale-thread test for more details).
> 
> Did you run into this problem, or just noticed it by inspecting the
> code? If the former, do you think it's worth adding a testcase to make
> sure this problem doesn't appear again?
> 

The command stopped working after I migrated the function to file_reader_t.
As Christina Schimpe mentioned it, bool operator () now returns false if the buffer is null, or if
the size returned is 0. This last is a change with the previous behavior.
When the thread-group leader exits, the /proc/pid/[s]maps files still exist, but become empty.
In such a case, the function should return false instead of trying to parse an empty string, which
results in no smaps data entry being processed, and so no callback called. This behavior is
erroneous in my opinion.

I don't think that it is worth adding a testcase for this, because the root cause was the wrong pid
being used.
>> A previous patch in this patch series fixed this issue by using the
>> first LWP ID of the current inferior instead of relying on the PID.
>> This patch adapts the code to use file_reader_t and makes the function
>> return an error is reading any of the procfs files fails.
> 
> Typo: is → if
> 

Fixed.

>> ---
>>  gdb/linux-tdep.c | 23 ++++++++++-------------
>>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 

Matthieu