Bug 32716 - [4.2 Regression] Wrong code generation. Alias and C++ virtual bases problem.
[4.2 Regression] Wrong code generation. Alias and C++ virtual bases problem.
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: c++
4.3.0
: P1 normal
: 4.3.0
Assigned To: Not yet assigned to anyone
: alias, wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-10 09:51 UTC by Pranav Bhandarkar
Modified: 2009-03-30 22:11 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.1.2 4.3.0
Known to fail: 4.2.0 4.2.1 4.2.5
Last reconfirmed: 2007-07-10 11:29:31


Attachments
Dump of the early inline pass, that highlights the problem with the inliner (108.09 KB, application/octet-stream)
2007-07-10 10:02 UTC, Pranav Bhandarkar
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pranav Bhandarkar 2007-07-10 09:51:54 UTC
Generating wrong code for the following code snippet (test.C)
using namespace std;
#include <iostream>
class A { public:int a;};
class B: public virtual A { public:     A::a;};
class C : public virtual A { public:    A::a;};
class D : public C, public B {};
void h ( D &x )
{
        x.a++;
}
int main ()
{
       int result ;
        D d;
        d.a = 0;
        h (d);
        result = !(d.a==1);
        result ? cout<<"FAILED": cout <<"SUCCESS";
        return 0;
}
$>arm-none-eabi-g++ -O3 test.C
$> arm-none-eabi-run a.out
FAILED
$>arm-none-eabi-g++ -O3 test.C -fno-inline-functions
$> arm-none-eabi-run a.out
SUCCESS
$>arm-none-eabi-g++ --version
arm-none-eabi-g++ (GCC) 4.3.0 20070703 (experimental)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Comment 1 Pranav Bhandarkar 2007-07-10 10:02:29 UTC
Created attachment 13876 [details]
Dump of the early inline pass, that highlights the problem with the inliner

h() gets inlined into main but the result of the increment in h() is not used in main after inlining causing 0 to be assigned to result.
Comment 2 Richard Biener 2007-07-10 11:29:31 UTC
This is an aliasing problem (or rather a C++ FE problem):

  # SFT.41_31 = VDEF <SFT.41_30(D)>
  d.D.2508.a = 0;
  x.0_10 = (struct A *) &d;
...
  D.2747_16 = x.0_10 + D.2746_15;
  # VUSE <SFT.44_27>
  D.2748_17 = D.2747_16->a;
  D.2749_18 = D.2748_17 + 1;
  # SFT.44_34 = VDEF <SFT.44_27>
  D.2747_16->a = D.2749_18;
  # VUSE <SFT.41_31>
  D.2707_1 = d.D.2508.a;
  D.2706_2 = D.2707_1 != 1;
  return D.2706_2;

note how we don't identify the contrived access through the virtual base
with the zero-initialization and use in main().  Instead, it seems to
alias with

  iftmp.1_6 = (int (*__vtbl_ptr_type) (void) *) D.2735_5;
  # SFT.44_21 = VDEF <SFT.44_20(D)>
  d.D.2504._vptr.C = iftmp.1_6;
...
  # SFT.44_27 = VDEF <SFT.44_21>
  d.D.2504._vptr.C = &_ZTV1D[3];


Of course the original trees created for the virtual base access is
"somewhat" contrieved:

  (void) ((struct A *) (struct D *) x + (long unsigned int) *(long int *) (((struct D *) x)->D.2504._vptr.C + 0xffffffffffffffffffffffffffffffe8))->a++ ;

while in main() we manage to do:

    struct D d;
  <<cleanup_point <<< Unknown tree: expr_stmt
  __comp_ctor  (&d) >>>
>>;
  <<cleanup_point <<< Unknown tree: expr_stmt
  (void) (d.D.2508.a = 0) >>>
>>;
  <<cleanup_point <<< Unknown tree: expr_stmt
  h ((struct D &) &d) >>>
>>;
  return <retval> = d.D.2508.a != 1;
