[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D34299#788908, @arphaman wrote:

> Ok, so now the null check `return.sloc.load` won't call the checker in compiler-rt and so the program won't `abort` and won't hit the `unreachable`. I have one question tough:
>
> This patch changes the behavior of this sanitizer for the example that I gave above.


Yes, in the case where there is no explicit return, and the return value happens to be null.

> Previously a runtime diagnostic was emitted, but now there is none. While I'm not saying that the previous behaviour was correct, I'm wondering if the new behaviour is right.  I think that for C++ it makes sense, but I don't know the right answer for C. I'm leaning more towards the new behaviour, since technically in C falling off without returning a value is not UB unless that return value is used by the caller. But at the same time, since we don't diagnose `return` UB for C, maybe it's still worth diagnosing this particular issue? The user might not catch it otherwise at all (or they might catch it later when they try to access it, but by that point they might not know where the pointer came from). WDYT?

Without seeing what the caller does with the result, the return value check can't do a good job of diagnosing missing returns. I think clang should emit a diagnostic in the example above, and -Wreturn-type does. So I think the new behavior is appropriate.


https://reviews.llvm.org/D34299