Bug 12114 - [3.3.2] Uninitialized memory accessed in dtor
Summary: [3.3.2] Uninitialized memory accessed in dtor
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 3.3.2
: P2 critical
Target Milestone: 3.3.2
Assignee: Mark Mitchell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-08-31 09:13 UTC by Andreas Jaeger
Modified: 2005-07-23 22:49 UTC (History)
3 users (show)

See Also:
Host: i686-linux-gnu
Target: i686-linux-gnu
Build: i686-linux-gnu
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Jaeger 2003-08-31 09:13:42 UTC
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;
}
Comment 1 GCC Commits 2003-09-01 19:18:06 UTC
Subject: Bug 12114

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	mmitchel@gcc.gnu.org	2003-09-01 19:18:03

Modified files:
	gcc/cp         : ChangeLog call.c cp-tree.h decl.c pt.c typeck.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/init: ref9.C 
	gcc/testsuite/g++.dg/template: nested4.C 

Log message:
	PR c++/12114
	* g++.dg/init/ref9.C: New test.
	
	PR c++/11972
	* g++.dg/template/nested4.C: New test.
	
	PR c++/12114
	* cp-tree.h (initialize_reference): Change prototype.
	* call.c (initialize_reference): Add cleanup parameter.
	* decl.c (grok_reference_init): Likewise.
	(check_initializer): Likewise.
	(cp_finish_decl): Insert a CLEANUP_STMT if necessary.
	(duplicate_decls): When replacing an anticipated builtin, do not
	honor TREE_NOTHROW.
	* typeck.c (convert_for_initialization): Correct call to
	initialize_reference.
	
	PR c++/11972
	* pt.c (dependent_type_p_r): Pass only the innermost template
	arguments to any_dependent_template_arguments_p.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&r1=1.3635&r2=1.3636
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&r1=1.427&r2=1.428
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/cp-tree.h.diff?cvsroot=gcc&r1=1.908&r2=1.909
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/decl.c.diff?cvsroot=gcc&r1=1.1119&r2=1.1120
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/pt.c.diff?cvsroot=gcc&r1=1.768&r2=1.769
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&r1=1.496&r2=1.497
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3015&r2=1.3016
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/ref9.C.diff?cvsroot=gcc&r1=NONE&r2=1.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/template/nested4.C.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 2 GCC Commits 2003-09-01 20:52:52 UTC
Subject: Bug 12114

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_3-branch
Changes by:	mmitchel@gcc.gnu.org	2003-09-01 20:52:50

Modified files:
	gcc/cp         : ChangeLog call.c cp-tree.h decl.c typeck.c 
	gcc/testsuite  : ChangeLog 
	gcc/testsuite/g++.old-deja/g++.abi: cxa_vec.C 
	gcc/testsuite/g++.old-deja/g++.mike: virt2.C 
	gcc/testsuite/gcc.dg: sibcall-1.c 
Added files:
	gcc/testsuite/g++.dg/init: ref9.C 

Log message:
	PR c++/12114
	* cp-tree.h (initialize_reference): Change prototype.
	* call.c (initialize_reference): Add cleanup parameter.
	* decl.c (grok_reference_init): Likewise.
	(check_initializer): Likewise.
	(cp_finish_decl): Insert a CLEANUP_STMT if necessary.
	(duplicate_decls): When replacing an anticipated builtin, do not
	honor TREE_NOTHROW.
	* typeck.c (convert_for_initialization): Correct call to
	initialize_reference.
	
	PR c++/12114
	* g++.dg/init/ref9.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.3076.2.195&r2=1.3076.2.196
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/call.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.341.2.29&r2=1.341.2.30
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/cp-tree.h.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.776.2.31&r2=1.776.2.32
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/decl.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.965.2.59&r2=1.965.2.60
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/cp/typeck.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.436.2.12&r2=1.436.2.13
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2261.2.263&r2=1.2261.2.264
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/init/ref9.C.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=NONE&r2=1.1.2.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.old-deja/g++.abi/cxa_vec.C.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.3&r2=1.3.50.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.old-deja/g++.mike/virt2.C.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2&r2=1.2.74.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/sibcall-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.1&r2=1.1.20.1

Comment 3 Mark Mitchell 2003-09-01 20:56:17 UTC
Fixed in GCC 3.3.2, GCC 3.4.