ganler opened a new pull request #8988:
URL: https://github.com/apache/tvm/pull/8988


   #### Background
   
   This is a fixing proposal to 
[Discussion#10988](https://discuss.tvm.apache.org/t/operators-will-crash-when-receiving-null-object/10988)
 and https://github.com/apache/tvm/issues/8979 .
   
   #### Approach
   
   This PR fixed unexpected C++ crash by:
   1. check `nullptr` before using `operator->` (in this case, it must crash);
   2. allow `None` containers (Array) being capable of range-based for; 
(otherwise crash);
   3. add specific nullptr checking to other complicated cases (e.g., 
`ICHECK(false) << ... << statement_causing_TVMError` will result in crash);
   
   This PR passed test cases contributed in 
https://github.com/apache/tvm/issues/8979 (the crashes that can be trigger by 
giving `None` parameters in TVM's core python APIs)
   
   #### Comparison with other options
   
   According to the discussion, I also tried other proposals but don't think 
they are good approaches: 
   (1) `NOTNULLABLE` and `tvm::runtime::Optional` suggested by @junrushao1994 
and @Hzfengsy ; -> still requires lots of code changes on legacy codes;
   (2) Add checkers for all those `.set_body*` statements; -> not fundamental 
that checkers will scale when new interface coming in;
   
   This PR fixed a fundamental issue causing invalid memory access crashes: 
avoid `nullptr` objects using `->` operator because it must result in program 
crash;
   
   #### Known issue 
   
   Statements like `ICHECK(false) << ... << statement_causing_TVMError` will 
still crash so that we have to manually add new checkers before it. e.g., line 
112 in `BlockReadWriteDetector::operator()`.
   


-- 
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]


Reply via email to