Re: UB in derivation.c

[Edoardo Sanguineti]

Thank you for your detailed response.
Should the other 'hyper-corrections 'be addressed?
I thought there was something wrong because of the inconsistent usage of
c_isspace where the 'unnecessary' cast to unsigned char was missing on one
occasion.
If the other casts to unsigned char can be safely removed, could that be
something worth doing?

On Wed, Nov 2, 2022 at 5:54 AM Kaz Kylheku <kaz@kylheku.com> wrote:

> On 2022-11-01 14:26, Edoardo Sanguineti wrote:
> > Hello,
> > I think I found undefined behaviour in the function all_spaces in the
> file
> > derivation.c
> > I believe there is a missing cast to unsigned char before the call to the
> > function c_isspace (see here:
> > https://git.savannah.gnu.org/cgit/bison.git/tree/src/derivation.c#n166).
>
> Your remarks would be correct about the ill-designed isspace function in
> ISO C. This is c_isspace from gnulib, which looks like this:
>
>   C_CTYPE_INLINE bool
>   c_isspace (int c)
>   {
>     switch (c)
>       {
>       case ' ': case '\t': case '\n': case '\v': case '\f': case '\r':
>         return true;
>       default:
>         return false;
>       }
>   }
>
> It is not using the argument as an index into a table, and so there is no
> risk of out-of-bounds access due to the argument being negative.
>
> Also, the behavior won't change if something in the process accidentally
> calls setlocale, which is laudable.
>
> ISO C provides very few text processing functions that don't fall
> victim to localization effects.
>
> Think you can lexically scan a floating-point number in C syntax with
> strtod? Check again: if the program has switched to a locale in which
> the comma serves as the decimal point, "1.234E+15" won't be understood
> properly by strotd.
>
> > I searched all usages of c_isspace in the source code of bison and this
> is
> > the only occurrence the cast to unsigned char is missing so I think that
> > should be fixed.
>
> Rather, the other unsigned char casts may be a hyper-correction
> probably left over from previous versions of the code before gnulib.
>