Comment 3 Richard Biener 2007-07-10 12:52:37 UTC
Fixed with "take3.diff".
Comment 4 Ramana Radhakrishnan 2007-07-10 15:14:56 UTC
(In reply to comment #3)
> Fixed with "take3.diff".
> 

Did you forget to attach take3.diff ? 
Comment 5 rguenther@suse.de 2007-07-10 15:32:51 UTC
Subject: Re:  [4.2/4.3 Regression] Wrong code generation.
 Alias and C++ virtual bases problem.

On Tue, 10 Jul 2007, ramana dot radhakrishnan at celunite dot com wrote:

> ------- Comment #4 from ramana dot radhakrishnan at celunite dot com  2007-07-10 15:14 -------
> (In reply to comment #3)
> > Fixed with "take3.diff".
> > 
> 
> Did you forget to attach take3.diff ? 

No, that was a hint to Danny ;)

Richard.
Comment 6 Daniel Berlin 2007-07-10 16:59:09 UTC
Subject: Re:  [4.2/4.3 Regression] Wrong code generation. Alias and C++ virtual bases problem.

On 10 Jul 2007 15:32:51 -0000, rguenther at suse dot de
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #5 from rguenther at suse dot de  2007-07-10 15:32 -------
> Subject: Re:  [4.2/4.3 Regression] Wrong code generation.
>  Alias and C++ virtual bases problem.
>
> On Tue, 10 Jul 2007, ramana dot radhakrishnan at celunite dot com wrote:
>
> > ------- Comment #4 from ramana dot radhakrishnan at celunite dot com  2007-07-10 15:14 -------
> > (In reply to comment #3)
> > > Fixed with "take3.diff".
> > >
> >
> > Did you forget to attach take3.diff ?
>
> No, that was a hint to Danny ;)


You never told me whether omnetpp/xalanbmc were still failing with it or not :)
Comment 7 rguenther@suse.de 2007-07-11 08:32:56 UTC
Subject: Re:  [4.2/4.3 Regression] Wrong code generation.
 Alias and C++ virtual bases problem.

On Tue, 10 Jul 2007, dberlin at dberlin dot org wrote:

> On 10 Jul 2007 15:32:51 -0000, rguenther at suse dot de
> <gcc-bugzilla@gcc.gnu.org> wrote:
> >
> >
> > ------- Comment #5 from rguenther at suse dot de  2007-07-10 15:32 -------
> > Subject: Re:  [4.2/4.3 Regression] Wrong code generation.
> >  Alias and C++ virtual bases problem.
> >
> > On Tue, 10 Jul 2007, ramana dot radhakrishnan at celunite dot com wrote:
> >
> > > ------- Comment #4 from ramana dot radhakrishnan at celunite dot com  2007-07-10 15:14 -------
> > > (In reply to comment #3)
> > > > Fixed with "take3.diff".
> > > >
> > >
> > > Did you forget to attach take3.diff ?
> >
> > No, that was a hint to Danny ;)
> 
> 
> You never told me whether omnetpp/xalanbmc were still failing with it or not :)

Doh!  Yes, they did.  You never got around sending me the variant with
more asserts either ;)

Richard.
Comment 8 Daniel Berlin 2007-08-19 23:23:54 UTC
Subject: Bug 32716

Author: dberlin
Date: Sun Aug 19 23:23:29 2007
New Revision: 127629

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127629
Log:
2007-08-19  Daniel Berlin  <dberlin@dberlin.org>

	Fix PR 32772
	Fix PR 32716
	Fix PR 32328
	Fix PR 32303

	* tree-flow.h (struct stmt_ann_d): Remove makes_clobbering_call.
	* tree-ssa-alias.c (init_transitive_clobber_worklist): Add
	on_worklist argument and avoid adding things to worklist multiple
	times.
	(add_to_worklist): Ditto.
	(mark_aliases_call_clobbered): Mark entire structure clobbered if
	single SFT is clobbered.
	(set_initial_properties): Ditto.
	(compute_call_clobbered): Update for changes to function
	arguments.
	(create_overlap_variables_for): Always create SFT for offset 0.
	(create_structure_vars): Handle PHI's, since we are in SSA form at
	this point.
	* tree-ssa-loop-ivopts.c (get_ref_tag): Don't return subvars.
	* tree-ssa-operands.c (access_can_touch_variable): Don't handle
	TARGET_MEM_REF.
	(add_vars_for_offset): Figure out aliases from access + points-to.
	(add_virtual_operand): Use add_vars-for_offset.
	(get_tmr_operands): Update for NMT changes, rewrite to be correct.
	(add_call_clobber_ops): Remove makes_clobbering_call set.
	(get_expr_operands): Always pass through the INDIRECT_REF
	reference.
	* tree-ssa-structalias.c (struct constraint_graph): Remove
	variables member.
	Add pe, pe_rep, pointer_label, loc_label, pointed_by, points_to,
	address_taken, pt_used, number_incoming.
	(FIRST_ADDR_NODE): Removed.
	(merge_graph_nodes): Remove broken code for the moment.
	(init_graph): New function.
	(build_pred_graph): Remove code to init_graph.
	Add location equivalence support.
	(struct scc_info): Rename roots to deleted.
	(scc_visit): Ditto.
	(init_scc_info): Ditto
	(init_topo_info): Use graph->size.
	(compute_topo_order): Ditto.
	(do_da_constraint): Removed.
	(do_sd_constraint): Remove calls to find().
	set_union_with_increment should always get 0 as last arg here.
	(do_complex_constraint): Replace do_da_constraint with assert.
	Stop calling find.
	(struct equiv_class_label): New.
	(pointer_equiv_class_table): Ditto.
	(location_equiv_class_table): Ditto.
	(equiv_class_label_hash): Ditto.
	(equiv_class_label_eq): Ditto
	(equiv_class_lookup): Ditto.
	(equiv_class_ladd): Ditto.
	(pointer_equiv_class): Ditto.
	(location_equiv_class): Ditto.
	(condense_visit): Rename and rewrite from label_visit to do only
	SCC related stuff for HU.
	(label_visit): Do HU work for HU.
	(perform_var_substitution): Update to do HU and location
	equivalence.
	(free_var_substitution_info): Update to free HU and location
	equivalence structures.  */
	(find_equivalent_node): Update for pointer but not location
	equivalence.
	(unite_pointer_equivalences): New function.
	(move_complex_constraints): Rewrite to only do moving.
	(rewrite_constraints): Split out of move_complex_constraints.
	(solve_graph): Use graph->size.
	(process_constraint_1): Add from_call argument, use it.
	Split *a = &b into two constraints.
	(process_constraint): Use new process_constraint_1.
	(get_constraint_for_component_ref): Handle bitmaxsize == -1 case.
	(get_constraint_for): Handle non-pointer integers properly.
	Remove code that used to handle structures.
	(handle_ptr_arith): Fix a few bugs in pointer arithmetic handling
	with unknown addends.
	(handle_rhs_call): New function.
	(find_func_aliases): Use handle_rhs_call.
	(set_uids_in_ptset): Add an assert.
	(set_used_smts): Fix bug in not considering unified vars.
	(compute_tbaa_pruning): Stop initing useless iteration_obstack.
	(compute_points_to_sets): Update for other function changes.
	(delete_points_to_sets): Ditto.
	(ipa_pta_execute): Ditto.
	(pass_ipa_pta): We need to update SSA after ipa_pta.
	


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-flow.h
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree-ssa-loop-ivopts.c
    trunk/gcc/tree-ssa-operands.c
    trunk/gcc/tree-ssa-structalias.c

Comment 9 Daniel Berlin 2007-08-20 01:54:34 UTC
All should be fixed
Comment 10 Hussam Al-Tayeb 2007-08-20 08:51:44 UTC
Will 4.2.2 get this fix or only trunk?
Comment 11 Richard Biener 2007-08-20 09:27:42 UTC
Re-opening, 4.2 is still affected.
Comment 12 Mark Mitchell 2007-10-09 19:20:41 UTC
Change target milestone to 4.2.3, as 4.2.2 has been released.
Comment 13 Joseph S. Myers 2008-02-01 16:54:33 UTC
4.2.3 is being released now, changing milestones of open bugs to 4.2.4.
Comment 14 Joseph S. Myers 2008-05-19 20:23:21 UTC
4.2.4 is being released, changing milestones to 4.2.5.
Comment 15 Ramana Radhakrishnan 2009-01-15 10:41:23 UTC
Add self to CC
Comment 16 Joseph S. Myers 2009-03-30 22:11:28 UTC
Closing 4.2 branch, fixed in 4.3.