Initialized "cmd" (and "buf" for symmetry) to sooth clang >= 8

[czurnieden]

Picked up work at #499 after merging of #500 and found out that a newer clang compiler (somewhere between 7.0.0 and 8.0.0) caused problems that show up with valgrind. GCC up to version 9.1.0 caused no trouble.

(Last good commit was 1b3792b so it was something I did)

Commandline : LTM_CFLAGS=" -g3 " ./testme.sh --with-cc=clang-8 --test-vs-mtest=333 --with-valgrind --valgrind-options="--track-origins=yes"

Result in test_vs_mtest_err.log

==1350== Memcheck, a memory error detector
==1350== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1350== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==1350== Command: ./mtest_opponent
==1350== 
==1350== Conditional jump or move depends on uninitialised value(s)
==1350==    at 0x4014FC: mtest_opponent (mtest_opponent.c:336)
==1350==    by 0x4E5FBF6: (below main) (libc-start.c:310)
==1350==  Uninitialised value was created by a stack allocation
==1350==    at 0x400B86: mtest_opponent (mtest_opponent.c:70)
==1350== 
==1350== 
==1350== HEAP SUMMARY:
==1350==     in use at exit: 0 bytes in 0 blocks
==1350==   total heap usage: 67,008 allocs, 67,008 frees, 7,175,992 bytes allocated
==1350== 
==1350== All heap blocks were freed -- no leaks are possible
==1350== 
==1350== For counts of detected and suppressed errors, rerun with: -v
==1350== ERROR SUMMARY: 4 errors from 1 contexts (suppressed: 0 from 0)

Line 70 is srand(LTM_MTEST_RAND_SEED);
Line 336 is } else if (strcmp(cmd, "invmod") == 0) {

Difference of assembler dumps (left column with cmd = {0}; right as it was):

        #DEBUG_VALUE: s_mp_get_token:s <- [DW_OP_plus_uconst 4368] $rs    |             #DEBUG_VALUE: s_mp_get_token:s_bar <- undef
        #DEBUG_VALUE: s_mp_get_token:s_bar <- [DW_OP_plus_uconst 4368]    <
        .loc    5 62 9 prologue_end     # demo/mtest_opponent.c:62:9      <
        movl    $4096, %edx             # imm = 0x1000                    <
        xorl    %esi, %esi                                                <
        callq   memset                                                    <
.Ltmp3:                                                                   <
        #DEBUG_VALUE: s_mp_get_token:s <- undef                                         #DEBUG_VALUE: s_mp_get_token:s <- undef
        #DEBUG_VALUE: s_mp_get_token:s_bar <- undef                                     #DEBUG_VALUE: s_mp_get_token:s_bar <- undef
        #DEBUG_VALUE: s_mp_get_token:s <- undef                                         #DEBUG_VALUE: s_mp_get_token:s <- undef
        #DEBUG_VALUE: s_mp_get_token:s_bar <- undef                                     #DEBUG_VALUE: s_mp_get_token:s_bar <- undef

                                                                             ...

        #DEBUG_VALUE: s_mp_get_token:s_bar <- undef                                     #DEBUG_VALUE: s_mp_get_token:s_bar <- undef
        #DEBUG_VALUE: s_mp_get_token:s <- undef                                         #DEBUG_VALUE: s_mp_get_token:s <- undef
        #DEBUG_VALUE: s_mp_get_token:s_bar <- undef                                     #DEBUG_VALUE: s_mp_get_token:s_bar <- undef
        .loc    5 70 4                  # demo/mtest_opponent.c:70:4      |             #DEBUG_VALUE: s_mp_get_token:s <- undef
                                                                          >             .loc    5 70 4 prologue_end     # demo/mtest_opponent.c:70:4
        movl    $23, %edi                                                               movl    $23, %edi
        callq   srand                                                                   callq   srand

Difference between Clang 7.0.0 and Clang 8.0.0, no initialisation of cmd

        .cfi_offset %r15, -24                                                           .cfi_offset %r15, -24
        .cfi_offset %rbp, -16                                                           .cfi_offset %rbp, -16
                                                                          >     .Ltmp2:
                                                                          >             #DEBUG_VALUE: s_mp_get_token:s_bar <- undef
                                                                          >             #DEBUG_VALUE: s_mp_get_token:s <- undef
                                                                          >             #DEBUG_VALUE: s_mp_get_token:s_bar <- undef

                                                                ...

                                                                          >             #DEBUG_VALUE: s_mp_get_token:s <- undef
                                                                          >             #DEBUG_VALUE: s_mp_get_token:s_bar <- undef
                                                                          >             #DEBUG_VALUE: s_mp_get_token:s <- undef
        .loc    5 70 4 prologue_end     # demo/mtest_opponent.c:70:4                    .loc    5 70 4 prologue_end     # demo/mtest_opponent.c:70:4
        movl    $23, %edi                                                               movl    $23, %edi
        callq   srand                                                                   callq   srand
.Ltmp2:                                                                   |     .Ltmp3:
        .loc    5 72 8                  # demo/mtest_opponent.c:72:8                    .loc    5 72 8                  # demo/mtest_opponent.c:72:8

Well, Clang is correct, technically, no doubt.
Mmh…is initializingcmd enough here?

(I just saw that I initialized the buffers with 0 instead of \0. To late for now, have to go.)

[czurnieden]

Yes, it was definitely the uninitialized memory in the buffer named cmd.
I took the liberty to look at the output of GET_TOKEN manually, that is with some printf's (stepping through such large loops with GDB is not good for may carpal tunnel). Addition in mtest_opponent.c

      GET_TOKEN(cmd, stdin);
if(strlen(cmd) == 0){fprintf(stderr,"ZERO strlen = 0\n");fflush(stdout);}
else {fprintf(stderr,"MORE strlen = %zu, for \"%s\" last \"%x\"\n", strlen(cmd), cmd, cmd[strlen(cmd)]);fflush(stdout);}

Run test with srand(23) but without initialization of cmd results in Valgrind complaining

==29170== Memcheck, a memory error detector
==29170== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==29170== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==29170== Command: ./mtest_opponent
==29170== 
MORE strlen = 5, for "sub_d" last "0"
==29170== Conditional jump or move depends on uninitialised value(s)
==29170==    at 0x4015E3: mtest_opponent (mtest_opponent.c:338)
==29170==    by 0x4E5FBF6: (below main) (libc-start.c:310)
==29170==  Uninitialised value was created by a stack allocation
==29170==    at 0x400C26: mtest_opponent (mtest_opponent.c:70)
==29170== 
MORE strlen = 5, for "add_d" last "0"
MORE strlen = 3, for "sqr" last "0"
...

The string "sub_d" has 5 characters and the strcmp asking for it comes after the strcmp asking for "invmod" which has 6 characters. Clang did nothing with the order of the if...else if... else if ... else, of course, so the strcmp asking for "invmod" touched one character out-of-bound. It shouldn't, if strcmp would have been implemented as described in most textbooks, something like glibc-2.32/string/strcmp.c but I have an AMD with SSE-4.2, so it is glibc-2.32/sysdeps/x86_64/multiarch/strcmp-sse42.S which has the following comment hidden inside:

/*
 * This implementation uses SSE to compare up to 16 bytes at a time.
 */

I did not dig through all 1827 lines with several preprocessor branches (the code also handles strncmp, strcasecmp, and strncasecmp) but it is easy to check the assumption: replace GLibC's strcmp with the one from glibc-2.32/string/strcmp.c locally in mtest_oppeonent.c aaaand it works flawlessly.

Who's to blame now? GLibC or me? Good question.
Will try to slap something together for an MWE for a GLiBC bug report. Will tell you if something, if anything comes out of it.

[czurnieden]

It was the version of Valgrind, that was too old. All that ado for nothing, great!
Will close this PR but will also take a note in the WIki if I find time to find the last non-working version of Valgrind.