[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7671
+                             AllArgs, CallType))
+    return true;
+
----------------
rjmccall wrote:
> yihanaa wrote:
> > rjmccall wrote:
> > > yihanaa wrote:
> > > > rjmccall wrote:
> > > > > You can just pull the argument expressions out of the `CallExpr`; you don't need to call `GatherArgumentsForCall`.
> > > > > You can just pull the argument expressions out of the `CallExpr`; you don't need to call `GatherArgumentsForCall`.
> > > > 
> > > > This GatherArgumentsForCall  was used to do the common sema checking and emit warning, like './main.cpp:5:40: warning: passing 'volatile char *' to parameter of type 'const void *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]' hahaha, for this is a common case, I also think  GatherArgumentsForCall is not a good choice
> > > > , so I try to find a replacement, e.g. ImpCastExprToType or other ways, what do you think about?
> > > `convertArgumentToType` should trigger any useful warnings in the second and third arguments.  For the first, I don't actually think there are any warnings we care about.
> > I'm sorry John, I can't find `convertArgumentToType ` in clang, did you mean `ConvertArgumentsForCall` or `ImpCastExprToType`. we can't use `ConvertArgumentsForCall `, because `ConvertArgumentsForCall ` has checked if current CallExpr calling a builtin function with custom sema checking, it will do nothing and return.
> Oh, sorry, it's a helper function in Apple's fork that we added for the ptrauth builtins but haven't upstreamed yet.  Feel free to add it yourself, at the top of the file right after `checkArgCount`:
> 
> ```
> static bool convertArgumentToType(Sema &S, Expr *&Value, QualType Ty) {
>   if (Value->isTypeDependent())
>     return false;
> 
>   InitializedEntity Entity =
>     InitializedEntity::InitializeParameter(S.Context, Ty, false);
>   ExprResult Result =
>     S.PerformCopyInitialization(Entity, SourceLocation(), Value);
>   if (Result.isInvalid())
>     return true;
>   Value = Result.get();
>   return false;
> }
> ```
wow,cool! Thanks John


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979