Re: [PATCH v3 11/19] elf: Do not duplicate the GLIBC_TUNABLES string

[Siddhesh Poyarekar <siddhesh@sourceware.org>]

On 2023-11-21 13:12, Adhemerval Zanella Netto wrote:
>>> -tunable_initialize (tunable_t *cur, const char *strval)
>>> +tunable_initialize (tunable_t *cur, const char *strval, size_t len)
>>>    {
>>> -  tunable_val_t val;
>>> +  tunable_val_t val = { 0 };
>>>        if (cur->type.type_code != TUNABLE_TYPE_STRING)
>>>        val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
>>
>> There's an implicit assumption that strval is NULL terminated for numeric values.  Is that safe?  Maybe all you need to do here is copy strval into a static buffer of size 21 (basically size of the string representing -1ULL) and pass that to _dl_strtoul.
> 
> Afaiu the environment variable will always be NULL terminated (although some
> might overlap).  This is due how the kernel will layout the argv/envp on
> process creation at sys_execve:

Sure, but the strval here should not be the entire envvar, just the 
value portion of it.  Granted that _dl_strtoul will bail out on the 
first non-numeric character, so maybe it's not

>>> -      if (disable)
>>> +      if (n.disable)
>>>            {
>>>              CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, POPCNT, 6);
>>>              CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_1, 6);
>>>              CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_2, 6);
>>> -          if (memcmp (n, "XSAVEC", 6) == 0)
>>> +          if (memcmp (n.str, "XSAVEC", 6) == 0)
>>
>> Why does this use the bare memcmp and not the tunables_strcmp?
> 
> If I recall correctly, I did tried it and it resulted in worse codegen.  The
> tunable_str_comma_strcmp also checks the size, which on the x86 code is not
> required and for some reason compiler can not eliminate.

Why isn't it required? Couldn't n.str be smaller than 6 chars?

Thanks,
Sid