gemini-code-assist[bot] commented on code in PR #387:
URL: https://github.com/apache/tvm-ffi/pull/387#discussion_r2670135798
##########
src/ffi/dtype.cc:
##########
@@ -293,19 +322,38 @@ inline DLDataType
StringViewToDLDataType_(std::string_view str) {
scan = str.data();
TVM_FFI_THROW(ValueError) << "unknown dtype `" << str << '`';
}
- char* xdelim; // emulate sscanf("%ux%u", bits, lanes)
- uint8_t bits = static_cast<uint8_t>(strtoul(scan, &xdelim, 10));
+ // Parse bits manually to handle non-null-terminated string_view
+ const char* xdelim = scan;
+ uint32_t bits_val = parse_digits(xdelim, str_end);
+ if (bits_val > UINT8_MAX) {
+ TVM_FFI_THROW(ValueError) << "Bits value " << bits_val << " is out of
range for uint8_t in dtype '"
+ << str << "'";
+ }
+ uint8_t bits = static_cast<uint8_t>(bits_val);
if (bits != 0) dtype.bits = bits;
int scalable_multiplier = 1;
- if (strncmp(xdelim, "xvscale", 7) == 0) {
+ // Check bounds before dereferencing xdelim
+ if (xdelim < str_end && strncmp(xdelim, "xvscale", 7) == 0) {
scalable_multiplier = -1;
xdelim += 7;
}
Review Comment:
The check `xdelim < str_end` is not sufficient to prevent an out-of-bounds
read with `strncmp(xdelim, "xvscale", 7)`. If `str_end - xdelim` is less than
7, `strncmp` will read past the end of the string view. You should check if
there are enough characters remaining before calling `strncmp`.
```suggestion
if ((str_end - xdelim >= 7) && strncmp(xdelim, "xvscale", 7) == 0) {
scalable_multiplier = -1;
xdelim += 7;
}
```
##########
src/ffi/dtype.cc:
##########
@@ -293,19 +322,38 @@ inline DLDataType
StringViewToDLDataType_(std::string_view str) {
scan = str.data();
TVM_FFI_THROW(ValueError) << "unknown dtype `" << str << '`';
}
- char* xdelim; // emulate sscanf("%ux%u", bits, lanes)
- uint8_t bits = static_cast<uint8_t>(strtoul(scan, &xdelim, 10));
+ // Parse bits manually to handle non-null-terminated string_view
+ const char* xdelim = scan;
+ uint32_t bits_val = parse_digits(xdelim, str_end);
+ if (bits_val > UINT8_MAX) {
+ TVM_FFI_THROW(ValueError) << "Bits value " << bits_val << " is out of
range for uint8_t in dtype '"
+ << str << "'";
+ }
+ uint8_t bits = static_cast<uint8_t>(bits_val);
if (bits != 0) dtype.bits = bits;
int scalable_multiplier = 1;
- if (strncmp(xdelim, "xvscale", 7) == 0) {
+ // Check bounds before dereferencing xdelim
+ if (xdelim < str_end && strncmp(xdelim, "xvscale", 7) == 0) {
scalable_multiplier = -1;
xdelim += 7;
}
- char* endpt = xdelim;
- if (*xdelim == 'x') {
- dtype.lanes = static_cast<uint16_t>(scalable_multiplier * strtoul(xdelim +
1, &endpt, 10));
+ const char* endpt = xdelim;
+ // Check bounds before dereferencing xdelim
+ if (xdelim < str_end && *xdelim == 'x') {
+ endpt = xdelim + 1;
+ const char* digits_start = endpt;
+ uint32_t lanes_val = parse_digits(endpt, str_end);
+ if (endpt == digits_start || lanes_val == 0) {
+ TVM_FFI_THROW(ValueError) << "Invalid lanes specification in dtype '" <<
str
+ << "'. Lanes must be a positive integer.";
+ }
+ if (lanes_val > UINT16_MAX) {
+ TVM_FFI_THROW(ValueError) << "Lanes value " << lanes_val
+ << " is out of range for uint16_t in dtype '"
<< str << "'";
+ }
+ dtype.lanes = static_cast<uint16_t>(scalable_multiplier * lanes_val);
}
Review Comment:
This logic for parsing lanes is very similar to the implementation inside
the `parse_float` lambda (lines 233-247). To improve maintainability and avoid
code duplication, consider extracting this logic into a shared helper lambda,
defined alongside `parse_digits`.
--
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]
