aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2020-11-23 10:22:48 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2020-11-23 10:22:48 +0200
commitb7f4e6154fa3c07ad3472bbd7c871df28d440a64 (patch)
tree124a07cd2050c573eb7b12821db5904264b86957
parent6a2d1e3062964fc16cfbc43bc69284f854c35dca (diff)
Suppress duplicates when extracting library options (GitHub issue #114)
-rw-r--r--libbuild2/cc/compile-rule.cxx77
-rw-r--r--libbuild2/cc/compile-rule.hxx26
-rw-r--r--libbuild2/cc/functions.cxx72
-rw-r--r--libbuild2/cc/link-rule.cxx162
-rw-r--r--libbuild2/cc/link-rule.hxx60
5 files changed, 298 insertions, 99 deletions
diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index 6ddff08..a2ff1bd 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -376,20 +376,26 @@ namespace build2
}
// Append or hash library options from a pair of *.export.* variables
- // (first one is cc.export.*) recursively, prerequisite libraries first.
+ // (first is x.* then cc.*) recursively, prerequisite libraries first.
//
template <typename T>
void compile_rule::
- append_lib_options (T& args,
- const scope& bs,
- action a, const file& l, bool la, linfo li) const
+ append_library_options (appended_libraries& ls, T& args,
+ const scope& bs,
+ action a, const file& l, bool la, linfo li) const
{
+ struct data
+ {
+ appended_libraries& ls;
+ T& args;
+ } d {ls, args};
+
// See through utility libraries.
//
auto imp = [] (const file& l, bool la) {return la && l.is_a<libux> ();};
- auto opt = [&args, this] (
- const file& l, const string& t, bool com, bool exp)
+ auto opt = [&d, this] (const file& l,
+ const string& t, bool com, bool exp)
{
// Note that in our model *.export.poptions are always "interface",
// even if set on liba{}/libs{}, unlike loptions.
@@ -397,6 +403,15 @@ namespace build2
if (!exp) // Ignore libux.
return;
+ // Suppress duplicates.
+ //
+ // Compilation is the simple case: we can add the options on the first
+ // occurrence of the library and ignore all subsequent occurrences.
+ // See GitHub issue #114 for details.
+ //
+ if (find (d.ls.begin (), d.ls.end (), &l) != d.ls.end ())
+ return;
+
const variable& var (
com
? c_export_poptions
@@ -404,7 +419,13 @@ namespace build2
? x_export_poptions
: l.ctx.var_pool[t + ".export.poptions"]));
- append_options (args, l, var);
+ append_options (d.args, l, var);
+
+ // From the process_libraries() semantics we know that the final call
+ // is always for the common options.
+ //
+ if (com)
+ d.ls.push_back (&l);
};
process_libraries (a, bs, li, sys_lib_dirs,
@@ -413,19 +434,21 @@ namespace build2
}
void compile_rule::
- append_lib_options (strings& args,
- const scope& bs,
- action a, const file& l, bool la, linfo li) const
+ append_library_options (appended_libraries& ls, strings& args,
+ const scope& bs,
+ action a, const file& l, bool la, linfo li) const
{
- append_lib_options<strings> (args, bs, a, l, la, li);
+ append_library_options<strings> (ls, args, bs, a, l, la, li);
}
template <typename T>
void compile_rule::
- append_lib_options (T& args,
- const scope& bs,
- action a, const target& t, linfo li) const
+ append_library_options (T& args,
+ const scope& bs,
+ action a, const target& t, linfo li) const
{
+ appended_libraries ls;
+
for (prerequisite_member p: group_prerequisite_members (a, t))
{
if (include (a, t, p) != include_type::normal) // Excluded/ad hoc.
@@ -444,7 +467,7 @@ namespace build2
(la = (f = pt->is_a<libux> ())) ||
( (f = pt->is_a<libs> ())))
{
- append_lib_options (args, bs, a, *f, la, li);
+ append_library_options (ls, args, bs, a, *f, la, li);
}
}
}
@@ -454,11 +477,11 @@ namespace build2
// recursively, prerequisite libraries first.
//
void compile_rule::
- append_lib_prefixes (prefix_map& m,
- const scope& bs,
- action a,
- target& t,
- linfo li) const
+ append_library_prefixes (prefix_map& m,
+ const scope& bs,
+ action a,
+ target& t,
+ linfo li) const
{
auto imp = [] (const file& l, bool la) {return la && l.is_a<libux> ();};
@@ -478,7 +501,7 @@ namespace build2
append_prefixes (m, l, var);
};
- // The same logic as in append_lib_options().
+ // The same logic as in append_library_options().
//
const function<bool (const file&, bool)> impf (imp);
const function<void (const file&, const string&, bool, bool)> optf (opt);
@@ -711,7 +734,7 @@ namespace build2
// A dependency on a library is there so that we can get its
// *.export.poptions, modules, etc. This is the library metadata
- // protocol. See also append_lib_options().
+ // protocol. See also append_library_options().
//
if (pi == include_type::normal &&
(p.is_a<libx> () ||
@@ -899,7 +922,7 @@ namespace build2
// Hash *.export.poptions from prerequisite libraries.
//
- append_lib_options (cs, bs, a, t, li);
+ append_library_options (cs, bs, a, t, li);
}
append_options (cs, t, c_coptions);
@@ -1485,7 +1508,7 @@ namespace build2
// Then process the include directories from prerequisite libraries.
//
- append_lib_prefixes (m, bs, a, t, li);
+ append_library_prefixes (m, bs, a, t, li);
return m;
}
@@ -2917,7 +2940,7 @@ namespace build2
// Add *.export.poptions from prerequisite libraries.
//
- append_lib_options (args, bs, a, t, li);
+ append_library_options (args, bs, a, t, li);
// Populate the src-out with the -I$out_base -I$src_base pairs.
//
@@ -4313,7 +4336,7 @@ namespace build2
append_options (args, t, x_poptions);
append_options (args, t, c_poptions);
- append_lib_options (args, t.base_scope (), a, t, li);
+ append_library_options (args, t.base_scope (), a, t, li);
if (md.symexport)
append_symexport_options (args, t);
@@ -5940,7 +5963,7 @@ namespace build2
// Add *.export.poptions from prerequisite libraries.
//
- append_lib_options (args, bs, a, t, li);
+ append_library_options (args, bs, a, t, li);
if (md.symexport)
append_symexport_options (args, t);
diff --git a/libbuild2/cc/compile-rule.hxx b/libbuild2/cc/compile-rule.hxx
index cbbb142..e52fed9 100644
--- a/libbuild2/cc/compile-rule.hxx
+++ b/libbuild2/cc/compile-rule.hxx
@@ -54,10 +54,12 @@ namespace build2
perform_clean (action, const target&) const;
public:
+ using appended_libraries = small_vector<const file*, 256>;
+
void
- append_lib_options (strings&,
- const scope&,
- action, const file&, bool, linfo) const;
+ append_library_options (appended_libraries&, strings&,
+ const scope&,
+ action, const file&, bool, linfo) const;
protected:
static void
functions (function_family&, const char*); // functions.cxx
@@ -72,15 +74,15 @@ namespace build2
template <typename T>
void
- append_lib_options (T&,
- const scope&,
- action, const file&, bool, linfo) const;
+ append_library_options (appended_libraries&, T&,
+ const scope&,
+ action, const file&, bool, linfo) const;
template <typename T>
void
- append_lib_options (T&,
- const scope&,
- action, const target&, linfo) const;
+ append_library_options (T&,
+ const scope&,
+ action, const target&, linfo) const;
// Mapping of include prefixes (e.g., foo in <foo/bar>) for auto-
// generated headers to directories where they will be generated.
@@ -107,9 +109,9 @@ namespace build2
append_prefixes (prefix_map&, const target&, const variable&) const;
void
- append_lib_prefixes (prefix_map&,
- const scope&,
- action, target&, linfo) const;
+ append_library_prefixes (prefix_map&,
+ const scope&,
+ action, target&, linfo) const;
prefix_map
build_prefix_map (const scope&, action, target&, linfo) const;
diff --git a/libbuild2/cc/functions.cxx b/libbuild2/cc/functions.cxx
index ca93b63..78ee212 100644
--- a/libbuild2/cc/functions.cxx
+++ b/libbuild2/cc/functions.cxx
@@ -27,15 +27,16 @@ namespace build2
struct lib_data
{
const char* x;
- void (*f) (strings&,
+ void (*f) (void*, strings&,
const vector_view<value>&, const module&, const scope&,
action, const file&, bool, linfo);
};
static value
- lib_thunk (const scope* bs,
- vector_view<value> vs,
- const function_overload& f)
+ lib_thunk_impl (void* ls,
+ const scope* bs,
+ vector_view<value> vs,
+ const function_overload& f)
{
const lib_data& d (*reinterpret_cast<const lib_data*> (&f.data));
@@ -114,7 +115,7 @@ namespace build2
(la = (f = t.is_a<liba> ())) ||
( (f = t.is_a<libs> ())))
{
- d.f (r, vs, *m, *bs, a, *f, la, li);
+ d.f (ls, r, vs, *m, *bs, a, *f, la, li);
}
else
fail << t << " is not a library target";
@@ -123,6 +124,16 @@ namespace build2
return value (move (r));
}
+ template <typename L>
+ static value
+ lib_thunk (const scope* bs,
+ vector_view<value> vs,
+ const function_overload& f)
+ {
+ L ls;
+ return lib_thunk_impl (&ls, bs, vs, f);
+ }
+
void compile_rule::
functions (function_family& f, const char* x)
{
@@ -132,19 +143,24 @@ namespace build2
// sources that depend on the specified libraries. The second argument
// is the output target type (obje, objs, etc).
//
- // Note that this function can only be called during execution after all
- // the specified library targets have been matched. Normally it is used
- // in ad hoc recipes to implement custom compilation.
+ // Note that passing multiple targets at once is not a mere convenience:
+ // this also allows for more effective duplicate suppression.
+ //
+ // Note also that this function can only be called during execution
+ // after all the specified library targets have been matched. Normally
+ // it is used in ad hoc recipes to implement custom compilation.
+ //
//
f[".lib_poptions"].insert<lib_data, names, names> (
- &lib_thunk,
+ &lib_thunk<appended_libraries>,
lib_data {
x,
- [] (strings& r,
+ [] (void* ls, strings& r,
const vector_view<value>&, const module& m, const scope& bs,
action a, const file& l, bool la, linfo li)
{
- m.append_lib_options (r, bs, a, l, la, li);
+ m.append_library_options (
+ *static_cast<appended_libraries*> (ls), r, bs, a, l, la, li);
}});
}
@@ -164,16 +180,19 @@ namespace build2
// If the last argument is false, then do not return the specified
// libraries themselves.
//
- // Note that this function can only be called during execution after all
- // the specified library targets have been matched. Normally it is used
- // in ad hoc recipes to implement custom linking.
+ // Note that passing multiple targets at once is not a mere convenience:
+ // this also allows for more effective duplicate suppression.
+ //
+ // Note also that this function can only be called during execution
+ // after all the specified library targets have been matched. Normally
+ // it is used in ad hoc recipes to implement custom linking.
//
f[".lib_libs"].insert<lib_data,
names, names, optional<names>, optional<names>> (
- &lib_thunk,
+ &lib_thunk<appended_libraries>,
lib_data {
x,
- [] (strings& r,
+ [] (void* ls, strings& r,
const vector_view<value>& vs, const module& m, const scope& bs,
action a, const file& l, bool la, linfo li)
{
@@ -193,7 +212,9 @@ namespace build2
bool self (vs.size () > 3 ? convert<bool> (vs[3]) : true);
- m.append_libraries (r, bs, a, l, la, lf, li, self);
+ m.append_libraries (*static_cast<appended_libraries*> (ls), r,
+ bs,
+ a, l, la, lf, li, self);
}});
// $<module>.lib_rpaths(<targets>, <otype> [, <link> [, <self>]])
@@ -209,22 +230,27 @@ namespace build2
// If the last argument is false, then do not return the options for the
// specified libraries themselves.
//
- // Note that this function can only be called during execution after all
- // the specified library targets have been matched. Normally it is used
- // in ad hoc recipes to implement custom linking.
+ // Note that passing multiple targets at once is not a mere convenience:
+ // this also allows for more effective duplicate suppression.
+
+ // Note also that this function can only be called during execution
+ // after all the specified library targets have been matched. Normally
+ // it is used in ad hoc recipes to implement custom linking.
//
f[".lib_rpaths"].insert<lib_data,
names, names, optional<names>, optional<names>> (
- &lib_thunk,
+ &lib_thunk<rpathed_libraries>,
lib_data {
x,
- [] (strings& r,
+ [] (void* ls, strings& r,
const vector_view<value>& vs, const module& m, const scope& bs,
action a, const file& l, bool la, linfo li)
{
bool link (vs.size () > 2 ? convert<bool> (vs[2]) : false);
bool self (vs.size () > 3 ? convert<bool> (vs[3]) : true);
- m.rpath_libraries (r, bs, a, l, la, li, link, self);
+ m.rpath_libraries (*static_cast<rpathed_libraries*> (ls), r,
+ bs,
+ a, l, la, li, link, self);
}});
}
}
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 240881a..8175489 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -1489,19 +1489,20 @@ namespace build2
}
void link_rule::
- append_libraries (strings& args,
+ append_libraries (appended_libraries& ls, strings& args,
const scope& bs, action a,
const file& l, bool la, lflags lf, linfo li,
bool self) const
{
struct data
{
+ appended_libraries& ls;
strings& args;
const file& l;
action a;
linfo li;
compile_target_types tts;
- } d {args, l, a, li, compile_types (li.type)};
+ } d {ls, args, l, a, li, compile_types (li.type)};
auto imp = [] (const file&, bool la)
{
@@ -1515,6 +1516,61 @@ namespace build2
{
const file* l (lc != nullptr ? *lc : nullptr);
+ // Suppress duplicates.
+ //
+ // Linking is the complicated case: we cannot add the libraries and
+ // options on the first occurrence of the library and ignore all
+ // subsequent occurrences because of the static linking semantics.
+ // Instead, we can ignore all the occurrences except the last which
+ // would normally be done with a pre-pass but would also complicate
+ // things quite a bit. So instead we are going to keep track of the
+ // duplicates like we've done in other places but in addition we will
+ // also keep track of the elements added to args corresponding to this
+ // library. And whenever we see a duplicate, we are going to "hoist"
+ // that range of elements to the end of args. See GitHub issue #114
+ // for details.
+ //
+ // From the process_libraries() semantics we know that this callback
+ // is always called and always after the options callbacks.
+ //
+ appended_library& al (l != nullptr
+ ? d.ls.append (*l, d.args.size ())
+ : d.ls.append (p, d.args.size ()));
+
+ if (al.end != appended_library::npos) // Closed.
+ {
+ // Hoist the elements corresponding to this library to the end.
+ //
+ if (al.begin != al.end)
+ {
+ // Rotate to the left the subrange starting from the first element
+ // of this library and until the end so that the element after the
+ // last element of this library becomes the first element of this
+ // subrange. We also need to adjust begin/end of libraries
+ // affected by the rotation.
+ //
+ rotate (d.args.begin () + al.begin,
+ d.args.begin () + al.end,
+ d.args.end ());
+
+ size_t n (al.end - al.begin);
+
+ for (appended_library& al1: d.ls)
+ {
+ if (al1.begin >= al.end)
+ {
+ al1.begin -= n;
+ al1.end -= n;
+ }
+ }
+
+ al.end = d.args.size ();
+ al.begin = al.end - n;
+ }
+
+ return;
+ }
+
if (l == nullptr)
{
// Don't try to link a library (whether -lfoo or foo.lib) to a
@@ -1542,7 +1598,7 @@ namespace build2
{
for (ptrdiff_t i (-1); lc[i] != nullptr; --i)
if (!lc[i]->is_a<libux> ())
- return;
+ goto done;
}
if (d.li.type == otype::a)
@@ -1558,10 +1614,10 @@ namespace build2
// care of by perform_update()). So we cut it off here.
//
if (!lu)
- return;
+ goto done;
if (l->mtime () == timestamp_unreal) // Binless.
- return;
+ goto done;
for (const target* pt: l->prerequisite_targets[d.a])
{
@@ -1593,7 +1649,7 @@ namespace build2
//
if (l->mtime () == timestamp_unreal) // Binless.
- return;
+ goto done;
// On Windows a shared library is a DLL with the import library as
// an ad hoc group member. MinGW though can link directly to DLLs
@@ -1622,13 +1678,16 @@ namespace build2
d.args.push_back ("-Wl,--whole-archive");
d.args.push_back (move (p));
d.args.push_back ("-Wl,--no-whole-archive");
- return;
+ goto done;
}
}
d.args.push_back (move (p));
}
}
+
+ done:
+ al.end = d.args.size (); // Close.
};
auto opt = [&d, this] (const file& l,
@@ -1649,6 +1708,11 @@ namespace build2
if (d.li.type == otype::a || !exp)
return;
+ // Suppress duplicates.
+ //
+ if (d.ls.append (l, d.args.size ()).end != appended_library::npos)
+ return;
+
// If we need an interface value, then use the group (lib{}).
//
if (const target* g = exp && l.is_a<libs> () ? l.group : &l)
@@ -1673,6 +1737,11 @@ namespace build2
const scope& bs, action a,
const file& l, bool la, lflags lf, linfo li) const
{
+ // Note that we don't do any duplicate suppression here: there is no way
+ // to "hoist" things once they are hashed and hashing only the first
+ // occurrence could miss changes to the command line (e.g., due to
+ // "hoisting").
+
struct data
{
sha256& cs;
@@ -1770,7 +1839,7 @@ namespace build2
}
void link_rule::
- rpath_libraries (strings& args,
+ rpath_libraries (rpathed_libraries& ls, strings& args,
const scope& bs,
action a, const file& l, bool la,
linfo li, bool link, bool self) const
@@ -1808,9 +1877,10 @@ namespace build2
//
struct
{
- strings& args;
- bool link;
- } d {args, link};
+ rpathed_libraries& ls;
+ strings& args;
+ bool link;
+ } d {ls, args, link};
auto lib = [&d, this] (const file* const* lc,
const string& f,
@@ -1832,6 +1902,16 @@ namespace build2
if (l->mtime () == timestamp_unreal) // Binless.
return;
+
+ // Suppress duplicates.
+ //
+ // We handle rpath similar to the compilation case by adding the
+ // options on the first occurrence and ignoring all the subsequent.
+ //
+ if (find (d.ls.begin (), d.ls.end (), l) != d.ls.end ())
+ return;
+
+ d.ls.push_back (l);
}
else
{
@@ -1882,7 +1962,10 @@ namespace build2
// It is either matched or imported so should be a cc library.
//
if (!cast_false<bool> (l.vars[c_system]))
+ {
args.push_back ("-Wl,-rpath," + l.path ().directory ().string ());
+ ls.push_back (&l);
+ }
}
}
@@ -1896,6 +1979,8 @@ namespace build2
const scope& bs, action a,
const target& t, linfo li, bool link) const
{
+ rpathed_libraries ls;
+
for (const prerequisite_target& pt: t.prerequisite_targets[a])
{
if (pt == nullptr)
@@ -1908,7 +1993,7 @@ namespace build2
(la = (f = pt->is_a<libux> ())) ||
( f = pt->is_a<libs> ()))
{
- rpath_libraries (args, bs, a, *f, la, li, link, true);
+ rpath_libraries (ls, args, bs, a, *f, la, li, link, true);
}
}
}
@@ -2862,35 +2947,42 @@ namespace build2
// inside append_libraries().
//
bool seen_obj (false);
- for (const prerequisite_target& p: t.prerequisite_targets[a])
{
- const target* pt (p.target);
+ appended_libraries als;
+ for (const prerequisite_target& p: t.prerequisite_targets[a])
+ {
+ const target* pt (p.target);
- if (pt == nullptr)
- continue;
+ if (pt == nullptr)
+ continue;
- if (modules)
- {
- if (pt->is_a<bmix> ()) // @@ MODHDR: hbmix{} has no objx{}
- pt = find_adhoc_member (*pt, tts.obj);
- }
+ if (modules)
+ {
+ if (pt->is_a<bmix> ()) // @@ MODHDR: hbmix{} has no objx{}
+ pt = find_adhoc_member (*pt, tts.obj);
+ }
- const file* f;
- bool la (false), ls (false);
+ const file* f;
+ bool la (false), ls (false);
- if ((f = pt->is_a<objx> ()) ||
- (!lt.utility &&
- (la = (f = pt->is_a<libux> ()))) ||
- (!lt.static_library () &&
- ((la = (f = pt->is_a<liba> ())) ||
- (ls = (f = pt->is_a<libs> ())))))
- {
- if (la || ls)
- append_libraries (sargs, bs, a, *f, la, p.data, li);
- else
+ if ((f = pt->is_a<objx> ()) ||
+ (!lt.utility &&
+ (la = (f = pt->is_a<libux> ()))) ||
+ (!lt.static_library () &&
+ ((la = (f = pt->is_a<liba> ())) ||
+ (ls = (f = pt->is_a<libs> ())))))
{
- sargs.push_back (relative (f->path ()).string ()); // string()&&
- seen_obj = true;
+ if (la || ls)
+ append_libraries (als, sargs, bs, a, *f, la, p.data, li);
+ else
+ {
+ // Do not hoist libraries over object files since such object
+ // files might satisfy symbols in the preceding libraries.
+ //
+ als.clear ();
+ sargs.push_back (relative (f->path ()).string ()); // string()&&
+ seen_obj = true;
+ }
}
}
}
diff --git a/libbuild2/cc/link-rule.hxx b/libbuild2/cc/link-rule.hxx
index a93defc..b369da6 100644
--- a/libbuild2/cc/link-rule.hxx
+++ b/libbuild2/cc/link-rule.hxx
@@ -54,8 +54,62 @@ namespace build2
public:
// Library handling.
//
+ struct appended_library
+ {
+ static const size_t npos = size_t (~0);
+
+ uintptr_t l; // Pointer to library target (last bit 0) or to
+ // library name (-lpthread or path; last bit 1).
+ size_t begin = npos; // First arg belonging to this library.
+ size_t end = npos; // Past last arg belonging to this library.
+ };
+
+ class appended_libraries: public small_vector<appended_library, 128>
+ {
+ public:
+ // Find existing or append new entry. If appending new, use the second
+ // argument as the begin value.
+ //
+ appended_library&
+ append (const file& l, size_t b)
+ {
+ auto p (reinterpret_cast<uintptr_t> (&l));
+ auto i (find_if (begin (), end (),
+ [p] (const appended_library& al)
+ {
+ return al.l == p;
+ }));
+
+ if (i != end ())
+ return *i;
+
+ push_back (appended_library {p, b, appended_library::npos});
+ return back ();
+ }
+
+ appended_library&
+ append (const string& l, size_t b)
+ {
+ auto i (find_if (begin (), end (),
+ [&l] (const appended_library& al)
+ {
+ return (al.l & 1) != 0 &&
+ l == *reinterpret_cast<const string*> (
+ al.l & ~uintptr_t (1));
+ }));
+
+ if (i != end ())
+ return *i;
+
+ push_back (
+ appended_library {
+ reinterpret_cast<uintptr_t> (&l) | 1, b, appended_library::npos});
+ return back ();
+ }
+ };
+
void
- append_libraries (strings&,
+ append_libraries (appended_libraries&, strings&,
const scope&, action,
const file&, bool, lflags, linfo, bool = true) const;
@@ -64,8 +118,10 @@ namespace build2
const scope&, action,
const file&, bool, lflags, linfo) const;
+ using rpathed_libraries = small_vector<const file*, 256>;
+
void
- rpath_libraries (strings&,
+ rpath_libraries (rpathed_libraries&, strings&,
const scope&,
action, const file&, bool, linfo, bool, bool) const;