vsk added a comment.
In https://reviews.llvm.org/D34299#787795, @arphaman wrote:
> It looks like if we have a function without the `return` (like the sample
> below), we will pass in a `0` as the location pointer. This will prevent a
> report of a runtime error as your compiler-rt change ignores the location
> pointers that are `nil`. Is this a bug or is this the intended behaviour?
>
> int *_Nonnull nonnull_retval1(int *p) {
> }
>
This is the intended behavior (I'll add a test). Users should not see a "null
return value" diagnostic here. There is another check, -fsanitize=return, which
can catch this issue.
@filcab --
> Splitting the attrloc from the useloc might make sense since we would be able
> to emit attrloc just once. But I don't see why we need to store/load those
> pointers in runtime instead of just caching the Constant* in CodeGenFunction.
The source locations aren't constants. The ubsan runtime uses a bit inside of
source location structures as a flag. When an issue is diagnosed at a
particular source location, that bit is atomically set. This is how ubsan
implements issue deduplication.
> I'd also like to have some asserts and explicit resets to nullptr after use
> on the ReturnLocation variable, if possible.
Resetting Address fields in CodeGenFunction doesn't appear to be an established
practice. Could you explain what this would be in aid of?
https://reviews.llvm.org/D34299
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
