Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong OrtApi size for v1.17.0/v1.17.1 ? #19893

Closed
Hopobcn opened this issue Mar 13, 2024 · 1 comment
Closed

Wrong OrtApi size for v1.17.0/v1.17.1 ? #19893

Hopobcn opened this issue Mar 13, 2024 · 1 comment
Assignees
Labels
core runtime issues related to core runtime

Comments

@Hopobcn
Copy link

Hopobcn commented Mar 13, 2024

Describe the issue

Hi, first of all sorry if this is already tracked.

I was scanning onnxruntime_c_api.cc for v1.17.0 and I have noticed that the protection against OrtApi modifications:

static_assert(offsetof(OrtApi, SetUserLoggingFunction) / sizeof(void*) == 266, "Size of version 17 API cannot change");

looks wrong for v1.17.0

https://github.com/microsoft/onnxruntime/blob/v1.17.0/onnxruntime/core/session/onnxruntime_c_api.cc#L2717

    &OrtApis::SetUserLoggingFunction,                            <-- it's pointing here
    &OrtApis::ShapeInferContext_GetInputCount,
    &OrtApis::ShapeInferContext_GetInputTypeShape,
    &OrtApis::ShapeInferContext_GetAttribute,
    &OrtApis::ShapeInferContext_SetOutputTypeShape,
    &OrtApis::SetSymbolicDimensions,
    &OrtApis::ReadOpAttr,
    &OrtApis::SetDeterministicCompute,
    &OrtApis::KernelContext_ParallelFor,
    &OrtApis::SessionOptionsAppendExecutionProvider_OpenVINO_V2, <-- should point here
    // End of Version 17                                         <-- comment is missing?
};

v1.17.1 looks to be the same.

To reproduce

No sepecific step. Just compare last function in OrtApi https://github.com/microsoft/onnxruntime/blob/v1.17.0/onnxruntime/core/session/onnxruntime_c_api.cc#L2726 with the static_assert https://github.com/microsoft/onnxruntime/blob/v1.17.1/onnxruntime/core/session/onnxruntime_c_api.cc#L2756

Urgency

No response

Platform

Windows

OS Version

any

ONNX Runtime Installation

Built from Source

ONNX Runtime Version or Commit ID

v1.17.0

ONNX Runtime API

Python

Architecture

X64

Execution Provider

Default CPU

Execution Provider Library Version

No response

@github-actions github-actions bot added the platform:windows issues related to the Windows platform label Mar 13, 2024
@snnn snnn added core runtime issues related to core runtime and removed platform:windows issues related to the Windows platform labels Mar 13, 2024
@YUNQIUGUO
Copy link
Contributor

YUNQIUGUO commented Mar 13, 2024

Looks like a bunch of new C OrtApis are introduced in 1.17.0 time frame, however the static assert sanity check has not been updated along the prs.

Got a fix.

YUNQIUGUO added a commit that referenced this issue Mar 18, 2024
### Description
<!-- Describe your changes. -->

Looks like a bunch of new C OrtApis are introduced in 1.17.0 time frame,
however the static assert sanity check has not been updated along the
prs.

Fix OrtApi marker.

Related issue: 
#19893 


### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->

The branch is checked out against rel-1.17.2. Once rel-1.17.3 is made,
it will be based on this right version.

---------

Co-authored-by: rachguo <[email protected]>
Co-authored-by: George Wu <[email protected]>
fs-eire pushed a commit that referenced this issue Mar 21, 2024
### Description
<!-- Describe your changes. -->

Looks like a bunch of new C OrtApis are introduced in 1.17.0 time frame,
however the static assert sanity check has not been updated along the
prs.

Fix OrtApi marker.

Related issue: 
#19893 


### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->

The branch is checked out against rel-1.17.2. Once rel-1.17.3 is made,
it will be based on this right version.

---------

Co-authored-by: rachguo <[email protected]>
Co-authored-by: George Wu <[email protected]>
@Hopobcn Hopobcn closed this as completed Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core runtime issues related to core runtime
Projects
None yet
Development

No branches or pull requests

3 participants