From 3925ff459203333ef2d80fe167272cf075f5557a Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Tue, 15 Aug 2023 16:07:49 -0700 Subject: [PATCH] addressed performance and correctness issues flagged by Coverity --- src/Expr.cc | 2 +- src/Func.cc | 6 +++--- src/Stmt.cc | 2 +- src/Stmt.h | 2 +- src/parse.y | 2 +- src/script_opt/Expr.cc | 17 +++++++++-------- src/script_opt/Stmt.cc | 2 +- src/script_opt/ZAM/Ops.in | 24 ++++++++++++------------ src/script_opt/ZAM/Vars.cc | 4 +++- 9 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index d001706196..af0cdb22f3 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4649,7 +4649,7 @@ LambdaExpr::LambdaExpr(FunctionIngredientsPtr arg_ing, IDPList arg_outer_ids, st auto ingr_t = ingredients->GetID()->GetType(); SetType(ingr_t); - if ( ! CheckCaptures(when_parent) ) + if ( ! CheckCaptures(std::move(when_parent)) ) { SetError(); return; diff --git a/src/Func.cc b/src/Func.cc index 5642b96775..885a4333cd 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -142,7 +142,7 @@ void Func::AddBody(detail::StmtPtr new_body, size_t new_frame_size) { std::vector no_inits; std::set no_groups; - AddBody(new_body, no_inits, new_frame_size, 0, no_groups); + AddBody(std::move(new_body), no_inits, new_frame_size, 0, no_groups); } void Func::AddBody(detail::StmtPtr /* new_body */, @@ -588,7 +588,7 @@ void ScriptFunc::CreateCaptures(std::unique_ptr> cvec) if ( c_i.IsManaged() ) ZVal::DeleteManagedType(cv_i); - cv_i = ZVal(new_cv_i, t); + cv_i = ZVal(std::move(new_cv_i), t); } } } @@ -725,7 +725,7 @@ FuncPtr ScriptFunc::DoClone() // Need to clone cv_i. auto& t_i = c.Id()->GetType(); auto cv_i_val = cv_i->ToVal(t_i)->Clone(); - other->captures_vec->push_back(ZVal(cv_i_val, t_i)); + other->captures_vec->push_back(ZVal(std::move(cv_i_val), t_i)); ++cv_i; } } diff --git a/src/Stmt.cc b/src/Stmt.cc index 5acacbc1f9..6d3abdc963 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -2079,7 +2079,7 @@ void WhenInfo::BuildProfile() if ( ! is_present ) { IDPtr wl_ptr = {NewRef{}, const_cast(wl)}; - cl->emplace_back(wl_ptr, false); + cl->emplace_back(std::move(wl_ptr), false); } // In addition, don't treat them as external locals that diff --git a/src/Stmt.h b/src/Stmt.h index 7829165f20..4e73da5728 100644 --- a/src/Stmt.h +++ b/src/Stmt.h @@ -647,7 +647,7 @@ private: StmtPtr s; StmtPtr timeout_s; ExprPtr timeout; - FuncType::CaptureList* cl; + FuncType::CaptureList* cl = nullptr; bool is_return = false; diff --git a/src/parse.y b/src/parse.y index 367c116831..94e6f524e3 100644 --- a/src/parse.y +++ b/src/parse.y @@ -1935,7 +1935,7 @@ stmt: | when_clause { std::shared_ptr wi($1); - $$ = new WhenStmt(wi); + $$ = new WhenStmt(std::move(wi)); script_coverage_mgr.AddStmt($$); } diff --git a/src/script_opt/Expr.cc b/src/script_opt/Expr.cc index 427f719b1b..3e8a71ce90 100644 --- a/src/script_opt/Expr.cc +++ b/src/script_opt/Expr.cc @@ -309,7 +309,7 @@ StmtPtr Expr::ReduceToSingletons(Reducer* c) if ( op3 && ! op3->IsSingleton(c) ) SetOp3(op3->ReduceToSingleton(c, red3_stmt)); - return MergeStmts(red1_stmt, red2_stmt, red3_stmt); + return MergeStmts(std::move(red1_stmt), std::move(red2_stmt), std::move(red3_stmt)); } ExprPtr Expr::ReduceToConditional(Reducer* c, StmtPtr& red_stmt) @@ -354,7 +354,7 @@ ExprPtr Expr::ReduceToConditional(Reducer* c, StmtPtr& red_stmt) { auto red2_stmt = ReduceToSingletons(c); auto res = ReduceToConditional(c, red_stmt); - red_stmt = MergeStmts(red2_stmt, red_stmt); + red_stmt = MergeStmts(std::move(red2_stmt), red_stmt); return res; } @@ -611,7 +611,7 @@ ExprPtr BinaryExpr::Reduce(Reducer* c, StmtPtr& red_stmt) if ( ! op2->IsSingleton(c) ) op2 = op2->ReduceToSingleton(c, red2_stmt); - red_stmt = MergeStmts(red_stmt, red2_stmt); + red_stmt = MergeStmts(red_stmt, std::move(red2_stmt)); auto op1_fold_val = op1->FoldVal(); auto op2_fold_val = op2->FoldVal(); @@ -1528,7 +1528,8 @@ StmtPtr CondExpr::ReduceToSingletons(Reducer* c) if ( ! red3_stmt ) red3_stmt = make_intrusive(); - if_else = make_intrusive(op1->Duplicate(), red2_stmt, red3_stmt); + if_else = make_intrusive(op1->Duplicate(), std::move(red2_stmt), + std::move(red3_stmt)); } return MergeStmts(red1_stmt, if_else); @@ -1897,7 +1898,7 @@ StmtPtr IndexExpr::ReduceToSingletons(Reducer* c) StmtPtr red2_stmt = op2->ReduceToSingletons(c); - return MergeStmts(red1_stmt, red2_stmt); + return MergeStmts(red1_stmt, std::move(red2_stmt)); } ExprPtr IndexExprWhen::Duplicate() @@ -1927,7 +1928,7 @@ ExprPtr HasFieldExpr::Reduce(Reducer* c, StmtPtr& red_stmt) if ( ! op->GetType()->FieldHasAttr(field, ATTR_OPTIONAL) ) { auto true_constant = make_intrusive(val_mgr->True()); - return TransformMe(true_constant, c, red_stmt); + return TransformMe(std::move(true_constant), c, red_stmt); } return UnaryExpr::Reduce(c, red_stmt); @@ -2334,7 +2335,7 @@ ExprPtr ScheduleExpr::Reduce(Reducer* c, StmtPtr& red_stmt) // We assume that EventExpr won't transform itself fundamentally. (void)event->Reduce(c, red2_stmt); - red_stmt = MergeStmts(red_stmt, red2_stmt); + red_stmt = MergeStmts(red_stmt, std::move(red2_stmt)); return ThisPtr(); } @@ -2419,7 +2420,7 @@ ExprPtr CallExpr::Reduce(Reducer* c, StmtPtr& red_stmt) // ### could check here for (1) pure function, and (2) all // arguments constants, and call it to fold right now. - red_stmt = MergeStmts(red_stmt, red2_stmt); + red_stmt = MergeStmts(red_stmt, std::move(red2_stmt)); if ( GetType()->Tag() == TYPE_VOID ) return ThisPtr(); diff --git a/src/script_opt/Stmt.cc b/src/script_opt/Stmt.cc index e6084138c0..fc042cdaf6 100644 --- a/src/script_opt/Stmt.cc +++ b/src/script_opt/Stmt.cc @@ -1012,7 +1012,7 @@ StmtPtr WhenStmt::DoReduce(Reducer* c) if ( red_e_stmt ) { auto s = make_intrusive(red_e_stmt, ThisPtr()); - return TransformMe(s, c); + return TransformMe(std::move(s), c); } } diff --git a/src/script_opt/ZAM/Ops.in b/src/script_opt/ZAM/Ops.in index 96e3e1c9d0..0747e8f00f 100644 --- a/src/script_opt/ZAM/Ops.in +++ b/src/script_opt/ZAM/Ops.in @@ -752,7 +752,7 @@ macro EvalVal2InTableCore(op1, op2) ListValPtr lvp = {NewRef{}, &lv}; macro EvalVal2InTableAssignCore(slot) - frame[z.v1].int_val = frame[z.slot].table_val->Find(lvp) != nullptr; + frame[z.v1].int_val = frame[z.slot].table_val->Find(std::move(lvp)) != nullptr; macro EvalVal2InTablePre(op1, op2, op3) auto& tt_ind = frame[z.op3].table_val->GetType()->AsTableType()->GetIndexTypes(); @@ -838,12 +838,12 @@ eval auto op1 = z.c.ToVal(z.t); internal-op List-Is-In-Table type VV eval auto op1 = z.aux->ToListVal(frame); - frame[z.v1].int_val = frame[z.v2].table_val->Find(op1) != nullptr; + frame[z.v1].int_val = frame[z.v2].table_val->Find(std::move(op1)) != nullptr; internal-op List-Is-In-Table type VC eval auto op1 = z.aux->ToListVal(frame); - frame[z.v1].int_val = z.c.table_val->Find(op1) != nullptr; + frame[z.v1].int_val = z.c.table_val->Find(std::move(op1)) != nullptr; internal-op Val-Is-In-Vector type VVV @@ -918,7 +918,7 @@ macro EvalIndexVecBoolSelect(op1, op2) auto vt = cast_intrusive(z.t); auto v2 = op1.vector_val; auto v3 = op2.vector_val; - auto v = vector_bool_select(vt, v2, v3); + auto v = vector_bool_select(std::move(vt), v2, v3); Unref(frame[z.v1].vector_val); frame[z.v1].vector_val = v.release(); @@ -936,7 +936,7 @@ macro EvalIndexVecIntSelect(op1, op2) auto vt = cast_intrusive(z.t); auto v2 = op1.vector_val; auto v3 = op2.vector_val; - auto v = vector_int_select(vt, v2, v3); + auto v = vector_int_select(std::move(vt), v2, v3); Unref(frame[z.v1].vector_val); frame[z.v1].vector_val = v.release(); @@ -1136,7 +1136,7 @@ type VV op1-read eval auto& tbl = frame[z.v1].table_val; auto lambda = frame[z.v2].ToVal(z.t); - tbl->InitDefaultVal(lambda); + tbl->InitDefaultVal(std::move(lambda)); direct-unary-op Set-Constructor ConstructSet @@ -1294,7 +1294,7 @@ internal-op Record-Coerce type VV eval auto rt = cast_intrusive(z.t); auto v = frame[z.v2].record_val; - auto to_r = coerce_to_record(rt, v, z.aux->map); + auto to_r = coerce_to_record(std::move(rt), v, z.aux->map); Unref(frame[z.v1].record_val); frame[z.v1].record_val = to_r.release(); @@ -1426,7 +1426,7 @@ eval ValPtr vec = {NewRef{}, frame[z.v1].vector_val}; auto slice = z.aux->ToListVal(frame); ValPtr vals = {NewRef{}, frame[z.v2].vector_val}; bool iterators_invalidated; - auto error = assign_to_index(vec, slice, vals, iterators_invalidated); + auto error = assign_to_index(std::move(vec), std::move(slice), std::move(vals), iterators_invalidated); if ( error ) ZAM_run_time_error(z.loc, error); if ( iterators_invalidated ) @@ -1442,7 +1442,7 @@ macro EvalTableElemAssign(value) auto indices = z.aux->ToListVal(frame); auto val = value.ToVal(z.t); bool iterators_invalidated = false; - frame[z.v1].table_val->Assign(indices, val, true, &iterators_invalidated); + frame[z.v1].table_val->Assign(std::move(indices), std::move(val), true, &iterators_invalidated); if ( iterators_invalidated ) ZAM_run_time_warning(z.loc, "possible loop/iterator invalidation"); @@ -1999,7 +1999,7 @@ method-post z.aux = v->aux; macro EvalAddStmt(ind) auto index = ind; bool iterators_invalidated = false; - frame[z.v1].table_val->Assign(index, nullptr, true, &iterators_invalidated); + frame[z.v1].table_val->Assign(std::move(index), nullptr, true, &iterators_invalidated); if ( iterators_invalidated ) ZAM_run_time_warning(z.loc, "possible loop/iterator invalidation"); @@ -2040,7 +2040,7 @@ eval auto r = new RecordVal(cast_intrusive(z.t)); internal-op Init-Vector type V eval auto vt = cast_intrusive(z.t); - auto vec = new VectorVal(vt); + auto vec = new VectorVal(std::move(vt)); Unref(frame[z.v1].vector_val); frame[z.v1].vector_val = vec; @@ -2071,7 +2071,7 @@ macro BuildWhen(timeout) auto wi = aux->wi; FuncPtr func{NewRef{}, frame[z.v1].func_val}; auto lambda = make_intrusive(func); - wi->Instantiate(lambda); + wi->Instantiate(std::move(lambda)); std::vector local_aggrs; for ( int i = 0; i < aux->n; ++i ) { diff --git a/src/script_opt/ZAM/Vars.cc b/src/script_opt/ZAM/Vars.cc index 61460d3cbd..1440320c94 100644 --- a/src/script_opt/ZAM/Vars.cc +++ b/src/script_opt/ZAM/Vars.cc @@ -32,7 +32,9 @@ bool ZAMCompiler::IsCapture(const ID* id) const int ZAMCompiler::CaptureOffset(const ID* id) const { - return pf->CapturesOffsets().find(id)->second; + auto id_offset = pf->CapturesOffsets().find(id); + ASSERT(id_offset != pf->CapturesOffsets().end()); + return id_offset->second; } void ZAMCompiler::LoadParam(const ID* id)