gemini-code-assist[bot] commented on code in PR #99:
URL: https://github.com/apache/tvm-ffi/pull/99#discussion_r2422308137
##########
python/tvm_ffi/cython/base.pxi:
##########
@@ -114,7 +114,7 @@ cdef extern from "tvm/ffi/c_api.h":
uint64_t combined_ref_count
int32_t type_index
uint32_t __padding
- void (*deleter)(TVMFFIObject* self)
+ void (*deleter)(void* self, int flags)
Review Comment:
While the change from `TVMFFIObject*` to `void*` is a step in the right
direction for generality, the signature of the `deleter` function pointer in
this struct definition is incorrect and does not match the C header. Here, it's
defined with one argument (`void* self`), but the corresponding C header
`include/tvm/ffi/c_api.h` (line 258) defines it with two arguments: `void
(*deleter)(void* self, int flags);`.
This discrepancy in the ABI definition can lead to stack corruption and
undefined behavior when the deleter is invoked by the C++ runtime. It's
critical to ensure the Cython struct definition matches the C++ struct layout.
Please update the definition to match the C header. This appears to be a
pre-existing issue, but it's important to fix it to ensure correctness and
prevent subtle bugs.
```
void (*deleter)(void* self, int flags)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
