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

Hang when pass failed to modify the variant shortly before STOP #156

Open
emaxx-google opened this issue Jan 14, 2025 · 2 comments · May be fixed by #162
Open

Hang when pass failed to modify the variant shortly before STOP #156

emaxx-google opened this issue Jan 14, 2025 · 2 comments · May be fixed by #162

Comments

@emaxx-google
Copy link
Contributor

I experience occasional hangs in particular passes, e.g.:

01:23:45 INFO ===< ClangPass::replace-dependent-typedef >===
01:23:45 INFO (0.0%, 22566 bytes, 541 lines)
01:23:46 INFO (0.0%, 22566 bytes, 541 lines)
// ... the same repeats - even hours later

According to the initial investigation, there seems to be a bug in the replace-dependent-typedef pass, so that it sometimes reports success despite not making any changes in the file. This is normally caught by this check:

if not self.report_pass_bug(test_env, 'pass failed to modify the variant'):

, however this line is not reached if quit_loop has already been set to True because an earlier invocation returned PassResult.STOP.

The end effect is that the execution gets stuck in this state ~forever: the pass keeps being applied, the file remains the same all the time, and the file comparison check is skipped.

@emaxx-google
Copy link
Contributor Author

emaxx-google commented Jan 14, 2025

For what it's worth, this particular bug in replace-dependent-typedef will likely be fixed upstream: csmith-project/creduce#281 . However, the potential hang scenario is still very unfortunate because there's no 100% guarantee that no other pass has the same bug.

@emaxx-google
Copy link
Contributor Author

As for the hang problem itself, one easy fix would probably be to compare the files in TestManager.process_result(). However, we might want to do some code reuse and also to check the other things that process_done_futures() was performing, like the one against max_improvement.

emaxx-google added a commit to emaxx-google/cvise that referenced this issue Jan 21, 2025
Implement unit tests specifically for the TestManager class, with
stubbing out the pass logic. Cover the basic successful minimization
scenarios and basic error scenarios.

This lays groundwork for the regression testing of marxin#156.
emaxx-google added a commit to emaxx-google/cvise that referenced this issue Jan 21, 2025
Implement unit tests specifically for the TestManager class, with
stubbing out the pass logic. Cover the basic successful minimization
scenarios and basic error scenarios.

This lays groundwork for the regression testing of marxin#156.
emaxx-google added a commit to emaxx-google/cvise that referenced this issue Jan 21, 2025
Implement unit tests specifically for the TestManager class, with
stubbing out the pass logic. Cover the basic successful minimization
scenarios and basic error scenarios.

This lays groundwork for the regression testing of marxin#156.
emaxx-google added a commit to emaxx-google/cvise that referenced this issue Jan 21, 2025
Close the file handle after it's not needed anymore.

This problem was detected by pytest in my work for marxin#156:

  cvise/utils/misc.py:8: ResourceWarning: unclosed file
  <_io.TextIOWrapper name='input.txt' mode='r' encoding='UTF-8'>
    open(filename).read()
emaxx-google added a commit to emaxx-google/cvise that referenced this issue Jan 21, 2025
Make sure that we don't proceed with an unmodified file - for this,
consistently validate the pass' results on both codepaths (in
process_done_futures() and wait_for_first_success()).

Besides fixing the hang that was possible when a pass misbehaved,
there are a couple of other side effects:

* more cases of ERRORs in a pass will be reported - previously only
  the ones on the first codepath were handled;
* max_improvement is enforced stricter now - previously the second
  codepath allowed a larger reduction to sneak in.

This fixes marxin#156.
emaxx-google added a commit to emaxx-google/cvise that referenced this issue Jan 21, 2025
Make sure that we don't proceed with an unmodified file - for this,
consistently validate the pass' results on both codepaths (in
process_done_futures() and wait_for_first_success()).

Besides fixing the hang that was possible when a pass misbehaved,
there are a couple of other side effects:

* more cases of ERRORs in a pass will be reported - previously only
  the ones on the first codepath were handled;
* max_improvement is enforced stricter now - previously the second
  codepath allowed a larger reduction to sneak in.

This fixes marxin#156.
emaxx-google added a commit to emaxx-google/cvise that referenced this issue Jan 21, 2025
Make sure that we don't proceed with an unmodified file - for this,
consistently validate the pass' results on both codepaths (in
process_done_futures() and wait_for_first_success()).

Besides fixing the hang that was possible when a pass misbehaved,
there are a couple of other side effects:

* more cases of ERRORs in a pass will be reported - previously only
  the ones on the first codepath were handled;
* max_improvement is enforced stricter now - previously the second
  codepath allowed a larger reduction to sneak in.

This fixes marxin#156.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant