[Core] Reject negative values when converting to unsigned by om4rrr · Pull Request #33482 · openvinotoolkit/openvino

om4rrr · 2026-01-06T15:28:29Z

Fix

This PR prevents negative values from being accepted for ov::hint::num_requests.
Previously, passing a signed negative value could end up as a large unsigned number when stored/handled as an unsigned property. The property helper now validates inputs and rejects negative signed values early with a clear exception.

Tests

Added unit tests in src/inference/tests/unit/core.cpp:
- PropertiesValidation.HintNumRequestsRejectsNegativeSigned (negative values are rejected)
- PropertiesValidation.HintNumRequestsAcceptsUnsigned (valid unsigned values are accepted and stored correctly)

Ticket

[Good First Issue][Bug][Core]: negative values are not checked in property with unsigned value type #23194

Copilot

Pull request overview

This PR enhances type safety in ov::Any conversions by adding validation for integral type conversions that could result in data loss or unexpected wraparound. The changes prevent silent errors like converting -1 to 4294967295 when casting from signed to unsigned types.

Adds range checking for signed-to-unsigned and unsigned-to-signed integral conversions
Rejects negative signed values when converting to unsigned types
Validates that values fit within target type's numeric limits before conversion

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T15:31:48Z

src/core/include/openvino/core/any.hpp

+            if (std::is_signed<T>::value) {
+                if (value > static_cast<unsigned long long>(std::numeric_limits<T>::max())) {
+                    OPENVINO_THROW("Bad cast (out of range) from ", value, " to: ", typeid(T).name());
+                }
+            } else {
+                if (value > static_cast<unsigned long long>(std::numeric_limits<T>::max())) {
+                    OPENVINO_THROW("Bad cast (out of range) from ", value, " to: ", typeid(T).name());
+                }


The if-else branches for signed vs unsigned target types perform identical checks. Both branches check if the value exceeds the maximum limit of the target type. These branches can be consolidated into a single check that applies to both signed and unsigned target types, reducing code duplication.

Suggested change

if (std::is_signed<T>::value) {

if (value > static_cast<unsigned long long>(std::numeric_limits<T>::max())) {

OPENVINO_THROW("Bad cast (out of range) from ", value, " to: ", typeid(T).name());

}

} else {

if (value > static_cast<unsigned long long>(std::numeric_limits<T>::max())) {

OPENVINO_THROW("Bad cast (out of range) from ", value, " to: ", typeid(T).name());

}

if (value > static_cast<unsigned long long>(std::numeric_limits<T>::max())) {

OPENVINO_THROW("Bad cast (out of range) from ", value, " to: ", typeid(T).name());

src/core/include/openvino/core/any.hpp

om4rrr · 2026-01-15T03:01:31Z

Hi @praasz
I aligned the fix with your note about Any::as being a cast-only API.
I reverted the Any-level change and added validation at the ov::hint::num_requests property layer instead, so negative values are rejected instead of wrapping.
I added unit tests in src/inference/tests/unit/core.cpp to cover both the negative case and a valid unsigned value.

mlukasze · 2026-01-15T06:24:01Z

please resolve conflict before we go any further

om4rrr · 2026-01-15T16:09:48Z

Hi @mlukasze
I have updated the PR to address the merge conflict and ensured the tests are passing. When you have time, could you please review the PR?

src/inference/include/openvino/runtime/properties.hpp

…:num_requests(-1).

olpipi · 2026-01-22T11:50:35Z

I don't have any fundamental objections regarding rejecting negative values for all properties.
Please add tests to check if converting from "-1" as string shows the same behavior as from int.
And please check if binary size is not increased dramatically after adding implementation into .hpp file.

om4rrr · 2026-01-22T19:00:42Z

Hi @olpipi
I added coverage to ensure negative values are rejected consistently for ov::hint::num_requests, including "-1" passed as a string, and verified core.set_property(..., ov::hint::num_requests(...)) rejects both -1 and "-1".

could you please review it?

src/inference/include/openvino/runtime/properties.hpp

src/inference/tests/unit/core.cpp

praasz · 2026-01-23T06:44:40Z

Hi @olpipi I added coverage to ensure negative values are rejected consistently for ov::hint::num_requests, including "-1" passed as a string, and verified core.set_property(..., ov::hint::num_requests(...)) rejects both -1 and "-1".

could you please review it?

I agree with @olpipi that this change is valid.
Just make more detailed tests for different property types and input value type to check corner case of conversion and remove the validation if it is not required to avoid dead code which will be reported by static analysis tool.

- Use std::decay_t<> instead of typename std::decay<>::type - Add overflow check only when source type max > target type max

- Use OV_EXPECT_THROW instead of EXPECT_THROW - Use std::ignore= instead of (void) C-style cast - Keep Core tests for set_property validation

- Cover property types: uint32_t, uint64_t, int32_t - Validate input types: int, long, int64_t, short, unsigned types, string - Test corner cases: 0, -1, INT_MIN, INT64_MIN, max values, overflow - Cover num_requests, auto_batch_timeout, dynamic_quantization_group_size, key_cache_group_size, value_cache_group_size, inference_num_threads, compilation_num_threads

- Test that string "-1" behaves same as int -1 for uint32_t properties - Verify num_requests and auto_batch_timeout reject negative strings - Verify positive strings are accepted correctly

olpipi · 2026-02-04T12:31:03Z

src/inference/include/openvino/runtime/properties.hpp

+                if constexpr (sizeof(VType) > sizeof(UType) ||
+                              (sizeof(VType) == sizeof(UType) && std::is_signed_v<VType>)) {
+                    if (static_cast<unsigned long long>(value) >
+                        static_cast<unsigned long long>(std::numeric_limits<UType>::max())) {
+                        OPENVINO_THROW("Value ", value, " exceeds maximum for target unsigned type");
+                    }
+                }


Suggested change

if constexpr (sizeof(VType) > sizeof(UType) ||

(sizeof(VType) == sizeof(UType) && std::is_signed_v<VType>)) {

if (static_cast<unsigned long long>(value) >

static_cast<unsigned long long>(std::numeric_limits<UType>::max())) {

OPENVINO_THROW("Value ", value, " exceeds maximum for target unsigned type");

}

}

if constexpr (sizeof(VType) > sizeof(UType)) {

if (value > static_cast<VType>(std::numeric_limits<UType>::max())) {

OPENVINO_THROW("Value ", value, " exceeds maximum for target unsigned type");

}

}

(sizeof(VType) == sizeof(UType) && std::is_signed_v<VType>) - is redundant
static_cast<unsigned long long>(value) - what about case if U = uint128_t or a custom type?

Copilot AI review requested due to automatic review settings January 6, 2026 15:28

om4rrr requested a review from a team as a code owner January 6, 2026 15:28

github-actions bot added category: Core OpenVINO Core (aka ngraph) category: CPP API OpenVINO CPP API bindings labels Jan 6, 2026

sys-openvino-ci added the ExternalPR External contributor label Jan 6, 2026

Copilot started reviewing on behalf of om4rrr January 6, 2026 15:29 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

praasz requested changes Jan 13, 2026

View reviewed changes

src/core/include/openvino/core/any.hpp Show resolved Hide resolved

praasz assigned olpipi Jan 13, 2026

om4rrr requested a review from a team as a code owner January 15, 2026 02:53

github-actions bot added category: inference OpenVINO Runtime library - Inference and removed category: Core OpenVINO Core (aka ngraph) labels Jan 15, 2026

om4rrr force-pushed the fix-negative-unsigned branch from 5e2523d to 6cd2054 Compare January 15, 2026 15:52

om4rrr requested a review from praasz January 15, 2026 15:57

praasz reviewed Jan 16, 2026

View reviewed changes

src/inference/include/openvino/runtime/properties.hpp Outdated Show resolved Hide resolved

om4rrr requested a review from praasz January 16, 2026 15:08

om4rrr added 2 commits January 19, 2026 13:42

Validate unsigned property inputs

3ae2899

Add a Core-level test that expects set_property() to reject ov::hint:…

5e9b295

…:num_requests(-1).

om4rrr force-pushed the fix-negative-unsigned branch from c131491 to 5e9b295 Compare January 19, 2026 18:49

Merge branch 'master' into fix-negative-unsigned

5e69e99

praasz self-assigned this Jan 20, 2026

Add tests for negative string to unsigned property

af96b00

praasz reviewed Jan 23, 2026

View reviewed changes

praasz added this to the 2026.1 milestone Jan 23, 2026

mlukasze linked an issue Jan 23, 2026 that may be closed by this pull request

[Good First Issue][Bug][Core]: negative values are not checked in property with unsigned value type #23194

Open

om4rrr and others added 5 commits January 23, 2026 16:45

Refine property validation and remove dead code

6dc11a1

- Use std::decay_t<> instead of typename std::decay<>::type - Add overflow check only when source type max > target type max

Update Core property tests with proper macros

3ed1694

- Use OV_EXPECT_THROW instead of EXPECT_THROW - Use std::ignore= instead of (void) C-style cast - Keep Core tests for set_property validation

Add string input tests

39d0f05

- Test that string "-1" behaves same as int -1 for uint32_t properties - Verify num_requests and auto_batch_timeout reject negative strings - Verify positive strings are accepted correctly

Merge branch 'master' into fix-negative-unsigned

0934d62

om4rrr requested a review from praasz January 23, 2026 15:19

olpipi reviewed Feb 4, 2026

View reviewed changes

Conversation

om4rrr commented Jan 6, 2026 • edited by praasz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

Tests

Ticket

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

om4rrr commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlukasze commented Jan 15, 2026

Uh oh!

om4rrr commented Jan 15, 2026

Uh oh!

Uh oh!

olpipi commented Jan 22, 2026

Uh oh!

om4rrr commented Jan 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

praasz commented Jan 23, 2026

Uh oh!

olpipi Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

om4rrr commented Jan 6, 2026 •

edited by praasz

Loading

om4rrr commented Jan 15, 2026 •

edited

Loading

olpipi Feb 4, 2026 •

edited

Loading