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

Handle the case when arraystruct->meminfo is null to close gh-965 #972

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

oleksandr-pavlyk
Copy link
Contributor

Closes gh-965 by raising Python exception when arraystruct->meminfo field is null, and unhandled exception would have caused dereferencing of the null pointer and subsequent crash.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • If this PR is a work in progress, are you filing the PR as a draft?

Copy link
Contributor

@chudur-budur chudur-budur left a comment

Choose a reason for hiding this comment

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

LGTM

@oleksandr-pavlyk oleksandr-pavlyk merged commit 4c18ebd into main Mar 13, 2023
@oleksandr-pavlyk oleksandr-pavlyk deleted the fix-gh-965-issue-by-scan-tool branch March 13, 2023 15:57
github-actions bot added a commit that referenced this pull request Mar 13, 2023
Handle the case when arraystruct->meminfo is null to close gh-965 4c18ebd
@diptorupd
Copy link
Contributor

diptorupd commented Mar 13, 2023

@oleksandr-pavlyk @chudur-budur the solution is fine for now especially as it has already been merged.

I wanted to implement it in a different way. Look at what Numba does for NumPy arrays: https://github.com/numba/numba/blob/ce69f3010c77942123815d86900cb841c0e5d2fa/numba/core/runtime/_nrt_python.c#L366

Numba uses the arraystruct.meminfo if the meminfo object is populated. Otherwise it lets the ndarray base to be NULL when calling PyArray_NewFromDescr.

    array = (PyArrayObject *) PyArray_NewFromDescr(retty, descr, ndim,
                                                   shape, strides, arystruct->data,
                                                   flags, (PyObject *) miobj);

We should have equivalent C-API for tensor and dpnp and handle the case where we do not have meminfo.

@oleksandr-pavlyk
Copy link
Contributor Author

@diptorupd I think it should be reimplemented as needed. The fix went in like this to comply with SDL requirements.

github-actions bot added a commit to chudur-budur/numba-dpex that referenced this pull request Mar 14, 2023
…by-scan-tool

Handle the case when arraystruct->meminfo is null to close IntelPythongh-965 4c18ebd
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.

Potential use of undefined pointer in _dpexrt_python.c
3 participants