Skip to content

Fix two issues that undefined behavior sanitizer caught #419

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.

Sign up for GitHub

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

Open
henrylhtsang wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix two issues that undefined behavior sanitizer caught #419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03396ce
fix(string): avoid undefined behavior when memcpy source is null
henrylhtsang Jan 19, 2026
2550deb
fix(object): use offsetof instead of null pointer arithmetic
henrylhtsang Jan 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions include/tvm/ffi/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <tvm/ffi/base_details.h>
#include <tvm/ffi/c_api.h>

#include <cstddef>
#include <optional>
#include <string>
#include <type_traits>
Expand Down Expand Up @@ -1094,8 +1095,8 @@ struct ObjectUnsafe {

template <typename Class>
TVM_FFI_INLINE static int64_t GetObjectOffsetToSubclass() {
return (reinterpret_cast<int64_t>(&(static_cast<Class*>(nullptr)->header_)) -
reinterpret_cast<int64_t>(&(static_cast<Object*>(nullptr)->header_)));
return static_cast<int64_t>(offsetof(Class, header_)) -
Copy link
Member

@tqchen tqchen Jan 21, 2026

Choose a reason for hiding this comment

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

it is helpful to likely use __builtin_offsetof instead, mainly because it is more permissive per standard

static_cast<int64_t>(offsetof(Object, header_));
}

template <typename T>
Expand Down
4 changes: 3 additions & 1 deletion include/tvm/ffi/string.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,9 @@ class String {
}
void InitData(const char* data, size_t size) {
char* dest_data = InitSpaceForSize(size);
std::memcpy(dest_data, data, size);
if (size > 0) {
std::memcpy(dest_data, data, size);
}
Comment on lines +757 to +759
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 20, 2026

Choose a reason for hiding this comment

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

high

This is a great fix to prevent undefined behavior when memcpy is called with a size of 0 and a null source pointer. I noticed that the Bytes::InitData method in this same file has a similar implementation without this check. It would be beneficial to apply the same fix there to ensure consistency and prevent the same undefined behavior in the Bytes class.

dest_data[size] = '\0';
}
/*!
Expand Down
Loading