Skip to content

Add size check on partial_ops x input vector#34595

Open
mangguo321 wants to merge 4 commits intoopenvinotoolkit:masterfrom
mangguo321:mang/fix_paddle_partial_ops
Open

Add size check on partial_ops x input vector#34595
mangguo321 wants to merge 4 commits intoopenvinotoolkit:masterfrom
mangguo321:mang/fix_paddle_partial_ops

Conversation

@mangguo321
Copy link
Contributor

Details:

  • Add input count validation in partial_ops() to prevent out-of-bounds access

Tickets:

@mangguo321 mangguo321 requested a review from a team as a code owner March 10, 2026 02:45
@github-actions github-actions bot added the category: PDPD FE OpenVINO PaddlePaddle FrontEnd label Mar 10, 2026
@yuxu42 yuxu42 requested a review from Copilot March 10, 2026 03:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds defensive input-count validation to the Paddle frontend partial_ops() converter to avoid out-of-bounds access when the "X" input list is malformed or incomplete.

Changes:

  • Validate that "X" contains exactly 2 inputs before indexing x[0] / x[1] in partial_ops().

You can also share your feedback on Copilot code review. Take the survey.


NamedOutputs partial_ops(const NodeContext& node, const std::string type) {
auto x = node.get_ng_inputs("X");
PADDLE_OP_CHECK(node, x.size() == 2, "partial ops require exactly 2 inputs in X.");
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] This adds a hard validation to prevent out-of-bounds access when X has fewer than 2 inputs, but there is no regression test ensuring conversion fails gracefully (rather than crashing) for malformed models / cut models where the X list is size 0/1. Please add a Paddle FE negative test (e.g., reuse an existing partial_sum/partial_concat model and override inputs to leave only one, then assert conversion throws an expected frontend exception and includes this message).

Copilot generated this review using guidance from repository custom instructions.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


NamedOutputs partial_ops(const NodeContext& node, const std::string type) {
auto x = node.get_ng_inputs("X");
PADDLE_OP_CHECK(node, x.size() == 2, "partial ops require exactly 2 inputs in X.");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PADDLE_OP_CHECK(node, x.size() == 2, "partial ops require exactly 2 inputs in X.");
PADDLE_OP_CHECK(node, x.size() == 2, "`partial_ops` requires exactly 2 inputs in X.");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

FAIL() << "Expected load to fail due to insufficient X inputs for partial_sum";
} catch (const std::exception& ex) {
const std::string msg = ex.what();
ASSERT_NE(msg.find("partial ops require exactly 2 inputs in X."), std::string::npos) << msg;
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BLOCKER] The expected substring in the assertion ("partial ops require exactly 2 inputs in X.") does not match the actual error message introduced in partial_ops.hpp ("partial_ops requires exactly 2 inputs in X."). As written, this test will fail even when the new validation triggers. Align the test expectation with the thrown message (or vice versa) so the substring check matches exactly (including underscore and verb tense).

Suggested change
ASSERT_NE(msg.find("partial ops require exactly 2 inputs in X."), std::string::npos) << msg;
ASSERT_NE(msg.find("partial_ops requires exactly 2 inputs in X."), std::string::npos) << msg;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: PDPD FE OpenVINO PaddlePaddle FrontEnd

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants