From f7f22db6030464f63eb942da04b3d5e10351f770 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 2 Nov 2022 09:56:31 +0200 Subject: More work on child process diagnostics buffering --- libbuild2/bin/def-rule.cxx | 158 ++++++++++++++++++++++++++++++++++----------- libbuild2/bin/guess.cxx | 42 +++++++----- libbuild2/bin/guess.hxx | 8 +-- libbuild2/bin/init.cxx | 14 ++-- 4 files changed, 157 insertions(+), 65 deletions(-) (limited to 'libbuild2/bin') diff --git a/libbuild2/bin/def-rule.cxx b/libbuild2/bin/def-rule.cxx index 421074c..257fad8 100644 --- a/libbuild2/bin/def-rule.cxx +++ b/libbuild2/bin/def-rule.cxx @@ -33,8 +33,10 @@ namespace build2 }; static void - read_dumpbin (istream& is, symbols& syms) + read_dumpbin (diag_buffer& dbuf, ifdstream& is, symbols& syms) { + // Note: io_error is handled by the caller. + // Lines that describe symbols look like: // // 0 1 2 3 4 5 6 @@ -72,29 +74,27 @@ namespace build2 // // Note that an UNDEF data symbol with non-zero OFFSET is a "common // symbol", equivalent to the nm `C` type. - - // Map of read-only (.rdata, .xdata) and uninitialized (.bss) sections - // to their types (R and B, respectively). If a section is not found in - // this map, then it's assumed to be normal data (.data). // - map sections; - - string l; - while (!eof (getline (is, l))) + // We keep a map of read-only (.rdata, .xdata) and uninitialized (.bss) + // sections to their types (R and B, respectively). If a section is not + // found in this map, then it's assumed to be normal data (.data). + // + auto parse_line = [&syms, + secs = map ()] (const string& l) mutable { size_t b (0), e (0), n; // IDX (note that it can be more than 3 characters). // if (next_word (l, b, e) == 0) - continue; + return; // OFFSET (always 8 characters). // n = next_word (l, b, e); if (n != 8) - continue; + return; string off (l, b, n); @@ -103,7 +103,7 @@ namespace build2 n = next_word (l, b, e); if (n == 0) - continue; + return; string sec (l, b, n); @@ -112,7 +112,7 @@ namespace build2 n = next_word (l, b, e); if (l.compare (b, n, "notype") != 0) - continue; + return; bool dat; if (l[e] == ' ' && l[e + 1] == '(' && l[e + 2] == ')') @@ -128,7 +128,7 @@ namespace build2 n = next_word (l, b, e); if (n == 0) - continue; + return; string vis (l, b, n); @@ -137,14 +137,14 @@ namespace build2 n = next_word (l, b, e); if (n != 1 || l[b] != '|') - continue; + return; // SYMNAME // n = next_word (l, b, e); if (n == 0) - continue; + return; string s (l, b, n); @@ -162,23 +162,23 @@ namespace build2 }; if (cmp (".rdata", 6) || - cmp (".xdata", 6)) sections.emplace (move (sec), 'R'); - else if (cmp (".bss", 4)) sections.emplace (move (sec), 'B'); + cmp (".xdata", 6)) secs.emplace (move (sec), 'R'); + else if (cmp (".bss", 4)) secs.emplace (move (sec), 'B'); - continue; + return; } // We can only export extern symbols. // if (vis != "External") - continue; + return; if (dat) { if (sec != "UNDEF") { - auto i (sections.find (sec)); - switch (i == sections.end () ? 'D' : i->second) + auto i (secs.find (sec)); + switch (i == secs.end () ? 'D' : i->second) { case 'D': syms.d.insert (move (s)); break; case 'R': syms.r.insert (move (s)); break; @@ -196,20 +196,54 @@ namespace build2 if (sec != "UNDEF") syms.t.insert (move (s)); } + }; + + // Read until we reach EOF on all streams. + // + // Note that if dbuf is not opened, then we automatically get inactive + // nullfd entry. + // + fdselect_set fds {is.fd (), dbuf.is.fd ()}; + fdselect_state& ist (fds[0]); + fdselect_state& dst (fds[1]); + + for (string l; ist.fd != nullfd || dst.fd != nullfd; ) + { + if (ist.fd != nullfd && getline_non_blocking (is, l)) + { + if (eof (is)) + ist.fd = nullfd; + else + { + parse_line (l); + l.clear (); + } + + continue; + } + + ifdselect (fds); + + if (dst.ready) + { + if (!dbuf.read ()) + dst.fd = nullfd; + } } } static void - read_posix_nm (istream& is, symbols& syms) + read_posix_nm (diag_buffer& dbuf, ifdstream& is, symbols& syms) { + // Note: io_error is handled by the caller. + // Lines that describe symbols look like: // // // // The types that we are interested in are T, D, R, and B. // - string l; - while (!eof (getline (is, l))) + auto parse_line = [&syms] (const string& l) { size_t b (0), e (0), n; @@ -218,7 +252,7 @@ namespace build2 n = next_word (l, b, e); if (n == 0) - continue; + return; string s (l, b, n); @@ -227,7 +261,7 @@ namespace build2 n = next_word (l, b, e); if (n != 1) - continue; + return; switch (l[b]) { @@ -238,6 +272,39 @@ namespace build2 case 'C': syms.c.insert (move (s)); break; case 'T': syms.t.insert (move (s)); break; } + }; + + // Read until we reach EOF on all streams. + // + // Note that if dbuf is not opened, then we automatically get inactive + // nullfd entry. + // + fdselect_set fds {is.fd (), dbuf.is.fd ()}; + fdselect_state& ist (fds[0]); + fdselect_state& dst (fds[1]); + + for (string l; ist.fd != nullfd || dst.fd != nullfd; ) + { + if (ist.fd != nullfd && getline_non_blocking (is, l)) + { + if (eof (is)) + ist.fd = nullfd; + else + { + parse_line (l); + l.clear (); + } + + continue; + } + + ifdselect (fds); + + if (dst.ready) + { + if (!dbuf.read ()) + dst.fd = nullfd; + } } } @@ -656,6 +723,8 @@ namespace build2 if (verb == 1) text << "def " << t; + diag_buffer dbuf (ctx); + // Extract symbols from each object file. // symbols syms; @@ -674,22 +743,37 @@ namespace build2 // Both dumpbin.exe and nm send their output to stdout. While nm sends // diagnostics to stderr, dumpbin sends it to stdout together with the - // output. + // output. To keep things uniform we will buffer stderr in both cases. // - process pr (run_start (nm, - args, - 0 /* stdin */, - -1 /* stdout */)); + process pr ( + run_start (nm, + args, + 0 /* stdin */, + -1 /* stdout */, + dbuf.open (args[0], + false /* force */, + fdstream_mode::non_blocking | + fdstream_mode::skip) /* stderr */)); + bool io (false); try { - ifdstream is ( - move (pr.in_ofd), fdstream_mode::skip, ifdstream::badbit); + // Note that while we read both streams until eof in the normal + // circumstances, we cannot use fdstream_mode::skip for the + // exception case on both of them: we may end up being blocked + // trying to read one stream while the process may be blocked + // writing to the other. So in case of an exception we only skip the + // diagnostics and close stdout hard. The latter should happen first + // so the order of the dbuf/is variables is important. + // + ifdstream is (move (pr.in_ofd), + fdstream_mode::non_blocking, + ifdstream::badbit); if (lid == "msvc" || nid == "msvc") - read_dumpbin (is, syms); + read_dumpbin (dbuf, is, syms); else - read_posix_nm (is, syms); + read_posix_nm (dbuf, is, syms); is.close (); } @@ -701,7 +785,7 @@ namespace build2 io = true; } - if (!run_finish_code (args.data (), pr) || io) + if (!run_finish_code (dbuf, args, pr) || io) fail << "unable to extract symbols from " << arg; } diff --git a/libbuild2/bin/guess.cxx b/libbuild2/bin/guess.cxx index e935913..e9759b8 100644 --- a/libbuild2/bin/guess.cxx +++ b/libbuild2/bin/guess.cxx @@ -92,7 +92,7 @@ namespace build2 static global_cache ar_cache; const ar_info& - guess_ar (const path& ar, const path* rl, const char* paths) + guess_ar (context& ctx, const path& ar, const path* rl, const char* paths) { tracer trace ("bin::guess_ar"); @@ -234,7 +234,11 @@ namespace build2 // (yes, it goes to stdout) but that seems harmless. // sha256 cs; - arr = run (3, are, "--version", f, false, false, &cs); + arr = run (ctx, + 3, + are, "--version", + f, + false , false, &cs); if (!arr.empty ()) arr.checksum = cs.string (); @@ -254,10 +258,10 @@ namespace build2 : guess_result (); }; - // Redirect STDERR to STDOUT and ignore exit status. + // Redirect stderr to stdout and ignore exit status. // sha256 cs; - arr = run (3, are, f, false, true, &cs); + arr = run (ctx, 3, are, f, false, true, &cs); if (!arr.empty ()) { @@ -300,7 +304,11 @@ namespace build2 }; sha256 cs; - rlr = run (3, rle, "--version", f, false, false, &cs); + rlr = run (ctx, + 3, + rle, "--version", + f, + false, false, &cs); if (!rlr.empty ()) rlr.checksum = cs.string (); @@ -317,10 +325,10 @@ namespace build2 : guess_result (); }; - // Redirect STDERR to STDOUT and ignore exit status. + // Redirect stderr to stdout and ignore exit status. // sha256 cs; - rlr = run (3, rle, f, false, true, &cs); + rlr = run (ctx, 3, rle, f, false, true, &cs); if (!rlr.empty ()) { @@ -385,7 +393,7 @@ namespace build2 static global_cache ld_cache; const ld_info& - guess_ld (const path& ld, const char* paths) + guess_ld (context& ctx, const path& ld, const char* paths) { tracer trace ("bin::guess_ld"); @@ -497,12 +505,12 @@ namespace build2 : guess_result (move (id), move (l), move (ver))); }; - // Redirect STDERR to STDOUT and ignore exit status. Note that in case + // Redirect stderr to stdout and ignore exit status. Note that in case // of link.exe we will hash the diagnostics (yes, it goes to stdout) // but that seems harmless. // sha256 cs; - r = run (3, env, "--version", f, false, true, &cs); + r = run (ctx, 3, env, "--version", f, false, true, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -533,7 +541,7 @@ namespace build2 }; sha256 cs; - r = run (3, env, "-v", f, false, false, &cs); + r = run (ctx, 3, env, "-v", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -560,7 +568,7 @@ namespace build2 // option. // sha256 cs; - r = run (3, env, "-version", f, false, false, &cs); + r = run (ctx, 3, env, "-version", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -598,7 +606,7 @@ namespace build2 static global_cache rc_cache; const rc_info& - guess_rc (const path& rc, const char* paths) + guess_rc (context& ctx, const path& rc, const char* paths) { tracer trace ("bin::guess_rc"); @@ -654,7 +662,7 @@ namespace build2 // option. // sha256 cs; - r = run (3, env, "--version", f, false, false, &cs); + r = run (ctx, 3, env, "--version", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -687,7 +695,7 @@ namespace build2 }; sha256 cs; - r = run (3, env, "/?", f, false, false, &cs); + r = run (ctx, 3, env, "/?", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); @@ -715,7 +723,7 @@ namespace build2 static global_cache nm_cache; const nm_info& - guess_nm (const path& nm, const char* paths) + guess_nm (context& ctx, const path& nm, const char* paths) { tracer trace ("bin::guess_nm"); @@ -799,7 +807,7 @@ namespace build2 // option. // sha256 cs; - r = run (3, env, "--version", f, false, false, &cs); + r = run (ctx, 3, env, "--version", f, false, false, &cs); if (!r.empty ()) r.checksum = cs.string (); diff --git a/libbuild2/bin/guess.hxx b/libbuild2/bin/guess.hxx index 52c0e1b..7dc7b33 100644 --- a/libbuild2/bin/guess.hxx +++ b/libbuild2/bin/guess.hxx @@ -54,7 +54,7 @@ namespace build2 // attemplated and the returned ranlib_* members will be left empty. // const ar_info& - guess_ar (const path& ar, const path* ranlib, const char* paths); + guess_ar (context&, const path& ar, const path* ranlib, const char* paths); // ld information. // @@ -100,7 +100,7 @@ namespace build2 }; const ld_info& - guess_ld (const path& ld, const char* paths); + guess_ld (context&, const path& ld, const char* paths); // rc information. // @@ -132,7 +132,7 @@ namespace build2 }; const rc_info& - guess_rc (const path& rc, const char* paths); + guess_rc (context&, const path& rc, const char* paths); // nm information. // @@ -166,7 +166,7 @@ namespace build2 }; const nm_info& - guess_nm (const path& nm, const char* paths); + guess_nm (context&, const path& nm, const char* paths); } } diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx index 4b6090a..d7c5b5a 100644 --- a/libbuild2/bin/init.cxx +++ b/libbuild2/bin/init.cxx @@ -236,9 +236,9 @@ namespace build2 // if (!hint && config_sub) { - s = run (3, - *config_sub, - s.c_str (), + s = run (ctx, + 3, + *config_sub, s.c_str (), [] (string& l, bool) {return move (l);}); l5 ([&]{trace << "config.sub target: '" << s << "'";}); } @@ -692,7 +692,7 @@ namespace build2 nullptr, config::save_default_commented))); - const ar_info& ari (guess_ar (ar, ranlib, pat.paths)); + const ar_info& ari (guess_ar (rs.ctx, ar, ranlib, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). @@ -843,7 +843,7 @@ namespace build2 path (apply_pattern (ld_d, pat.pattern)), config::save_default_commented))); - const ld_info& ldi (guess_ld (ld, pat.paths)); + const ld_info& ldi (guess_ld (rs.ctx, ld, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). @@ -1004,7 +1004,7 @@ namespace build2 path (apply_pattern (rc_d, pat.pattern)), config::save_default_commented))); - const rc_info& rci (guess_rc (rc, pat.paths)); + const rc_info& rci (guess_rc (rs.ctx, rc, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). @@ -1116,7 +1116,7 @@ namespace build2 path (apply_pattern (nm_d, pat.pattern)), config::save_default_commented))); - const nm_info& nmi (guess_nm (nm, pat.paths)); + const nm_info& nmi (guess_nm (rs.ctx, nm, pat.paths)); // If this is a configuration with new values, then print the report // at verbosity level 2 and up (-v). -- cgit v1.1