Bug 12114

We noted a bug in the GiNaC testsuite and Michael and Honza have done
the analysis below:

Okay, I think it's a compiler error, but in the C++ frontend (or in some
later optimization, but hold on).  And it happens only very seldom.  And
if it happens one must get unlucky to see any segfaults.  That's why we
don't see it anywhere else.

A testcase is attached below.  It was extracted basically from GiNaC, but
it doesn't exhibit any segfaults, as this requires much more surrouding
code from GiNaC, for which the libraries and whatnot are needed.  One can
see the error only when it is run under valgrind, when it gives warnings
about accessing uninitialized data.  The uninitialized thing is the
"bp->refcount".  What happens in this function

void ex::construct_from_basic(const basic &b) {
  const ex & tmpex = b.eval();
  bp = tmpex.bp;
  bp->refcount++;
}

is the following: a temporary is created for the result of b.eval(), and
because eval might throw (which it then indeed does) a cleanup must also
be created for this temporary.  But it forgets adding any call to
the ctor of that temporary, or initializing it by any other means,
therefore when calling the dtor in the cleanup, it accesses uninit memory.

One can see this easily with the added printfs.  In the faulting case only
the dtor, but not the ctor are called.

If one removes the '&', i.e. there is no need for a temporary, no such
things happen, and valgrind gives no warnings.
OK, I found what is gonig on, but the fix does not seem to be easy.
The problem seems to be the patch:
2003-06-19  Mark Mitchell  <mark@codesourcery.com>

	PR c++/11041
	* call.c (initialize_reference): Do not use cp_finish_decl to emit
	temporary variables.
	* cp-tree.h (static_aggregates): Declare.
	(pushdecl_top_level_and_finish): Likewise.
	* decl.c (pushdecl_top_level_1): New function.
	(pushdecl_top_level): Use it.
	(pushdecl_top_level_and_finish): New function.
	(initialize_local_var): Remove redundant code.
	(cp_finish_decl): Remove support for RESULT_DECLs.  Don't check
	building_stmt_tree.
	* decl.h (static_aggregates): Remove.
	* decl2.c (get_guard): Use pushdecl_top_level_and_finish.
	* rtti.c (get_tinfo_decl): Use pushdecl_top_level_and_finish.
	(tinfo_base_init): Likewise.

Basically we initially generated something like:
   const ex temporary = b.eval();
   const ex & tmpex = &tempoary;
and attached cleanup to the second statement and forth.  The patch
changes it into something like:
   const ex temporary;
   const ex & tmpex = (temporary=b.eval(), &tempoary);
But we can not get the cleanup to the middle of expression.  This is
needed to fix different testcase
class hop
{
public:
    hop operator* () const;
};
int main(void)
{
    const hop &x = *x;
}
Where we need the variable x already in existence in order to initialize
the temporary.

Here's the testcase:

/* Compile this and run it under valgrind.  If the bug is there one should
   see uninitialized accesses in ex::~ex.  It's coming from the temporary
   to which the reference is bound, in construct_from_basic().
   If one replaces it with an object instead an reference, valgrind doesn't
   warn anymore.  I suspect this is the underlying of GiNaCs segfaults
   int the exam_numeric testsuite.  One can also see, how the dtor is
   called, without ever calling the ctor.  */
extern "C" void printf(const char*, ...);
struct ex;
struct basic {
  int refcount;
  ex eval() const;
  basic() : refcount(0) {}
};

struct ex {
  basic *bp;
  ex() : bp(0) {printf("ex::ex\n");}
  ex(const basic &);
  virtual ~ex();
  void construct_from_basic(const basic &);
};

ex basic::eval() const {
  throw 1;
}

inline ex::ex(const basic &b) { construct_from_basic (b); }
inline ex::~ex() { if (--bp->refcount == 0) delete bp; printf("ex::~ex\n"); }
void ex::construct_from_basic(const basic &b) {
#ifdef WORKAROUND
  const ex tmpex = b.eval();
#else
  const ex & tmpex = b.eval();
#endif
  bp = tmpex.bp;
  bp->refcount++;
}

ex pow() { return basic(); }

int main()
{
  try { pow (); } catch (...) {}
  return 0;
}