[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:
> > > 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.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7684
+  if (FirstArgResult.isInvalid())
+    return true;
+
----------------
rjmccall wrote:
> There is in fact a `DefaultFunctionArrayLvalueConversion` method you can use to do all of this at once, and you don't need to explicitly check for a function or array type first.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979