Re: [PATCH] left shift of negative value do_binop src/arith_yacc.c:111:12

Aleksander Ushakov wrote in
 <[email protected]>:
 |
 |01/08/25 00:50, Harald van Dijk wrote:
 |> On 31/07/2025 19:49, Aleksander Ushakov wrote:
 |>> There is a fix of the problem:
 |>>
 |>> diff --git a/src/arith_yacc.c b/src/arith_yacc.c
 |>> index 1a087c3..e23060d 100644
 |>> --- a/src/arith_yacc.c
 |>> +++ b/src/arith_yacc.c
 |>> @@ -108,7 +108,9 @@ static intmax_t do_binop(int op, intmax_t a, 
 |>> intmax_t b)
 |>>          case ARITH_SUB:
 |>>                  return a - b;
 |>>          case ARITH_LSHIFT:
 |>> -               return a << b;
 |>> +               if (a < 0)
 |>> +                       yyerror("left shift of negative value");
 |>> +               return (intmax_t)a << b;
 |>>          case ARITH_RSHIFT:
 |>>                  return a >> b;
 |>>          case ARITH_LT:
 |> If other major shells already issued an error here, this change would 
 |> look fine (except again incomplete -- there is a lot more potential to 
 |> trigger sanitizer messages in this file), but as other major shells 
 |> accept this, I think dash should continue to accept this too.
 |> 
 |> Cheers,
 |> Harald van Dijk
 |
 |Well, if we take bash with UBSAN instrumentation and try to use a 
 |similar command with a negative number shift to the left, then at least 
 |we won't get a sanitizer error. Yes, you can remove the message that 

Really?  How does it do that if it uses intmax_t for all values
and then does

            case LSH:
              lvalue <<= value;
              break;
and

  while ((curtok == LSH) || (curtok == RSH))
    {
      int op = curtok;

      readtok ();
      val2 = expaddsub ();

      if (op == LSH)
        val1 = val1 << val2;

Having said that i could simply try it out myself, but too lazy
i am.

 |this shift is invalid, but it is still UB to perform this shift 
 |according to the C standard. I mean, a check should be added here so as 
 |not to trigger UBSAN, as it is done in bash.

Where is that test?

 --End of <[email protected]>

--steffen
|
|Der Kragenbaer,                The moon bear,
|der holt sich munter           he cheerfully and one by one
|einen nach dem anderen runter  wa.ks himself off
|(By Robert Gernhardt)