[PATCH] D122431: Basic support for posix_memalign / __builtin_object_size interaction

[Nikita Popov via Phabricator via llvm-commits]

nikic added reviewers: fhahn, asbirlea, reames.
nikic added a comment.

This is pretty unusual in terms of approach, it would be more typical to do a limited upward walk to find the clobbering store (a la FindAvailableLoadedValue).



================
Comment at: llvm/lib/Analysis/MemoryBuiltins.cpp:847
+  if (!(isa<AllocaInst>(LoadingFrom) || (A && A->hasNoAliasAttr())))
+    return Unknown();
+
----------------
What about the case where the object size is taken prior to initialization? In the alloca case it will be an undef initialization -- is it fine to simply ignore that case? I'd probably expect min at least to return zero for that.

Probably more obviously problematic is the noalias case, because that one doesn't make any statement on the initial contents of the memory. For a typical allocator that will be either undef or null, but this is not a requirement of noalias (an obvious violation is strdup).


================
Comment at: llvm/lib/Analysis/MemoryBuiltins.cpp:909
+    else if (auto *SI = dyn_cast<StoreInst>(V)) {
+      if (Options.AA && Options.AA->isNoAlias(SI->getOperand(0), &LI)) {
+        // According to AA, this store is dead at `LI` point, so skip it.
----------------
I don't get what this check is supposed to do. You're asking here whether an escaping pointer and an escape-source alias, for which the answer should always be "yes" for the SimpleCaptureInfo-based AA used here. Can you give an example where this check does something useful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122431