sysprof_capture_reader_list_files is broken
I'm getting a test suite failure building sysprof 3.38.0 on Arch Linux:
==> Starting check()...
ninja: Entering directory `/build/sysprof/src/build'
ninja: no work to do.
1/5 test-capture FAIL 0.48s (killed by signal 6 SIGABRT)
2/5 test-capture-cursor OK 0.02s
3/5 test-mapped-ring-buffer OK 0.04s
4/5 test-model-filter OK 0.08s
5/5 test-zoom OK 0.05s
Ok: 4
Expected Fail: 0
Fail: 1
Unexpected Pass: 0
Skipped: 0
Timeout: 0
The output from the failed tests:
1/5 test-capture FAIL 0.48s (killed by signal 6 SIGABRT)
--- command ---
18:47:47 MALLOC_CHECK_='2' NO_AT_BRIDGE='1' G_TEST_SRCDIR='"/build/sysprof/src/sysprof/src/tests"' GSETTINGS_BACKEND='memory' G_DEBUG='gc-friendly' G_TEST_BUILDDIR='"/build/sysprof/src/build/src/tests"' /build/sysprof/src/build/src/tests/test-capture
--- stdout ---
# random seed: R02S67c4ab9543dc6aaf9ae195f185c6d766
1..9
# Start of SysprofCapture tests
ok 1 /SysprofCapture/ReaderWriter
# Start of ReaderWriter tests
ok 2 /SysprofCapture/ReaderWriter/alloc_free
ok 3 /SysprofCapture/ReaderWriter/log
ok 4 /SysprofCapture/ReaderWriter/mark
ok 5 /SysprofCapture/ReaderWriter/metadata
Bail out! test-capture:ERROR:../sysprof/src/tests/test-capture.c:790:test_reader_writer_file: 'files[1]' should be NULL
--- stderr ---
**
test-capture:ERROR:../sysprof/src/tests/test-capture.c:790:test_reader_writer_file: 'files[1]' should be NULL
-------
Full log written to /build/sysprof/src/build/meson-logs/testlog.txt
Further investigation shows that the array returned is of a random length and, among one or more pointers to a "/proc/cpuinfo"
string and a NULL
at the end, also contains pointers to garbage.
I believe I've found at least two problems with the code:
-
strcmp
is passed directly toqsort
: This can't work. The sorting function gets pointers to array elements which are string pointers, so you need a cmp function which usesconst char *const *
, notconst char *
. -
sysprof_capture_reader_read_file
returns a pointer into a volatile buffer, which can get invalidated by another call to it, or any other function callingsysprof_capture_reader_ensure_space_for
, such assysprof_capture_reader_peek_type
. Iterating through the chunks can invalidate old chunks and their path arrays.
So the change introduced by 75b69d0a (@pwithnall) seems to be broken. The paths must be copied out in order to remain valid.