[PATCH] D149514: Check if First argument in _builtin_assume_aligned_ is of pointer type

Yurong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 2 07:02:29 PDT 2023


yronglin added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7979-7986
   {
     ExprResult FirstArgResult =
         DefaultFunctionArrayLvalueConversion(FirstArg);
-    if (FirstArgResult.isInvalid())
+    if (checkBuiltinArgument(*this, TheCall, 0))
       return true;
+    /// In-place updation of FirstArg by checkBuiltinArgument is ignored.
     TheCall->setArg(0, FirstArgResult.get());
----------------
rsmith wrote:
> This still seems to be more complex than necessary. If we can't just do this, we should have a comment explaining why. (Eg, does CodeGen really want the final conversion from `T*` to `const void*` to not be in the AST?)
As far as I know, alignment sanitizer need a user written type descriptor (but not `const void *`) to emit diagnostic message, Eg:
```
/app/example.cpp:7:35: runtime error: assumption of 8 byte alignment for pointer of type 'B *' failed

```
So, not only convert `T *` to `const void *`, but also we need keep the user written type info. Previously, the solution to this problem was to use below code in codegen:
```
if (auto *CE = dyn_cast<CastExpr>(E))
    E = CE->getSubExprAsWritten();
```
since D133202 and D133583, we only do de default array/function conversion in sema, and convert the 1st arg to `const void *` in codegen. And I think the current solution is not very good, do you have any other better solutions? Should we keep the original type info in somewhere? Then we can simplify our work in sema. WDYT?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149514/new/

https://reviews.llvm.org/D149514



More information about the cfe-commits mailing list