Normally a newly recreated file descriptor gets the lowest available number. This might cause old file descriptor numbers to be reused and hides bad file descriptor accesses (because the old number is new again). Using --modify-fds=high option makes Valgrind to get a new file descriptors that is ahighest availablenot yet before number. This is implemented by making record_fd_open* return a new file descriptor (by dupping the given file descriptor only a higher never used before number and closing the one returned from the kernel) and returning that to the application. Using --modify-fds=strict makes an exception for file descriptors 0, 1, 2 which programs might explicitly want to reassign.
This depends on https://bugs.kde.org/show_bug.cgi?id=493433
Created attachment 180988 [details] patch Use --modify-fds=yes to restrict the option from affecting the 0/1/2 file descriptors as they're often used for stdout/stderr redirection.
Looks good, but some nitpicks. I like extended commit messages, this doesn't really explain why we picked "yes" (it is because the default should work for most). It also doesn't explain what happens when the number isn't 0/1/2 (they are handled like with "high"). Not a fan of having to remember what numbers stand for. If at all possible please define and use something like MODIFY_FD_NO/YES/HIGH. The \ don't really line up in the POST_newFd_RES define (nitpick) For the testcase it would probably be good to use dup The comment says "redirect stdout as stderr" dup2 (2, 1); does that indeed, dup stderr (2) into stdout (1) But for the test it would be better to do it as close (1); dup (2); Since dup (2) is supposed to pick the lowest but with =high it would pick some high fd number with =yes it should pick 1 since that is the lowest (just closed). You probably also want to test that. So write something to file descriptor 1 and see it appear in stderr (so you can check with track_yes.stderr.exp) And stdout should still be empty. +cleanup: rm -f foobad.txt foobar.txt For track_yes.vgtest seems not needed. You don't create those files in this test.
Created attachment 181651 [details] patch Add "yes" argument for the --modify-fds option. Use --modify-fds=yes to restrict the option from affecting the 0/1/2 file descriptors as they're often used for stdin/tdout/stderr redirection. The new possibility is named "yes" because "yes" is used as the default in general. The default behaviour of the --modify-fds option is then such, that highest available file descriptor is returned execept when the lowest stdin/stdout/stderr (0, 1, 2) are available. For example, if we want to redirect stdout to stderr by closing stdout (file descriptor 1) and then calling dup (), file descriptor 1 will be returned and not the highest number available. This is because the following is a common pattern to redirect stdout to stderr: close (1); /* stdout becomes stderr */ ret = dup (2); Add none/tests/track_yes.vgtest and none/tests/track_high.vgtest tests to test --modify-fds=yes/high behave as expected. https://bugs.kde.org/show_bug.cgi?id=502359
Looks good. Thanks. Pushed as: commit ebd7dd5ea9504e0d8490507fd2b894647477b085 Author: Alexandra Hájková <ahajkova@redhat.com> Date: Tue May 6 06:50:44 2025 -0400 Add "yes" argument for the --modify-fds option. Use --modify-fds=yes to restrict the option from affecting the 0/1/2 file descriptors as they're often used for stdin/tdout/stderr redirection. The new possibility is named "yes" because "yes" is used as the default in general. The default behaviour of the --modify-fds option is then such, that highest available file descriptor is returned execept when the lowest stdin/stdout/stderr (0, 1, 2) are available. For example, if we want to redirect stdout to stderr by closing stdout (file descriptor 1) and then calling dup (), file descriptor 1 will be returned and not the highest number available. This is because the following is a common pattern to redirect stdout to stderr: close (1); /* stdout becomes stderr */ ret = dup (2); Add none/tests/track_yes.vgtest and none/tests/track_high.vgtest tests to test --modify-fds=yes/high behave as expected. https://bugs.kde.org/show_bug.cgi?id=502359