From b7f4e6154fa3c07ad3472bbd7c871df28d440a64 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 23 Nov 2020 10:22:48 +0200 Subject: Suppress duplicates when extracting library options (GitHub issue #114) --- libbuild2/cc/link-rule.cxx | 162 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 127 insertions(+), 35 deletions(-) (limited to 'libbuild2/cc/link-rule.cxx') 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 ()) - 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 () ? 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 (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 ())) || ( f = pt->is_a ())) { - 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 ()) // @@ MODHDR: hbmix{} has no objx{} - pt = find_adhoc_member (*pt, tts.obj); - } + if (modules) + { + if (pt->is_a ()) // @@ 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 ()) || - (!lt.utility && - (la = (f = pt->is_a ()))) || - (!lt.static_library () && - ((la = (f = pt->is_a ())) || - (ls = (f = pt->is_a ()))))) - { - if (la || ls) - append_libraries (sargs, bs, a, *f, la, p.data, li); - else + if ((f = pt->is_a ()) || + (!lt.utility && + (la = (f = pt->is_a ()))) || + (!lt.static_library () && + ((la = (f = pt->is_a ())) || + (ls = (f = pt->is_a ()))))) { - 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; + } } } } -- cgit v1.1