Exported inner_product() reads past the end of its second argument (undocumented precondition)

[robertodr]

🤖 AI text below 🤖

What

inner_product iterates to v.size() while indexing into w, with no length precondition anywhere.

cpp/monoprop/MPFunctions.cpp:138-146

auto inner_product(const VecD &v, const VecD &w) -> double {
    const auto *v_data = v.data();
    const auto *w_data = w.data();
    double result = 0.0;
    for (size_t i = 0; i < v.size(); ++i) {
        result += v_data[i] * w_data[i];   // out of bounds if w.size() < v.size()
    }
    return result;
}

The declaration in cpp/include/monoprop/MPFunctions.h says nothing about it:

monoprop_EXPORT auto inner_product(const VecD &v, const VecD &w) -> double;

Why this is a problem

This is an exported symbol (monoprop_EXPORT) with an unstated, unchecked precondition. Every current
caller happens to satisfy it:

• EvalState::dot (dense arm) checks op.size() >= length_ first, and length_ == values_.size()
  for a dense state.
• ev_and_grad_impl:122 passes scratch.state and scratch.op, both sized to the operator's term
  count.

So there is no live bug today — but the invariant lives entirely in the callers, and neighbouring code
in the same file (EvalState::dot) does bounds-check and throw. The asymmetry is the hazard: the next
caller has nothing telling it the contract exists.

Suggested fix

Document the precondition on the declaration in MPFunctions.h, and add an assert(w.size() >= v.size()) in the definition. Assertions compile out under NDEBUG, so the hot path is untouched.

(If a Clang RelWithDebInfo build is in use, note the separate issue about that configuration
currently leaving assertions enabled.)

Verification

Builds and the existing suite are unaffected; the assert should be exercised by a debug-build run of
just test.

---

Found by a code-reading review of the repository at 29a8050. No build tree was available, so the
analysis is from source inspection and should be confirmed against a build.