Reading redirected stderr results in bizarre side-effects.

[CamJN]

Minimized reproduction: https://gist.github.com/CamJN/dc4b1f61cec5aa2953cbe17bdb919ea4

Compiles and run with: clang -l criterion test.c && ./a.out --verbose 2

Weird results:

[----] Criterion v2.3.3
[====] Running 1 test from parseInt:
[RUN ] parseInt::failure
[----] stderr contents were: 'testSuite: Numerical argument out of domain
[----] ', expected: 'testSuite: Numerical argument out of domain
[----] '
[----] test.c:52: Assertion failed: The file contents of __stderrp does not match the string "testSuite: Numerical argument out of domain
[----]   ".
[FAIL] parseInt::failure: (0.00s)
[RUN ] parseInt::failure
[----] stderr contents were: 'testSuite: Numerical argument out of domain
[----] ', expected: 'testSuite: Numerical argument out of domain
[----] '
[----] test.c:52: Assertion failed: The file contents of __stderrp does not match the string "testSuite: Numerical argument out of domain
[----]   ".
[FAIL] parseInt::failure: (0.00s)
[RUN ] parseInt::failure
[----] stderr contents were: 'testSuite: Numerical argument out of domain
[----] ', expected: 'testSuite: Numerical argument out of domain
[----] '
[----] test.c:52: Assertion failed: The file contents of __stderrp does not match the string "testSuite: Numerical argument out of domain
[----]   ".
[FAIL] parseInt::failure: (0.00s)
[RUN ] parseInt::failure
[----] stderr contents were: 'testSuite: Invalid argument
[----] ', expected: 'testSuite: Numerical argument out of domain
[----] '
[----] test.c:52: Assertion failed: The file contents of __stderrp does not match the string "testSuite: Numerical argument out of domain
[----]   ".
[FAIL] parseInt::failure: (0.00s)
[====] Synthesis: Tested: 4 | Passing: 0 | Failing: 4 | Crashing: 0

Note the 2 extra spaces added to the expected string. Those only get added when lines 47-50 aren't commented out. If I don't read stderr, then the expected string is unmodified.

[MrAnno]

The root cause of the issue is that cr_get_redirected_stderr() and cr_assert_stderr_eq_str() share the same pipe under the hood, so read_file() "steals data" from the assert function.

Note: cr_get_redirected_stderr() uses fdopen(), but it does not call dup() on the file descriptor, so an fclose(f_stderr) + cr_assert_stderr_eq_str() sequence can produce a crash because stderr_redir gets closed.

Minimal reproducer:

#include <criterion/criterion.h>
#include <criterion/redirect.h>

#include <stdio.h>
#include <ctype.h>

void redirect_all_std(void)
{
    cr_redirect_stdout();
    cr_redirect_stderr();
}

void just_read(FILE *f)
{
    char buf[512];
    while (fread(buf, 1, sizeof (buf), f) > 0);
}

Test(redirect, test_outputs, .init = redirect_all_std) {
    fprintf(stderr, "bar");
    fflush(stderr);

    FILE *f_stderr = cr_get_redirected_stderr();
    just_read(f_stderr); // commenting this out will make the test pass
    /* crash: fclose(f_stderr); */

    cr_assert_stderr_eq_str("bar");
}

[Snaipe]

Yeah, I don't think it's a bug (except the fclose crash). cr_assert_stderr_eq_str will consume all of stderr, if you need to assert that the next 3 bytes of stderr are some arbitrary string value, you should use fread() to pull that data first.

That said, it seems this bites users often. I don't think we can change the behaviour of cr_assert_stderr_eq_str at this point, but we could fix that in the experimental stream API.

[CamJN]

I only read the stderr to be able to print it in case of a failure, so if that was possible none of this would matter.