aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--build2/cc/link-rule.cxx41
-rw-r--r--build2/rule.cxx2
-rw-r--r--build2/target.hxx18
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: