[PATCH] Fix infinite recursion bug in SROA

[Chandler Carruth]

I hate that routine. We've been here before, repeatedly, each time with
order of iteration bugs.

Your patch is certainly correct, but I think its worth setting this
question to rest by making this a more direct body of code. Here is my
suggestion:

- Remove the 'return' from the loop. Every time there is an early exit from
this kind of thing, it all goes to shit complexity wise.
- Have the loop pick a best type *and* the best integral type.
- Keep an explicit "no longer viable" state for the basic "best type" case
(due to non-load/store or conflicting types)

After we look at every slice, return the best type if viable, else the best
integral type if viable, else 0.

Essentially, Ty and ITy should have the following transitions:

Ty: 0 -> <good> -> <bad>
ITy: 0 -> <good>

Ty saturates in the 'bad' state.

I think that would be more clear than even after your patch.



On Mon, Jan 20, 2014 at 5:08 AM, James Molloy <james.molloy at arm.com> wrote:

> Hi Chandler,
>
>
>
> The attached patch fixes a bug that could cause SROA to go into infinite
> recursion (stack overflow in debug mode, infinite loop in release mode).
>
>
>
> The function findCommonType() attempts to use a type from a load/store
> inst, and if there are non-load-store instructions or their types do not
> match it falls back to trying to find an integral type that covers all
> uses. Otherwise, it should return failure (NULL).
>
>
>
> The mechanism to fall back to integral types does not reset the “winning”
> type to NULL. This means that depending on the order in which the input
> uses are iterated over changes the output of the function. If a
> non-load-store instruction comes first, it will return NULL (as Ty is never
> set). If a load/store inst comes first, it will return the Ty of that
> instruction.
>
>
>
> There is no testcase as the bug depends on memory layout; I could not
> force it to fail with opt/llc.
>
>
>
> Is it OK to commit?
>
>
>
> Cheers,
>
>
>
> James
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140121/fead0cbc/attachment.html>