ci: add open fds valgrind check

[boquan-fang]

Resolved issues:

Partially solve #4005

Description of changes:

• Add --track-fds=yes options to VALGRIND_DEFAULT option.
• Add s2n_open_fds_test.py script to test for any unexpected open file descriptors at exit.
• Integrate this script to buildspec_valgrind.yml.

Call-outs:

• Valgrind doesn't treat leaking file descriptors as error by default.
  • Here are some discussions I found to support such argument (discussion 1, disucssion 2).
  • Our current way of doing similar check is in test_exec_leak.sh. We are also storing open fd error information in a log file and then query from there.
    

    s2n-tls/codebuild/bin/test_exec_leak.sh

    Lines 83 to 97 in 2587f74

    	valgrind_log_dir=valgrind_log_dir
    	for test_file in detect_exec_leak detect_exec_leak_finish; do
    	LD_LIBRARY_PATH="build/lib:$TARGET_LIBCRYPTO_PATH/lib:$LD_LIBRARY_PATH" S2N_VALGRIND=1 \
    	valgrind --leak-check=full --show-leak-kinds=all --errors-for-leak-kinds=all \
    	--run-libc-freeres=yes -q --gen-suppressions=all --track-fds=yes \
    	--leak-resolution=high --undef-value-errors=no --trace-children=yes \
    	--suppressions=tests/unit/valgrind.suppressions --log-file="build/$valgrind_log_dir/$test_file" \
    	build/bin/$test_file
    	# search for all leaked file descriptors, excluding the valgrind_log_dir file
    	cat build/$valgrind_log_dir/$test_file | \
    	grep "Open file descriptor" | \
    	grep --invert-match $valgrind_log_dir \
    	&& fail "file leak detected while running $test_file"
    	done

  • That is why I choose this approach to write a Python script to parse LastDynamicAnalysis log file generated directly by CTest memcheck.
• The new Valgrind test will first test for memory loss. Once that test passed, then it will run s2n_open_fds_test.py script to test for open fds.
  • If a test has both memory loss and leaking open fds, then CTest memcheck will display both information.
  • If a test doesn't have any memory loss, but it leaks fds, then the Python script will capture those fd leaks and display them after CTest memcheck.
• If there is no fd leaks, then the number of open fds is going to be one more than the number of open std fds.
  • Valgrind version greater than 3.17 provides a good summary of how many fds are opened and how many std fds are opened.
  • Hence, we need to update the existing pedantic check to use Ubuntu 24, in order for this test to be merged. The pedantic check is running in Ubuntu 18 currently, and the Valgrind version in Ubuntu 18 is less than 3.17.
• We can't simply check if there are more than 4 open file descriptors. There are tests who intentionally close one or more std.
  

  s2n-tls/tests/unit/s2n_examples_test.c

  Line 238 in 9cd117f

  fclose(stdout);

  • If a test close a std, and leak another fd, that is still problematic, but if we are only checking the number of open fds is four, then that will treat that error as correct.
• One of the PR to close all /dev/urandom is not merged into the main branch. This branch is also not updated with main, so the valgrind test is supposed to fail right now.
  • Furthermore, those fixes are discovered in Ubuntu 18. If this test detects more problems on other platforms, then I will fix them in future PRs.
  • Here are some previous PRs that fixes all open fds issues on Ubuntu 18 (clean io pair, clean io pair with fork).

Testing:

• Test it locally with Ubuntu 18, 22, and 24 docker images.
• Test it on AWS codebuild. A failed result looks like this. A success test result looks like this.
  By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[dougch]

For regex use, I'd really like to see a couple tests showing the parsing working/failing.

[dougch]

With James' document comments addressed...

[dougch]

Without the regex, lgtm

[boquan-fang]

This PR needs to update the Ubuntu version from 18 to 24 for pedantic Valgrind test. Such update can only be done after PR#4852 is merged.

Such update is done. This test will success in Codebuild now. The result is attached.