[development-gcc] The GiNaC failure

[Jan Hubicka]

> 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.  Indeed if I do exactly
> this change (i.e. going from reference to copied object) in the
> ex::construct_from_basic() function in GiNaC (patch attached, jj you might
> simply want to add it to the package), the testsuite runs through, and all
> is happy.

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.
I am not quite sure how to fix it but for our purposes I would suggest
reverting that patch as ICE is better than misscompilation.
Mark, better ideas?

Honza