[llvm] r254794 - CodeGen: Move the SlotIndexes BumpPtrAllocator before the list it allocates

[Sean Silva via llvm-commits]

Theoretically ASan can catch this, right? Any idea what we weren't tripping
ASan? It seems like this use-after-free would be consistent.

-- Sean Silva

On Fri, Dec 4, 2015 at 3:00 PM, Justin Bogner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: bogner
> Date: Fri Dec  4 17:00:54 2015
> New Revision: 254794
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254794&view=rev
> Log:
> CodeGen: Move the SlotIndexes BumpPtrAllocator before the list it allocates
>
> When a `SlotIndexes` is destroyed, `ileAllocator` will currently be
> destructed before `IndexList`, but all of `IndexList`'s storage has
> been allocated by `ileAllocator`. This means we'll call destructors on
> garbage data, which is very bad. This can be avoided by putting the
> BumpPtrAllocator earlier in the class than anything it allocates.
>
> Unfortunately, I don't know how to test this. It depends very much on
> memory layout, and the only evidence I have that this is actually
> happening in practice are backtraces that might be explained by this.
> By inspection though, the code is obviously dangerous/wrong, and this
> is the right thing to do.
>
> I'll follow up later with a patch that calls clearAndLeakNodesUnsafely
> on the list, since there isn't much point in destructing them when
> they're allocated in a BPA anyway, but I figured it makes sense to
> commit the correctness fix separately from that optimization.
>
> Modified:
>     llvm/trunk/include/llvm/CodeGen/SlotIndexes.h
>
> Modified: llvm/trunk/include/llvm/CodeGen/SlotIndexes.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SlotIndexes.h?rev=254794&r1=254793&r2=254794&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SlotIndexes.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SlotIndexes.h Fri Dec  4 17:00:54 2015
> @@ -333,6 +333,8 @@ namespace llvm {
>    /// This pass assigns indexes to each instruction.
>    class SlotIndexes : public MachineFunctionPass {
>    private:
> +    // IndexListEntry allocator.
> +    BumpPtrAllocator ileAllocator;
>
>      typedef ilist<IndexListEntry> IndexList;
>      IndexList indexList;
> @@ -353,9 +355,6 @@ namespace llvm {
>      /// and MBB id.
>      SmallVector<IdxMBBPair, 8> idx2MBBMap;
>
> -    // IndexListEntry allocator.
> -    BumpPtrAllocator ileAllocator;
> -
>      IndexListEntry* createEntry(MachineInstr *mi, unsigned index) {
>        IndexListEntry *entry =
>          static_cast<IndexListEntry*>(
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151204/43a2af59/attachment.html>