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

BUG fuzzing found a bug in the resolver, we need a complete set of conflicts to do backjumping #5988

Merged
merged 22 commits into from
Sep 17, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
only skip in the backtrack_stack if the conflicting_activations i…
…s complete
  • Loading branch information
Eh2406 committed Sep 14, 2018
commit 2b1f093ab432d1456010401e6c07efb55c528e53
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/conflict_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl ConflictCache {
.entry(dep.clone())
.or_insert_with(Vec::new);
if !past.contains(con) {
trace!("{} adding a skip {:?}", dep.package_name(), con);
trace!("{} = \"{}\" adding a skip {:?}", dep.package_name(), dep.version_req(), con);
past.push(con.clone());
for c in con.keys() {
self.dep_from_pid
Expand Down
26 changes: 15 additions & 11 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ fn activate_deps_loop(
past_conflicting_activations.insert(&dep, &conflicting_activations);
}

match find_candidate(&mut backtrack_stack, &parent, &conflicting_activations) {
match find_candidate(&mut backtrack_stack, &parent, backtracked, &conflicting_activations) {
Some((candidate, has_another, frame)) => {
// Reset all of our local variables used with the
// contents of `frame` to complete our backtrack.
Expand Down Expand Up @@ -518,18 +518,18 @@ fn activate_deps_loop(
find_candidate(
&mut backtrack_stack.clone(),
&parent,
backtracked,
&conflicting_activations,
).is_none()
}
};

// If we're only here for the error messages
// and we did not use backtracking to explore our deps
// then we know one of our candidate deps will fail,
// meaning we will fail and that none of the backtrack frames will find a
// If we're only here for the error messages then we know
// one of our candidate deps will fail, meaning we will
// fail and that none of the backtrack frames will find a
// candidate that will help. Consequently let's clean up the
// no longer needed backtrack frames.
if !backtracked && activate_for_error_message {
if activate_for_error_message {
backtrack_stack.clear();
}

Expand Down Expand Up @@ -810,6 +810,7 @@ fn compatible(a: &semver::Version, b: &semver::Version) -> bool {
fn find_candidate(
backtrack_stack: &mut Vec<BacktrackFrame>,
parent: &Summary,
backtracked: bool,
conflicting_activations: &HashMap<PackageId, ConflictReason>,
) -> Option<(Candidate, bool, BacktrackFrame)> {
while let Some(mut frame) = backtrack_stack.pop() {
Expand All @@ -831,11 +832,14 @@ fn find_candidate(
// active in this back up we know that we're guaranteed to not actually
// make any progress. As a result if we hit this condition we can
// completely skip this backtrack frame and move on to the next.
if frame
.context_backup
.is_conflicting(Some(parent.package_id()), conflicting_activations)
{
continue;
if !backtracked {
if frame
.context_backup
.is_conflicting(Some(parent.package_id()), conflicting_activations)
{
trace!("{} = \"{}\" skip as not solving {}: {:?}", frame.dep.package_name(), frame.dep.version_req(), parent.package_id(), conflicting_activations);
continue;
}
}

return Some((candidate, has_another, frame));
Expand Down