vsk marked 8 inline comments as done.
vsk added a comment.
Thanks for your comments, and sorry for jumping the gun earlier with an updated
diff. I'll attach a fixed-up diff shortly.
================
Comment at: lib/CodeGen/CGDecl.cpp:1911
+ if (auto Nullability = Ty->getNullability(getContext())) {
+ if (Nullability && *Nullability == NullabilityKind::NonNull) {
+ SanitizerScope SanScope(this);
----------------
jroelofs wrote:
> aprantl wrote:
> > Is this a typo, or do you. really want `Nullability` here? Asking because
> > everywhere else there is a check for `! Nullability && *Nullability ==
> > NullabilityKind::NonNull`.
> > Ans should this condition be factored out?
> Could just stick `if (auto Nullability =` in there, and remove the
> `Nullability &&`.
'Nullability' should not have been checked for truthiness twice. By factoring
out, do you mean moving the check's logic out of EmitParmDecl and into a
helper? If so, I'm not sure if that would make the code cleaner, but I'm open
to it.
================
Comment at: test/CodeGenObjC/ubsan-null-retval.m:14
+ // CHECK-NEXT: [[ICMP:%.*]] = icmp ne i32* [[ARG]], null, !nosanitize
+ // CHECK-NEXT: br i1 [[ICMP]], label %[[CONT:.+]], label %[[HANDLE:[^,]+]]
+ // CHECK: [[HANDLE]]:
----------------
jroelofs wrote:
> The:
>
> ```
> alloca
> store
> load
> ```
>
> part of this looks like it's just making extra work for mem2reg. Is the
> alloca actually necessary?
>
>
Clang likes its allocas :). This is typical -O0 behavior: IIUC it simplifies
IRGen. E.g for expressions which take the address of an argument, because there
is guaranteed storage for the arg.
https://reviews.llvm.org/D30762
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
