[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 29 10:15:24 PDT 2022
rjmccall added a comment.
In D131979#3753358 <https://reviews.llvm.org/D131979#3753358>, @yihanaa wrote:
> In D131979#3752208 <https://reviews.llvm.org/D131979#3752208>, @rjmccall wrote:
>
>> From the test case, it looks like the builtin just ignores pointers to volatile types, which should be preserved by the conversions you're now doing in Sema. That is, you should be able to just check `Ptr->getType()->castAs<PointerType>()->getPointeeType().isVolatile()`.
>>
>> It would be different if it ignored pointers loaded out of volatile l-values or something, but that's explicitly not what it's doing.
>
> Thanks for your tips John, form the test case global constants and compiler-rt ubsan class TypeDescriptor, it looks like pass an type description to __ubsan_handle_alignment_assumption, so, we have to use getSubExprAsWritten to get the origin(user written arg type) type description of 1st arg, but not 'const void *', for example, in catch-alignment-assumption-builtin_assume_aligned-three-params.cpp, the 1st arg type description is 'char **', but if we don't use getSubExprAsWritten, all the type description will be 'const void *'
The version of this patch which changed the builtin to use custom type-checking didn't need to worry about this because doing that will stop Sema from converting to `const void *`. You seem to have taken all of that code out and reverted to an earlier approach, though, and I don't see anything in this review explaining why.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131979/new/
https://reviews.llvm.org/D131979
More information about the cfe-commits
mailing list