Re: Test suite failures under UndefinedBehaviorSanitizer (UBSAN)

[arnold]

Hi.

Thank you for the bug report, and in particular for using the
gawkbug script.

I took a walk through the code.  I don't think that your patch
is correct -- the returned cnt should either be positive or
negative all the time, but if there was an error we want
to report it and then return the value.  errcode is initialized
to zero before being passed to get_a_record, so if it comes
back non-zero, we know something bad happened.

In any case, I don't think applying your patch is correct.

Thanks,

Arnold

Sam James <sam@gentoo.org> wrote:

> Configuration Information [Automatically generated, do not change]:
> Machine: x86_64
> OS: linux-gnu
> Compiler: x86_64-pc-linux-gnu-gcc
> Compilation CFLAGS: -O2 -pipe -march=native -fdiagnostics-color=always 
> -frecord-gcc-switches -Wreturn-type -ggdb3 -DNDEBUG
> uname output: Linux mop 5.15.85-gentoo-dist-hardened #1 SMP Thu Dec 22 
> 21:20:03 GMT 2022 x86_64 AMD Ryzen 9 3950X 16-Core Processor AuthenticAMD 
> GNU/Linux
> Machine Type: x86_64-pc-linux-gnu
>
> Gawk Version: 5.2.1
>
> Attestation 1:
>         I have read 
> https://www.gnu.org/software/gawk/manual/html_node/Bugs.html.
>
> Attestation 2:
>         I have not modified the sources before building gawk.
>
> Description:
>         When built with UndefinedBehaviorSanitizer (UBSAN), gawk's test suite
>         seems to have some errors which indicates the presence of undefined 
> behavior.
>
>         I noticed this when looking into a recent bug report on bug-gawk
>         about a miscompiled gawk when built with Clang:
>         https://lists.gnu.org/archive/html/bug-gawk/2022-12/msg00010.html.
>
>         (It's common for compiler developers to ask if you tested your code
>         with sanitizers when reporting a miscompilation.)
>
> Repeat-By:
>         1. git clone gawk
>         2. ./configure CFLAGS="-O2 -fsanitize=undefined -ggdb3" 
> LDFLAGS="-fsanitize=undefined -ggdb3"
>         3. make check (and see failure in pipeio2 test)
>         4. Run the pipeio2 test by itself to get more details:
> ```
> $ export UBSAN_OPTIONS=print_stacktrace=1
>  $ ./gawk -v SRCDIR=/tmp/gawk/test -f test/pipeio2.awk
> 'echo    January 1997 | sed "s/[0-9]/./g"'
> January ....
> 'echo  S  M Tu  W Th  F  S | sed "s/[0-9]/./g"'
> S M Tu W Th F S
> 'echo           1  2  3  4 | sed "s/[0-9]/./g"'
> . . . .
> 'echo  5  6  7  8  9 10 11 | sed "s/[0-9]/./g"'
> . . . . . .. ..
> 'echo 12 13 14 15 16 17 18 | sed "s/[0-9]/./g"'
> .. .. .. .. .. .. ..
> 'echo 19 20 21 22 23 24 25 | sed "s/[0-9]/./g"'
> .. .. .. .. .. .. ..
> 'echo 26 27 28 29 30 31 | sed "s/[0-9]/./g"'
> .. .. .. .. .. ..
> node.c:423:3: runtime error: null pointer passed as argument 2, which is 
> declared to never be null
>     #0 0x41191c in make_str_node /tmp/gawk/node.c:423
>     #1 0x550483 in do_getline_redir /tmp/gawk/io.c:2871
>     #2 0x4ef1f6 in r_interpret /tmp/gawk/interpret.h:1330
>     #3 0x41929e in main /tmp/gawk/main.c:538
>     #4 0x7fcfe19e064f in __libc_start_call_main 
> ../sysdeps/nptl/libc_start_call_main.h:58
>     #5 0x7fcfe19e0708 in __libc_start_main_impl ../csu/libc-start.c:381
>     #6 0x41af14 in _start ../sysdeps/x86_64/start.S:115
> ```
>
> Fix:
>         I don't think this is necessarily a correct fix, but this
>         patch sliences it at least:
> ```
> --- a/io.c
> +++ b/io.c
> @@ -2844,7 +2844,7 @@ do_getline_redir(int into_variable, enum redirval 
> redirtype)
>
>         errcode = 0;
>         cnt = get_a_record(& s, iop, & errcode, (lhs ? NULL : & field_width));
> -       if (errcode != 0) {
> +       if (!cnt || errcode != 0) {
>                 if (! do_traditional && (errcode != -1))
>                         update_ERRNO_int(errcode);
>                 return make_number((AWKNUM) cnt);
> ```
>
>         Feel free to ignore the patch if it doesn't look right. I won't be 
> offended ;)