Hi Paul,
> > The message "Saw a #BR!" is a bit cryptic
> > Does someone understand this argp-help.c code?
>
> I didn't, but after looking at the code for a bit I see a problem that
> could explain the symptoms you observe. hol_append subtracts pointers
> into different arrays, which has undefined behavior in C, and
> -fcheck-pointer-bounds can catch this after the resulting offset is used
> to calculate a pointer and the pointer is then later used. This is
> clearly a portability bug so I installed the first attached patch. Does
> it fix the problem on your platform?
Yes, it fixes the problem. Great! Thanks.
> I also tested argp under -fsanitize=undefined and found a different bug,
> fixed in the 2nd attached patch.
Sorry, I don't understand this patch: it avoids the undefined behaviour
on left-shift, but still makes assumptions about the "implementation-
defined" behaviour of right-shift [1], here:
user_key = shifted_user_key >> GROUP_BITS;
[1]
http://stackoverflow.com/questions/7622/are-the-shift-operators-arithmetic-or-logical-in-c
How about this? The intended operation (in terms of bits) is:
z.......abcdefghijklmnopqrstuvwx
-> zzzzzzzzabcdefghijklmnopqrstuvwx
AFAIK, bit operations (&, |, ~) don't have undefined or implementation-defined
behaviour on signed integer types. Once we assume two's complement - which your
comment already states - they are unambiguous. Proposed patch:
2017-05-20 Bruno Haible <br...@clisp.org>
argp: Simplify bit manipulation.
* lib/argp-parse.c (parser_parse_opt): Use &, |, ~ instead of shifts
on a signed integer type.
diff --git a/lib/argp-parse.c b/lib/argp-parse.c
index 4723a2b..fb04a4a 100644
--- a/lib/argp-parse.c
+++ b/lib/argp-parse.c
@@ -743,15 +743,8 @@ parser_parse_opt (struct parser *parser, int opt, char
*val)
/* A long option. Preserve the sign in the user key, without
invoking undefined behavior. Assume two's complement. */
{
- unsigned uopt = opt;
- unsigned ushifted_user_key = uopt << GROUP_BITS;
- int shifted_user_key = ushifted_user_key;
- int user_key;
- if (-1 >> 1 == -1)
- user_key = shifted_user_key >> GROUP_BITS;
- else
- user_key = ((ushifted_user_key >> GROUP_BITS)
- - (shifted_user_key < 0 ? 1 << USER_BITS : 0));
+ int user_key =
+ ((opt & (1 << (USER_BITS - 1))) ? ~USER_MASK : 0) | (opt & USER_MASK);
err =
group_parse (&parser->groups[group_key - 1], &parser->state,
user_key, parser->opt_data.optarg);
