[PATCH 10/11] argp: Fix shift bug

[Adhemerval Zanella Netto]

On 07/05/25 16:42, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> >From gnulib commits 06094e390b0 and 88033d3779362a.
>> ---
>>  argp/argp-parse.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/argp/argp-parse.c b/argp/argp-parse.c
>> index 82c7b784de..99f8d9ecd4 100644
>> --- a/argp/argp-parse.c
>> +++ b/argp/argp-parse.c
>> @@ -735,12 +735,15 @@ parser_parse_opt (struct parser *parser, int opt, char *val)
>>  	    }
>>      }
>>    else
>> -    /* A long option.  We use shifts instead of masking for extracting
>> -       the user value in order to preserve the sign.  */
>> -    err =
>> -      group_parse (&parser->groups[group_key - 1], &parser->state,
>> -		   (opt << GROUP_BITS) >> GROUP_BITS,
>> -		   parser->opt_data.optarg);
>> +    /* A long option.  Preserve the sign in the user key, without
>> +       invoking undefined behavior.  Assume two's complement.  */
>> +    {
>> +      int user_key =
>> +        ((opt & (1 << (USER_BITS - 1))) ? ~USER_MASK : 0) | (opt & USER_MASK);
> 
> Would this be clearer?
> 
> 		   (int) ((unsigned int) opt << GROUP_BITS) >> GROUP_BITS,

This would be another gnulib deviation, and the code comes from gnulib.
I think we should keep it simple to make the sync more straightforward.

> 
> Or does ubsan flag that as well?  Conversion to negative int is a GCC
> extension:
> 
> | For conversion to a type of width N, the value is reduced modulo 2^N
> | to be within range of the type; no signal is raised.

Yeah, but -fsanitize=undefinied still triggers this as UB.  I can add an
option to suppress this kind of shift, but this will add a bit more
complexity on the handler handling.

> 
> <https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Integers-implementation.html>