Re: [PATCH v5 08/20] linux-user: Load vdso image if available

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 30/8/23 18:17, Richard Henderson wrote:
> On 8/30/23 07:22, Philippe Mathieu-Daudé wrote:
>> Hi Richard,
>>
>> On 30/8/23 00:02, Richard Henderson wrote:
>>> The vdso image will be pre-processed into a C data array, with
>>> a simple list of relocations to perform, and identifying the
>>> location of signal trampolines.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   linux-user/elfload.c | 87 +++++++++++++++++++++++++++++++++++++++-----
>>>   1 file changed, 78 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>>> index f34fb64c0c..2a6adebb4a 100644
>>> --- a/linux-user/elfload.c
>>> +++ b/linux-user/elfload.c
>>> @@ -33,6 +33,19 @@
>>>   #undef ELF_ARCH
>>>   #endif
>>> +#ifndef TARGET_ARCH_HAS_SIGTRAMP_PAGE
>>> +#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 0
>>
>> I'd rather #error here so new targets are added with
>> a clearly defined TARGET_ARCH_HAS_SIGTRAMP_PAGE.
> 
> The next step after adding vdso's is to remove 
> TARGET_ARCH_HAS_SIGTRAMP_PAGE.

Ah great.

>>> +    for (unsigned i = 0, n = vdso->reloc_count; i < n; i++) {
>>
>> Do we really need 'n'?
> 
> You should always have an loop invariant condition if possible.

vdso->reloc_count doesn't seem updated, but I get your point.