Skip to content

fixing problem with CPN model parsing overflow and query parsing overflow - #232

Merged
srba merged 2 commits into
mainfrom
overflow-tests-cpn-query
Oct 5, 2026
Merged

srba merged 2 commits into
mainfrom
overflow-tests-cpn-query

Conversation

@srba

@srba srba commented Sep 19, 2026

Copy link
Copy Markdown
Member

@srba
srba requested review from a team and mtygesen and removed request for a team September 21, 2026 06:12
Comment thread src/PetriParse/PNMLParser.cpp Outdated
if (strcmp(element->name(), "numberconstant") == 0) {
auto value = element->first_attribute("value")->value();
return (uint32_t)atoll(value);
uint64_t parsed = atoll(value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

toBoundedTokens could be moved into errors,h and then reused here to avoid duplicated code

Comment thread include/utils/errors.h Outdated
}
errno = 0;
char* end = nullptr;
const long long parsed = std::strtoll(text, &end, 10);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could pass std::string_view to the function and then use std::from_chars here, which is a bit cleaner

Comment thread include/utils/errors.h Outdated
errno = 0;
char* end = nullptr;
const long long parsed = std::strtoll(text, &end, 10);
if (end == text) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this currently allows e.g. "123abc"

Comment thread include/utils/errors.h Outdated
if (errno == ERANGE ||
parsed < static_cast<long long>(std::numeric_limits<int32_t>::min()) ||
parsed > static_cast<long long>(std::numeric_limits<int32_t>::max())) {
throw base_error("Integer constant ", text, " exceeded ", std::numeric_limits<int32_t>::max());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Show exceeded on both overflow and underflow. Maybe there could be two separate error messages to make it easier to diagnose

@srba
srba requested a review from mtygesen September 30, 2026 09:45

@mtygesen mtygesen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good now

@srba srba left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Passes correctness tests on the cluster. In reachabilityCardinality about 30 fewer answers as there were queries with too large constants in GPPP-PT-0010G1... model.

@srba
srba merged commit 9c0f3b5 into main Oct 5, 2026
4 checks passed
@srba
srba deleted the overflow-tests-cpn-query branch October 5, 2026 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants