'make check' oddities

[claudioandre-br]

It doesn't work on Solaris:

Version: 1.9.0-jumbo-1+bleeding-22e272af6a 2024-05-20 20:19:15 +0200
Build: solaris2.11 64-bit x86_64 AVX2 AC OMP
SIMD: AVX2, interleaving: MD4:3 MD5:3 SHA1:1 SHA256:1 SHA512:1
CPU tests: AVX2
$JOHN is ../run/
[...]
Parsed terminal locale: UNDEF

--------------------------- make check ---------------------------
make: Fatal error in reader: Makefile, line 20: Unexpected end of line seen

Isn't it performing the same thing twice? Is the second not enough?

../run/john --test=0 --verbosity=2 --format=cpu
../run/john --test=0 --verbosity=2 --format=+dynamic,all

Then:

All 407 formats passed self-tests
All 477 formats passed self-tests

[solardiz]

make: Fatal error in reader: Makefile, line 20: Unexpected end of line seen

I think this is unrelated to make check and isn't a new thing. Rather, our generated Makefile is incompatible with Solaris' make, which is why on Solaris our configure output finishes with suggestion to use gmake instead:

Configure finished.  Now "gmake -s clean && gmake -sj8" to compile.

(BTW, looks like our recently added usage of nproc has worked fine here. Solaris 11 with GNU coreutils installed.)

[solardiz]

Isn't it performing the same thing twice? Is the second not enough?

../run/john --test=0 --verbosity=2 --format=cpu
../run/john --test=0 --verbosity=2 --format=+dynamic,all

It's weird. I've just tested, and there's only partial overlap between the two. On a system (edit: Linux/x86_64 with AVX512BW) where the first command tested 425 formats and the second 477, only 161 formats overlap between the two runs - some dynamic formats (not all dynamics) and thin dynamic formats (without dynamic in visible name).

We got to have a syntax to test all formats with one command.

[solardiz]

gmake check fails on the Solaris 11 system mentioned above, with:

**** Performing unit tests for functions from [sha2.c] ****
  test 34 test_sha224             -  Performed      129 tests 0.22 seconds used                                                    
  test 35 test_sha256             -  Performed      780 tests 0.37 seconds used                                                    
  test 36 test_sha384             -  Performed      908 tests 1.46 seconds used                                                    
  test 37 test_sha512             -  Performed      908 tests 1.47 seconds used                                                    
Performed testing on 37 total functions.
Total time taken : 56.79

gmake[1]: *** [Makefile:1728: ../run/unit-tests] Error 1
gmake[1]: Leaving directory '/export/home/solar/john/src'
gmake: *** [Makefile:1658: check] Error 2

where line 1658 is:

        $(MAKE) unit-tests

and line 1728 is:

        @ ${POSSIBLE_WINE} ../run/unit-tests

Re-running unit-tests manually, I see this:

**** Performing unit tests for functions from [misc.c] ****
  test 1 _nontest_gen_fgetl_files -  Performed        3 tests 0.41 seconds used
  test 2 test_fgetl               -  Performed    18003 tests WITH 1 FAILURES!!! 2.11 seconds used
  test 3 test_fgetll              -  Performed    18003 tests 1.15 seconds used

and the exit code is 1.

So looks like we actually got a bug somewhere in there.

[solardiz]

Skipping the unit tests, my external modes test was failing on Solaris at first. Turned out the tr command I use in the KDEPaste test failed to perform the replacement. Replacing it with sed 's/././g' made this test succeed.

[solardiz]

Out of these 3, the failing test is in the middle one:

        _tst_fget_l_ll("/tmp/jnk.txt", 1);
        _tst_fget_l_ll("/tmp/jnkd.txt", 1);
        _tst_fget_l_ll("/tmp/jnkfu.txt", 1);

This also highlights that we have insecure usage of /tmp in there, which I think is unacceptable even for tests. We ought to patch that.

[solardiz]

Making it work on files in the current directory (rather than /tmp) made no difference (still one failing test there). Checking against Linux, I see the files' content is not the same between Linux and Solaris. Same number of lines (5999 each), but the files on Solaris are slightly larger:

$ wc jnk*
    5999  617701 57274521 jnk.txt
    5999  617701 57280520 jnkd.txt
    5999  617701 57275269 jnkfu.txt

vs. Linux

     5999    617036  57213723 jnk.txt
     5999    617036  57219722 jnkd.txt
     5999    617036  57214471 jnkfu.txt

So the failing test doesn't necessarily indicate that our fgetl function works differently. And even if it does, that's probably a side-effect of a Solaris libc function that it uses working differently.

jnkd.txt has CR-LF pair on almost every line, with very long lines.

[solardiz]

It's failing here, and I added debugging output:

-       if (memcmp(&_fgetl_pad[i], &line[i], len_seen-i))
+       if (memcmp(&_fgetl_pad[i], &line[i], len_seen-i)) {
+               int w = len_seen-i;
+               printf("%.*s vs. %.*s length %d\n", w, &_fgetl_pad[i], w, &line[i], w);
                inc_failed_test();
+       }

With this, I see that &_fgetl_pad[i] ends in HIJ whereas &line[i] ends in HI^M (J replaced by CR), len_seen-i is 16370.

This is such a stress-tests of libc's stdio and, if I test it with external tools, also of those tools. On Linux, I get:

$ grep -c 'HIJ^M$' /tmp/jnkd.txt 
57

On Solaris, I get:

$ grep -c 'HIJ^M$' jnkd.txt 
55
$ ggrep -c 'HIJ^M$' jnkd.txt 
61

(On both, I entered the ^M by typing Ctrl-V, Ctrl-M.)

I think we can't really expect the kind of consistency this unit test expects now, while we use stdio. If we wanted that, we'd need to switch away from using stdio to our own code first, at least for high-level functions like fprintf used during creation of those test files.

[solardiz]

Reducing _FGETL_PAD_SIZE from 19000 to 16000 makes the test pass. Maybe we should just do that, at least on Solaris.

[claudioandre-br]

make: Fatal error in reader: Makefile, line 20: Unexpected end of line seen

I think this is unrelated to make check and isn't a new thing.

Indeed. I didn't pay attention to the error message.

It goes further now.

  --------------------------- gmake check ---------------------------
  gmake find_version
  gmake[1]: Entering directory '/root/tmp/src'
  echo "#define JTR_GIT_VERSION JUMBO_VERSION "\"-22e272af6a\" \" 2024-05-20 20:19:15 +0200\""" > version.h.new
  diff >/dev/null 2>/dev/null version.h.new version.h && rm -f version.h.new || mv -f version.h.new version.h
  gmake[1]: Leaving directory '/root/tmp/src'

[solardiz]

I got gmake check to pass on Solaris, so this issue will be closed by an upcoming PR shortly.

I'm not too worried about the duplicate testing of some dynamic formats because those are particularly quick to test. However, it does seem weird we don't use just one invocation of john to test them all.