Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Libcperciva import #382

Merged
merged 2 commits into from
Apr 6, 2024
Merged

Libcperciva import #382

merged 2 commits into from
Apr 6, 2024

Conversation

gperciva
Copy link
Member

No description provided.

We tried to support valgrind checking of fork() in:
    2023-09-20 tests/valgrind: improve handling for fork() and exec()
    38e07b49d7bc662fb19917f07d19176d22cf9b54

However, that method assumed that the lowest pid would be the parent,
while the higher one(s) would be the children.  That has two flaws:

1) If the pid rolls over, the order would be broken.  For example, if
   the parent pid was 99999 on FreeBSD (the maximum value), then the
   child would be something much lower (probably in the hundreds).

   (I knew about that problem, considered it sufficiently unlikely.)

2) Since $_valgrind_check_logfiles are defined by `ls "blah"*`, they are
   listed in lexicographical order, not numerical order.  For example,
   given two logfiles foo-9999.log and foo-10000.log, foo-10000.log
   would come first.

   (I did not anticipate this problem, but even if it had occurred to
   me, I probably would have deemed it sufficiently unlikely.  However,
   by unlikely fluke, this seems to occur roughly 50% of the time with
   github actions: when checking 24-fork-func with the gcc binaries, the
   parent lands on pid 9999 exactly and the child is 10000.)

The fix is not to rely on the order: instead, generate a list of all
pids involved.  If a valgrind logfile lists a parent pid which is in
that list, it's a child; otherwise, it's the parent.
@cperciva cperciva merged commit c3a97fc into master Apr 6, 2024
2 checks passed
@gperciva gperciva deleted the libcperciva-import branch April 7, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants