llvm dev - Aug 2013 - [LLVMdev] Incredible effects of extending AtomicSDNode::Ops

Hello,

As part of enhancing atomic operations in the ARM backend, we need to
extend the Ops array in AtomicSDNode.

This array is a private member of AtomicSDNode, so it's only
accessible within 85 lines of code, none of which have anything to do
with its size. Therefore, increasing the size of Ops shouldn't at all
affect the behaviour of LLVM, we supposed.

Much to our surprise, this isn't as trivial.

Even this innocuous change:
===================--- a/include/llvm/CodeGen/SelectionDAGNodes.h
+++ b/include/llvm/CodeGen/SelectionDAGNodes.h
@@ -1068,6 +1068,7 @@ public:
 ///
 class AtomicSDNode : public MemSDNode {
   SDUse Ops[4];
+  SDUse buffer;

   void InitAtomic(AtomicOrdering Ordering, SynchronizationScope SynchScope) {
     // This must match encodeMemSDNodeFlags() in SelectionDAG.cpp.
===================
-- makes LLVM fail 83 regression tests, basically everything related
to the atomic ops, in a bunch of unrelated ways: some segfaults, some
failing assertions, some wrong outputs, all suggesting there's some
kind of memory corruption going on.

Further experiments show that "SDUse buffer" causes these regressions
whether before or after Ops, but "int buffer[N]" (for various values
of N) always causes crashes when before Ops, and never causes them
when after Ops.

To rule out a compilation bug, we've tried building LLVM with a few
different versions of gcc, all running on x86_64 Linux dev boxes. The
failures are 100% reproducible, and their number varies a little bit,
depending on the kind of "buffer" inserted.

We've tried increasing the size of Ops in other SDNode subclasses, and
it didn't introduce crashes, so AtomicSDNode is special in this
respect.

We've just depleted our imagination on how this can at all be
possible, so we welcome your advice.

Hi Artyom,
> We've just depleted our imagination on how this can at all be
> possible, so we welcome your advice.
I suspect it's that SDNodes are allocated using a BumpPtrAllocator
(sort of) with a maximum size of sizeof(LargestSDNode). Changing that
typedef to AtomicSDNode with your additions makes all the tests pass
again for me.

Cheers.

Tim.