Re: UBSAN error in lib/sh/random.c:79

2023-01-10 Thread Chet Ramey

On 1/6/23 8:37 PM, Sam James wrote:

Hi folks,

I'm currently testing common Linux userland with UndefinedBehaviorSanitizer 
(UBSAN, -fsanitize=undefined).

With Bash 5.2_p15, I get the following with this script:
```
$ cat /tmp/guess_suffix
guess_suffix() {
 tmpdir="${TMPDIR}"/.ecompress$$.${RANDOM}
}
guess_suffix
```

It seems easier to trigger if I run it as an external script rather than as a 
function in an interactive
shell.

```
$ export UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1"
$ bash -x /tmp/guess_suffix
+ guess_suffix
random.c:79:21: runtime error: signed integer overflow: 31789 * 127773 cannot 
be represented in type 'int'


You must be extremely unlucky. I ran a script that generated 15 million
random numbers and got three integer overflows. The easiest fix is to use
the other variant algorithm in the paper and change the line in question to

l = ret % 127773;

That is equivalent mathemetically and won't overflow (the overflow isn't
actually damaging, though).

--
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/




Re: UBSAN error in lib/sh/random.c:79

2023-01-10 Thread Chet Ramey

On 1/7/23 1:45 PM, Martin Schulte wrote:

Hello!

Am Sat, 07 Jan 2023 19:08:06 +0100 schrieb Andreas Schwab 
:


On Jan 07 2023, Greg Wooledge wrote:
...
I think the original overflow can only happen if the argument of
intrand32 is bigger than INT_MAX.


Question might be if an overflow does any harm - or maybe even is intended...


Positive-to-negative overflow doesn't hurt the algorithm, but since
overflow is undefined behavior, anything can happen.

--
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/




Re: UBSAN error in lib/sh/random.c:79

2023-01-10 Thread Greg Wooledge
On Sat, Jan 07, 2023 at 01:42:20PM -0500, Greg Wooledge wrote:
> Or should the code do the multiplications with unsigned
> values, store them in unsigned variables, and then replace the subtraction
> with some kind of conditional that checks which of the two is greater?

Here's a version that does just that:

static u_bits32_t
intrand32 (last)
 u_bits32_t last;
{
  u_bits32_t h, l, t1, t2, ret;

  ret = (last == 0) ? 123459876 : last;
  h = ret / 127773;
  l = ret - (127773 * h);
  t1 = 16807 * l;
  t2 = 2836 * h;
  if (t1 < t2)
ret = 0x7fff - t2 + t1;
  else
ret = t1 - t2;

  return ret;
}

It passes the implementation test in the paper (checking the seed
after 1 iterations), and I believe it's free from any kind of
overflow.

Do with it as you like.  Feel free to replace the if/else with a
ternary ?: operator if that's your preference.



Re: UBSAN error in lib/sh/random.c:79

2023-01-10 Thread Chet Ramey

On 1/10/23 11:43 AM, Greg Wooledge wrote:

On Sat, Jan 07, 2023 at 01:42:20PM -0500, Greg Wooledge wrote:

Or should the code do the multiplications with unsigned
values, store them in unsigned variables, and then replace the subtraction
with some kind of conditional that checks which of the two is greater?


Here's a version that does just that:


Thanks, I pushed what I think is a simpler fix that should work fine.

Chet

--
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/




Re: Nested expansion in heredoc fails

2023-01-10 Thread Norbert Lange
On Wed, 14 Dec 2022, 16:09 Chet Ramey,  wrote:

> On 12/13/22 12:42 PM, Norbert Lange wrote:
>
> > Bash Version: 5.2
> > Patch Level: 2
> > Release Status: release
> >
> > Description:
> > Parameter expansion within heredocs fails.
> >
> > The code below works with other shells aswell as bash 5.1
>
> Thanks for the report. It's an off-by-one error that is specific to command
> substitution within parameter expansion in a here-document line. Here's a
> patch.
>
> Chet
>
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
>  ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Hello,

Are you waiting for feedback for the patch?

This bug can break scripts completely or change them silently, so I'd guess
there should be a new Patch-Release soon.

Regards, Norbert