[PATCH] D28494: Make `@llvm.objectsize` more conservative with null pointers.

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv added a comment.

Thanks for the comments!



================
Comment at: include/llvm/Analysis/MemoryBuiltins.h:146
+                   ObjSizeMode Mode = ObjSizeMode::Exact,
+                   bool NullIsUnknownSize = false);
 
----------------
echristo wrote:
> As an enhancement I'd love to see this encapsulated into a preferences struct or something rather than two bools and an enum.
Done.

FWIW, most users only care about `RoundToAlign`, so I played with having a `ObjectSizeOpts::withRoundToAlign(bool)` method, but I think `getObjectSize(..., ObjectSizeOpts{}.withRoundToAlign(RoundToAlign))` ends up being a bit uglier than just making an `ObjectSizeOpts` and setting `RoundToAlign` directly. Happy to do `withRoundToAlign` instead if you think that's better.


================
Comment at: lib/Analysis/MemoryBuiltins.cpp:602-603
 ObjectSizeOffsetVisitor::visitConstantPointerNull(ConstantPointerNull&) {
+  if (NullIsUnknownSize)
+    return unknown();
   return std::make_pair(Zero, Zero);
----------------
arsenm wrote:
> I think this will do the wrong thing with non-0 address spaces
Good point; fixed.


https://reviews.llvm.org/D28494