diff options
| -rw-r--r-- | build2/cc/link-rule.cxx | 41 | ||||
| -rw-r--r-- | build2/rule.cxx | 2 | ||||
| -rw-r--r-- | build2/target.hxx | 18 |
3 files changed, 50 insertions, 11 deletions
diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx index 1dd8726..0ff339d 100644 --- a/build2/cc/link-rule.cxx +++ b/build2/cc/link-rule.cxx @@ -857,6 +857,10 @@ namespace build2 ? &search (t, (mod ? tt.bmi : tt.obj), rt.dir, rt.out, rt.name) : &rt; + const target_type& rtt (mod + ? (group ? bmi::static_type : tt.bmi) + : (group ? obj::static_type : tt.obj)); + // If this obj*{} already has prerequisites, then verify they are // "compatible" with what we are doing here. Otherwise, synthesize // the dependency. Note that we may also end up synthesizing with @@ -864,7 +868,12 @@ namespace build2 // bool verify (true); - if (!pt->has_prerequisites ()) + // Note that we cannot use has_group_prerequisites() since the + // target is not yet matched. So we check the group directly. Of + // course, all of this is racy (see below). + // + if (!pt->has_prerequisites () && + (!group || !rt.has_prerequisites ())) { prerequisites ps {p.as_prerequisite ()}; // Source. @@ -922,9 +931,33 @@ namespace build2 } } - // Note: add to the group, not the member. + // Note: adding to the group, not the member. // verify = !rt.prerequisites (move (ps)); + + // Recheck that the target still has no prerequisites. If that's + // no longer the case, then verify the result is compatible with + // what we need. + // + // Note that there are scenarios where we will not detect this or + // the detection will be racy. For example, thread 1 adds the + // prerequisite to the group and then thread 2, which doesn't use + // the group, adds the prerequisite to the member. This could be + // triggered by something like this (undetectable): + // + // lib{foo}: cxx{foo} + // exe{foo}: cxx{foo} + // + // Or this (detection is racy): + // + // lib{bar}: cxx{foo} + // liba{baz}: cxx{foo} + // + // The current feeling, however, is that in non-contrived cases + // (i.e., the source file is the same) this should be harmless. + // + if (!verify && group) + verify = pt->has_prerequisites (); } if (verify) @@ -939,10 +972,6 @@ namespace build2 // async match here and finish this verification in the "harvest" // loop below. // - const target_type& rtt (mod - ? (group ? bmi::static_type : tt.bmi) - : (group ? obj::static_type : tt.obj)); - resolve_group (a, *pt); // Not matched yet so resolve group. bool src (false); diff --git a/build2/rule.cxx b/build2/rule.cxx index fc80816..85f4e36 100644 --- a/build2/rule.cxx +++ b/build2/rule.cxx @@ -116,7 +116,7 @@ namespace build2 // to unchanged. This is an important optimization on which quite a few // places that deal with predominantly static content rely. // - if (!t.has_prerequisites ()) + if (!t.has_group_prerequisites ()) // Group as in match_prerequisites(). return noop_recipe; // Match all the prerequisites. diff --git a/build2/target.hxx b/build2/target.hxx index d73a98c..fb1c68c 100644 --- a/build2/target.hxx +++ b/build2/target.hxx @@ -193,6 +193,10 @@ namespace build2 // special target_state::group state. You would normally also use the // group_recipe for group members. // + // Note that the group-member link-up can happen anywhere between the + // member creation and rule matching so reading the group before the + // member has been matched can be racy. + // const target* group = nullptr; // What has been described above is a "normal" group. That is, there is @@ -320,14 +324,20 @@ namespace build2 bool prerequisites (prerequisites_type&&) const; - // Check if there are any prerequisites, taking into account group - // prerequisites. + // Check if there are any prerequisites. Note that the group version may + // be racy (see target::group). // bool has_prerequisites () const { - return !prerequisites ().empty () || - (group != nullptr && !group->prerequisites ().empty ()); + return !prerequisites ().empty (); + } + + bool + has_group_prerequisites () const + { + return has_prerequisites () || + (group != nullptr && !group->has_prerequisites ()); } private: |