[compiler-rt] [SanitizerCommon] add null check for fopen64 interceptor (PR #68760)

Thurston Dang via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 10 22:05:35 PDT 2023


thurstond wrote:

> https://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html says "The fopen() function shall open the file whose pathname is the string pointed to by pathname, and associates a stream with it." A null argument cannot be used and the behavior is undefined. I cannot find historical documented precedent for accepting a null argument in https://www.kernel.org/doc/man-pages/ and elsewhere. There are many places that we don't check null arguments for `COMMON_INTERCEPTOR_WRITE_RANGE` and the like when passing null arguments causes UB. I think this patch should be rejected.

Since it's UB, we can do whatever we like, so accepting the null argument is not technically wrong, right?

Alternatively, would you be satisfied if the interceptor were changed to return an error in such cases?

I don't like the status quo (i.e., without this patch), whereby the interceptor will dereference a null pointer (e.g., output in https://github.com/llvm/llvm-project/pull/68760#issuecomment-1756771659), because it's not an intuitive error message.

https://github.com/llvm/llvm-project/pull/68760


More information about the llvm-commits mailing list