commit 4eb1b10d893b8020315df7dd5215a087ebd39372 Author: Frank Ch. Eigler Date: Mon May 19 16:51:42 2025 -0400 PR32964 part 1: improve diagnostics of @defined resolution The @defined construct is special in that it is attempted to be resolved to literal 1/0 fairly early during semantic analysis pass-2, so that it can guard $context variable references. But this early processing is not that systematic: not all script level constructs properly nest with it. Those cases that don't happen to match end up resolved to 0 as a fallback in late during pass-2, due to PR18079. This patch adds verbosity diagnostics for some of these cases, and a warning message for the fallback resolution path. This triggers for the test case associated with this bugzilla report. diff --git a/dwflpp.cxx b/dwflpp.cxx index 1105272f2..62ae15837 100644 --- a/dwflpp.cxx +++ b/dwflpp.cxx @@ -4427,8 +4427,9 @@ dwflpp::literal_stmt_for_pointer (location_context &ctx, Dwarf_Die *die_mem) { if (sess.verbose>2) - clog << _F("literal_stmt_for_pointer: finding value for %s (%s)\n", - dwarf_type_name(start_typedie).c_str(), (dwarf_diename(cu) ?: "")); + clog << _("literal_stmt_for_pointer: finding value for ") << *e->tok + << _F(" type %s (%s)\n", + dwarf_type_name(start_typedie).c_str(), (dwarf_diename(cu) ?: "")); assert(ctx.pointer != NULL); location *tail = ctx.translate_argument (ctx.pointer); diff --git a/elaborate.cxx b/elaborate.cxx index 865fe6555..023b4c08f 100644 --- a/elaborate.cxx +++ b/elaborate.cxx @@ -1894,7 +1894,7 @@ semantic_pass_symbols (systemtap_session& s) assert_no_interrupts(); stapfile* dome = s.files[i]; - // Pass 1: add globals and functions to systemtap-session primart list, + // Pass 1: add globals and functions to systemtap-session primary list, // so the find_* functions find them // // NB: tapset global/function definitions may duplicate or conflict @@ -2796,6 +2796,8 @@ symresolution_info::visit_symbol (symbol* e) vardecl* d = find_var (e->name, 0, e->tok); if (d) { + if (session.verbose > 4) + clog << "resolved variable " << *e->tok << " to " << *d->tok << endl; e->referent = d; e->name = d->name; } @@ -2814,6 +2816,8 @@ symresolution_info::visit_symbol (symbol* e) // must be probe-condition expression throw SEMANTIC_ERROR (_("probe condition must not reference undeclared global"), e->tok); e->referent = v; + if (session.verbose > 4) + clog << "resolved variable " << *e->tok << " to new local" << endl; } } @@ -5112,6 +5116,7 @@ const_folder::visit_defined_op (defined_op* e) // Don't be greedy... we'll only collapse one at a time so type // resolution can have another go at it. relaxed_p = false; + session.print_warning (_F("Collapsing unresolved @define to %ld", value), e->tok); literal_number* n = new literal_number (value); n->tok = e->tok; n->visit (this); diff --git a/tapsets.cxx b/tapsets.cxx index d25c85043..01fec29e3 100644 --- a/tapsets.cxx +++ b/tapsets.cxx @@ -3118,9 +3118,22 @@ var_expanding_visitor::visit_defined_op (defined_op* e) target_symbol* tsym = dynamic_cast (e->operand); if (tsym && tsym->saved_conversion_error) // failing - resolved = false; + { + if (sess.verbose>3) + { + for (const semantic_error *c = tsym->saved_conversion_error; + c != 0; + c = c->get_chain()) { + clog << _("variable location problem [man error::dwarf]: ") << c->what() << endl; + } + } + resolved = false; + } else if (e->operand == old_operand) // unresolved but not marked failing { + if (sess.verbose>3) + clog << _("@defined unresolved due to un-rewritten operand ") << *e << endl; + // There are some visitors that won't touch certain target_symbols, // e.g. dwarf_var_expanding_visitor won't resolve @cast. We should // leave it for now so some other visitor can have a chance. @@ -3132,6 +3145,8 @@ var_expanding_visitor::visit_defined_op (defined_op* e) resolved = true; } catch (const semantic_error& e) { // some uncooperative value like @perf("NO_SUCH_VALUE") + if (sess.verbose > 3) + clog << sess.build_error_msg (e); resolved = false; } defined_ops.pop (); commit b36c59692966c6207bf6f69b2f6f91079f7b89ab Author: Frank Ch. Eigler Date: Mon May 19 22:30:29 2025 -0400 PR32964 part 2: document new @defined() warning ... in [man stapprobes], though [man stap] is where autocast variable processing is documented in the "TYPECASTING" section. diff --git a/elaborate.cxx b/elaborate.cxx index 023b4c08f..c09e3ddca 100644 --- a/elaborate.cxx +++ b/elaborate.cxx @@ -5116,7 +5116,7 @@ const_folder::visit_defined_op (defined_op* e) // Don't be greedy... we'll only collapse one at a time so type // resolution can have another go at it. relaxed_p = false; - session.print_warning (_F("Collapsing unresolved @define to %ld", value), e->tok); + session.print_warning (_F("Collapsing unresolved @define to %ld [stapprobes]", value), e->tok); literal_number* n = new literal_number (value); n->tok = e->tok; n->visit (this); diff --git a/man/stapprobes.3stap b/man/stapprobes.3stap index 1f980567b..d395e3578 100644 --- a/man/stapprobes.3stap +++ b/man/stapprobes.3stap @@ -700,6 +700,11 @@ for use in conditionals such as .SAMPLE @defined($foo\->bar) ? $foo\->bar : 0 .ESAMPLE +Use full $context variables or @cast() expressions in @define parameters +for unambiguous processing. The TYPECAST script-level variable +facility is incompletely supported. Simplify the parameters by reducing +nesting or indirection, if encountering "Collapsing unresolved @define to ..." +warnings. .TP @probewrite($VAR) see the PROBES section of \fIstap\fR(1). commit e4786671a999d47003ea62525ed829c1784c03ea Author: Frank Ch. Eigler Date: Thu May 22 17:14:53 2025 -0400 PR32964 part 3: include autocast processing in initial_typeres_pass This type resolution pass is done early, but previously purposefully ommitted opportunistic autocast expansion. With it done there, once (in the usual relaxation loop), more autocast-based @defined / @choose_defined constructs will be processed in the way a user may expect. diff --git a/elaborate.cxx b/elaborate.cxx index c09e3ddca..b03f0aced 100644 --- a/elaborate.cxx +++ b/elaborate.cxx @@ -2602,7 +2602,7 @@ semantic_pass (systemtap_session& s) if (rc == 0) rc = semantic_pass_symbols (s); if (rc == 0) monitor_mode_write (s); if (rc == 0) rc = semantic_pass_conditions (s); - if (rc == 0) rc = semantic_pass_optimize1 (s); + if (rc == 0) rc = semantic_pass_optimize1 (s); // includes const_fold and last ditch @defined() processing if (rc == 0) rc = semantic_pass_types (s); if (rc == 0) rc = gen_dfa_table(s); if (rc == 0) add_global_var_display (s); @@ -6045,7 +6045,7 @@ struct initial_typeresolution_info : public typeresolution_info static int initial_typeres_pass(systemtap_session& s) { // minimal type resolution based off of semantic_pass_types(), without - // checking for complete type resolutions or autocast expanding + // checking for complete type resolutions, PR32964 but including autocast expanding initial_typeresolution_info ti(s); ti.assert_resolvability = false; @@ -6067,6 +6067,30 @@ static int initial_typeres_pass(systemtap_session& s) ti.current_function = fd; ti.t = pe_unknown; fd->body->visit (& ti); + + // Check and run the autocast expanding visitor. + if (ti.num_available_autocasts > 0) + { + autocast_expanding_visitor aev (s, ti); + aev.replace (fd->body); + + // PR18079, rerun the const-folder / dead-block-remover + // if autocast evaluation enabled a @defined() + if (! aev.relaxed()) + { + bool relaxed_p = true; + const_folder cf (s, relaxed_p); + cf.replace (fd->body); + if (! s.unoptimized) + { + dead_control_remover dc (s, relaxed_p); + fd->body->visit (&dc); + } + (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag + } + + ti.num_available_autocasts = 0; + } } for (unsigned j=0; jbody->visit (& ti); + // Check and run the autocast expanding visitor. + if (ti.num_available_autocasts > 0) + { + autocast_expanding_visitor aev (s, ti); + var_expand_const_fold_loop (s, pn->body, aev); + // PR18079, rerun the const-folder / dead-block-remover + // if autocast evaluation enabled a @defined() + if (! s.unoptimized) + { + bool relaxed_p; + dead_control_remover dc (s, relaxed_p); + pn->body->visit (&dc); + (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag + } + + ti.num_available_autocasts = 0; + } + probe_point* pp = pn->sole_location(); if (pp->condition) { @@ -6147,30 +6189,8 @@ semantic_pass_types (systemtap_session& s) // ti.unresolved (fd->tok); for (unsigned i=0; i < fd->locals.size(); ++i) ti.check_local (fd->locals[i]); - - // Check and run the autocast expanding visitor. - if (ti.num_available_autocasts > 0) - { - autocast_expanding_visitor aev (s, ti); - aev.replace (fd->body); - - // PR18079, rerun the const-folder / dead-block-remover - // if autocast evaluation enabled a @defined() - if (! aev.relaxed()) - { - bool relaxed_p = true; - const_folder cf (s, relaxed_p); - cf.replace (fd->body); - if (! s.unoptimized) - { - dead_control_remover dc (s, relaxed_p); - fd->body->visit (&dc); - } - (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag - } - ti.num_available_autocasts = 0; - } + // PR32964: autocast resolution is now done early in initial_typeres_pass } catch (const semantic_error& e) { @@ -6191,24 +6211,8 @@ semantic_pass_types (systemtap_session& s) pn->body->visit (& ti); for (unsigned i=0; i < pn->locals.size(); ++i) ti.check_local (pn->locals[i]); - - // Check and run the autocast expanding visitor. - if (ti.num_available_autocasts > 0) - { - autocast_expanding_visitor aev (s, ti); - var_expand_const_fold_loop (s, pn->body, aev); - // PR18079, rerun the const-folder / dead-block-remover - // if autocast evaluation enabled a @defined() - if (! s.unoptimized) - { - bool relaxed_p; - dead_control_remover dc (s, relaxed_p); - pn->body->visit (&dc); - (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag - } - ti.num_available_autocasts = 0; - } + // PR32964: autocast resolution is now done early in initial_typeres_pass probe_point* pp = pn->sole_location(); if (pp->condition) commit 3122ff592bdfcf553afa6683724e90c359f3c5c9 Author: Frank Ch. Eigler Date: Fri May 30 17:50:10 2025 -0400 PR32964 part 4: let autocast pass propagate vardecl/symbol type_details The initial_typeres_pass now communicates relaxed status, and its autocast_expanding_visitor now passes vardecl type_details to the symbols. This way, the existing const_folder etc. visitors can deduce enough type info for @defined(an_autocast_var->foo->bar) constructs, and early enough, that @defined() works as intended. New testsuite/semok/definedautocast.stp tests a simple case. diff --git a/elaborate.cxx b/elaborate.cxx index b03f0aced..7a1609f1b 100644 --- a/elaborate.cxx +++ b/elaborate.cxx @@ -5163,7 +5163,7 @@ const_folder::visit_target_symbol (target_symbol* e) } } -static int initial_typeres_pass(systemtap_session& s); +static int initial_typeres_pass(systemtap_session& s, bool& relaxed_p); static int semantic_pass_const_fold (systemtap_session& s, bool& relaxed_p) { // attempt an initial type resolution pass to see if there are any type @@ -5171,7 +5171,7 @@ static int semantic_pass_const_fold (systemtap_session& s, bool& relaxed_p) // with a const. // return if the initial type resolution pass reported errors (type mismatches) - int rc = initial_typeres_pass(s); + int rc = initial_typeres_pass(s, relaxed_p); if (rc) { relaxed_p = true; @@ -5994,6 +5994,25 @@ struct autocast_expanding_visitor: public var_expanding_visitor } } + void visit_symbol (symbol* e) // propagate referent exp_type + { + // PR32964: mimic typeresolution_info::resolve_details, for case + // where the symbol (autocast_op operand) is within a @defined(). + + if (e->referent && e->referent->type_details && !e->type_details) + { + const exp_type_ptr &src = e->referent->type_details; + exp_type_ptr &dest = e->type_details; + dest = src; + ti.num_newly_resolved++; + relaxed_p = false; + if (sess.verbose > 4) + clog << "resolved early type details " << *dest << " to " << *e->tok << endl; + } + var_expanding_visitor::visit_symbol (e); + } + + void visit_autocast_op (autocast_op* e) { const bool lvalue = is_active_lvalue (e); @@ -6004,7 +6023,7 @@ struct autocast_expanding_visitor: public var_expanding_visitor if (fc) { ti.num_newly_resolved++; - + relaxed_p = false; resolve_functioncall (fc); // NB: at this stage, the functioncall object has one // argument too few if we're in lvalue context. It will @@ -6042,7 +6061,7 @@ struct initial_typeresolution_info : public typeresolution_info void visit_cast_op (cast_op*) {} }; -static int initial_typeres_pass(systemtap_session& s) +static int initial_typeres_pass(systemtap_session& s, bool& relaxed_p) { // minimal type resolution based off of semantic_pass_types(), without // checking for complete type resolutions, PR32964 but including autocast expanding @@ -6069,7 +6088,7 @@ static int initial_typeres_pass(systemtap_session& s) fd->body->visit (& ti); // Check and run the autocast expanding visitor. - if (ti.num_available_autocasts > 0) + if (true || ti.num_available_autocasts > 0) { autocast_expanding_visitor aev (s, ti); aev.replace (fd->body); @@ -6078,7 +6097,7 @@ static int initial_typeres_pass(systemtap_session& s) // if autocast evaluation enabled a @defined() if (! aev.relaxed()) { - bool relaxed_p = true; + // bool relaxed_p = true; const_folder cf (s, relaxed_p); cf.replace (fd->body); if (! s.unoptimized) @@ -6086,7 +6105,7 @@ static int initial_typeres_pass(systemtap_session& s) dead_control_remover dc (s, relaxed_p); fd->body->visit (&dc); } - (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag + // (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag } ti.num_available_autocasts = 0; @@ -6104,7 +6123,7 @@ static int initial_typeres_pass(systemtap_session& s) pn->body->visit (& ti); // Check and run the autocast expanding visitor. - if (ti.num_available_autocasts > 0) + if (true || ti.num_available_autocasts > 0) { autocast_expanding_visitor aev (s, ti); var_expand_const_fold_loop (s, pn->body, aev); @@ -6112,10 +6131,10 @@ static int initial_typeres_pass(systemtap_session& s) // if autocast evaluation enabled a @defined() if (! s.unoptimized) { - bool relaxed_p; + // bool relaxed_p; dead_control_remover dc (s, relaxed_p); pn->body->visit (&dc); - (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag + // (void) relaxed_p; // we judge success later by num_still_unresolved, not this flag } ti.num_available_autocasts = 0; diff --git a/testsuite/semok/definedautocast.stp b/testsuite/semok/definedautocast.stp new file mode 100755 index 000000000..b28be6118 --- /dev/null +++ b/testsuite/semok/definedautocast.stp @@ -0,0 +1,6 @@ +#! stap -Wp2 + +probe kernel.function("do_exit") { + x = & @cast(0, "struct block_device") + println(@defined(x->__bd_flags->counter)) +} commit 778ba67e356dc1820acc8ecdaa2c4b608c2eb184 Author: Frank Ch. Eigler Date: Fri May 30 22:10:36 2025 -0400 PR32964 part 5: tweak type inference propagation Because some type inference is now performed earlier than before, coupled with early const-folding (so dead code elimination), tests like semko/binexpr_infer_type.stp started passing with immediately prior code. That's because constructs like (foo*0) got quietly replaced by 0, even if foo was typed pe_string. This patch compensates for this change in behaviour by: - typeresolution_info::resolve_2types now propagates types more aggressively & recursively in its three-way inference, so that the unknown x string x long case is detected in one incoming invocation of the function, instead of being deferred to some unknown later relaxation-iteration round. - typeresolution_info::mismatch reporting errors, independent of "assert_resolvability". That flag was most likely meant for detection & rejection of pe_unknown types at the end of all the optimization stuff, not for ignoring outright known conflicts. diff --git a/elaborate.cxx b/elaborate.cxx index 7a1609f1b..d93a83eca 100644 --- a/elaborate.cxx +++ b/elaborate.cxx @@ -6683,21 +6683,21 @@ void resolve_2types (Referrer* referrer, Referent* referent, // propagate from upstream re_type = t; r->resolved (re_tok, re_type); - // catch re_type/te_type mismatch later + resolve_2types (referrer, referent, r, t, accept_unknown); } else if (re_type == pe_unknown && te_type != pe_unknown) { // propagate from referent re_type = te_type; r->resolved (re_tok, re_type); - // catch re_type/t mismatch later + resolve_2types (referrer, referent, r, t, accept_unknown); } else if (re_type != pe_unknown && te_type == pe_unknown) { // propagate to referent te_type = re_type; r->resolved (re_tok, re_type, referent); - // catch re_type/t mismatch later + resolve_2types (referrer, referent, r, t, accept_unknown); } else if (! accept_unknown) r->unresolved (re_tok); @@ -7636,14 +7636,14 @@ typeresolution_info::mismatch (const binary_expression* e) { num_still_unresolved ++; - if (assert_resolvability && mismatch_complexity <= 1) + if (mismatch_complexity <= 1) { stringstream msg; msg << _F("type mismatch: left and right sides don't agree (%s vs %s)", lex_cast(e->left->type).c_str(), lex_cast(e->right->type).c_str()); session.print_error (SEMANTIC_ERROR (msg.str(), e->tok)); } - else if (!assert_resolvability) + else mismatch_complexity = max(1, mismatch_complexity); } @@ -7656,7 +7656,7 @@ typeresolution_info::mismatch (const token* tok, exp_type t1, exp_type t2) { num_still_unresolved ++; - if (assert_resolvability && mismatch_complexity <= 2) + if (mismatch_complexity <= 2) { stringstream msg; msg << _F("type mismatch: expected %s", lex_cast(t1).c_str()); @@ -7664,7 +7664,7 @@ typeresolution_info::mismatch (const token* tok, exp_type t1, exp_type t2) msg << _F(" but found %s", lex_cast(t2).c_str()); session.print_error (SEMANTIC_ERROR (msg.str(), tok)); } - else if (!assert_resolvability) + else mismatch_complexity = max(2, mismatch_complexity); } @@ -7679,7 +7679,7 @@ typeresolution_info::mismatch (const token *tok, exp_type type, { num_still_unresolved ++; - if (assert_resolvability && mismatch_complexity <= 3) + if (mismatch_complexity <= 3) { assert(decl != NULL); @@ -7737,7 +7737,7 @@ typeresolution_info::mismatch (const token *tok, exp_type type, err.set_chain(chain); session.print_error (err); } - else if (!assert_resolvability) + else mismatch_complexity = max(3, mismatch_complexity); } commit fd4322e38d1e6504341b67329e112ccc1af387b6 Author: Frank Ch. Eigler Date: Sat May 31 09:15:33 2025 -0400 PR32964 part 6: NEWS & old testcase It turns out there was one old testcase that relied on type non-checking upon elided variables. This test has been removed, and the change mentioned in NEWS. Unfortunately, this is is hard to make conditional on --compatible=XXXX, so hypothetical old (arguably buggy) scripts may get hit. diff --git a/NEWS b/NEWS index 01b74a671..f1d78e8fa 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,11 @@ +* What's new in version 5.4 + +- Type checking and autocast processing have been made more thorough, + so elided variables are checked more and @defined() tests may be + more complicated. Preexisting scripts that rely on elision for + bypassing type violations may now get caught. No --compatible + option exists to suppress this new behaviour. + * What's new in version 5.3, 2025-05-02 - Updated to support drastic linux 6.13 kbuild changes, numerous diff --git a/testsuite/semok/pr30570.stp b/testsuite/semok/pr30570.stp index b711b214c..dda7d027c 100755 --- a/testsuite/semok/pr30570.stp +++ b/testsuite/semok/pr30570.stp @@ -24,13 +24,16 @@ probe oneshot { } # error var and the try block are elided which means -# it's not an issue to use e4 as a long here -global e4 = 10 -probe oneshot { - try { } - catch (e4) - { } -} +# it's not an issue to use e4 as a long here ... +# except after PR32964, type checking is done more +# systematically on even elided variables, so this +# would be an error! +# global e4 = 10 +# probe oneshot { +# try { } +# catch (e4) +# { } +# } # e5 can be a global string global e5 = "foo"