gemini-code-assist[bot] commented on code in PR #619:
URL: https://github.com/apache/tvm-ffi/pull/619#discussion_r3389271022
##########
src/ffi/extra/serialization.cc:
##########
@@ -288,6 +298,38 @@ class ObjectGraphDeserializer {
}
private:
+ // Maximum decode recursion depth. Deeply nested object graphs beyond this
+ // throw rather than overflowing the C++ stack. Chosen well above any
+ // realistic IR nesting while staying safely within a default stack.
+ static constexpr int64_t kMaxRecursionDepth = 1024;
+
+ // RAII helper that bumps the recursion depth on construction and restores it
+ // on destruction, throwing when the configured limit is exceeded.
+ class RecursionGuard {
+ public:
+ explicit RecursionGuard(ObjectGraphDeserializer* self) : self_(self) {
+ if (++self_->recursion_depth_ > kMaxRecursionDepth) {
+ TVM_FFI_THROW(ValueError) << "Maximum object graph recursion depth ("
<< kMaxRecursionDepth
+ << ") exceeded while deserializing; input is
too deeply nested";
+ }
+ }
Review Comment:
In C++, if an exception is thrown inside a constructor, the object is
considered to have never been fully constructed, meaning its destructor
(`~RecursionGuard()`) will not be called. In this case, if `TVM_FFI_THROW` is
triggered, `self_->recursion_depth_` will remain incremented. While the current
usage of `ObjectGraphDeserializer` on the stack mitigates this (since the
entire deserializer is destroyed upon stack unwinding), it is a best practice
for exception safety to perform the bounds check before modifying any state.
This prevents potential bugs if the class is refactored or reused in the future.
```suggestion
explicit RecursionGuard(ObjectGraphDeserializer* self) : self_(self) {
if (self_->recursion_depth_ >= kMaxRecursionDepth) {
TVM_FFI_THROW(ValueError) << "Maximum object graph recursion depth
(" << kMaxRecursionDepth
<< ") exceeded while deserializing; input
is too deeply nested";
}
++self_->recursion_depth_;
}
```
--
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]
