[PATCH] D14350: [FunctionAttrs] Fix an iterator wraparound bug

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 4 14:50:32 PST 2015


sanjoy added a subscriber: nlewycky.

================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:445
@@ -444,2 +444,3 @@
       CallSite::arg_iterator B = CS.arg_begin(), E = CS.arg_end();
-      for (CallSite::arg_iterator A = B; A != E; ++A, ++AI) {
+      bool InVarArgSection = false;
+      for (CallSite::arg_iterator A = B; A != E; ++A) {
----------------
reames wrote:
> I would rather we just gave up on VarArg callsites unless you have a compelling reason we need to handle them here.  Return a conservative answer, and let someone else refine it later.
> 
> p.s. Does the other patch have the same issue?
I didn't add the code, so I'm not in a position to judge whether there is a compelling reason for this optimization or not. :)

Moreover, bailing out unconditionally on varargs functions breaks `test/Transforms/FunctionAttrs/readattrs.ll` (added by @nlewycky in Jul 2013), and I'd rather not pessimize transforms that have in-tree tests.

> p.s. Does the other patch have the same issue?

If you mean D14306, then I don't think so -- that change is fine as far as I can tell.



http://reviews.llvm.org/D14350





More information about the llvm-commits mailing list