This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: mutable members in global const objects


On Mon, Aug 03, 1998 at 10:46:38AM +0100, Nathan Sidwell wrote:
> Previously I reported a problem about mutable members in static const
> objects (http://www.cygnus.com/ml/egcs-bugs/1998-Jul/0346.html). The
> problem being that g++ can place these in the read only data section
> (rodata).

> It occurs to me that some optimizations might need to know about the
> presence of mutable members too, though I think it sufficient for them
> to know only when they access the mutable member itself (and that of
> course is indicated in the member's declaration).

I produced a patch to deal with the bug (mutable global objects in read
only data section), 2.5 years ago.  I don't why it never made it in.
Maybe I never submitted it :-)  Probably something to do with GCC 2.8
taking forever.

It also fixes a bug -- you're right about the optimiser needing to know
about mutable.  Otherwise it thinks some values are fixed when they're
not, and generates the wrong code.  At least, GCC 2.7.2.x did.

The file below includes a test case for the bug, a patch to fix it and
also to put objects with mutable members in the writable data section.
It applies to GCC 2.7.2.x.  I used it until mid 1997, after which I
stopped working on C++ projects, so it is at least reliable.

Someone might be able to do something useful with it.

-- Jamie

------------------------------------------------------------
This fixes a bug that causes incorrect code to be generated.  The bug
has been present from versions 2.7.0 through to 2.7.2.  It is not
present in version 2.6.3.

The program below tests for the bug.

extern "C" void printf (const char *, ...);
const struct S { mutable int x; } s = {2};
int main ()
{
  int i = 0;
  while (--s.x)
    if (++i == 10)
      break;
  if (i != 1)
    printf ("This code is wrong.\n", i);
  else
    printf ("This code is fine.\n");
  return 0;
}

Fixes: Doesn't set RTX_UNCHANGING_P for aggregates containing mutable fields.
Doesn't put such aggregates in the read-only data section.

diff -cr release-gcc-2.7.2/ChangeLog new-gcc-2.7.2/ChangeLog
*** release-gcc-2.7.2/ChangeLog	Thu Jan 18 02:59:50 1996
--- new-gcc-2.7.2/ChangeLog	Thu Jan 18 03:36:00 1996
***************
*** 1,3 ****
--- 1,15 ----
+ Wed Jan 17 22:39:16 1996  Jamie Lokier  <jamie@rebellion.co.uk>
+ 
+ 	* tree.h: (TREE_MUTABLE, TYPE_FIELDS_MUTABLE): New macros.
+ 	* print-tree.c (print_node): Show if TREE_MUTABLE is set.
+ 	* varasm.c (make_decl_rtl): Don't set RTX_UNCHANGING_P for const
+ 	aggregates that contain mutable fields.
+ 	* function.c (assign_parms): Likewise (two places)
+ 	* expr.c (expand_expr): Likewise for INDIRECT_REF and OFFSET_REF.
+ 
+ 	* varasm.c (assemble_variable): Don't put aggregates that contain
+ 	mutable fields in the read-only data section.
+ 
  Sun Nov 26 14:47:42 1995  Richard Kenner  <kenner@mole.gnu.ai.mit.edu>
  
  	* Version 2.7.2 released.
diff -cr release-gcc-2.7.2/cp/ChangeLog new-gcc-2.7.2/cp/ChangeLog
*** release-gcc-2.7.2/cp/ChangeLog	Thu Jan 18 02:04:04 1996
--- new-gcc-2.7.2/cp/ChangeLog	Thu Jan 18 02:44:53 1996
***************
*** 1,3 ****
--- 1,16 ----
+ Wed Jan 17 22:46:35 1996  Jamie Lokier  <jamie@rebellion.co.uk>
+ 
+ 	* typeck.c (build_component_ref_1): Set TREE_MUTABLE along with
+  	TREE_READONLY if DECL_MUTABLE_P is set.
+ 	(build_component_ref): Likewise.
+ 	* cp-tree.h (DECL_MUTABLE_P): Use TREE_MUTABLE.
+ 	* ptree.c (print_lang_type): Show if TYPE_FIELDS_MUTABLE is set
+ 	* decl.c (grokdeclarator): Set TYPE_FIELDS_MUTABLE for the current
+  	class when setting DECL_MUTABLE_P for a member of the class, or if
+ 	TYPE_FIELDS_MUTABLE is set for the member.
+ 	(xref_basetypes): Copy TYPE_FIELDS_MUTABLE from the base class to
+  	the derived one.
+ 
  Mon Nov 20 14:06:28 1995  Mike Stump  <mrs@cygnus.com>
  
  	* Version 2.7.2 released.
diff -cr release-gcc-2.7.2/cp/cp-tree.h new-gcc-2.7.2/cp/cp-tree.h
*** release-gcc-2.7.2/cp/cp-tree.h	Thu Jan 18 02:03:01 1996
--- new-gcc-2.7.2/cp/cp-tree.h	Wed Jan 17 23:11:21 1996
***************
*** 946,952 ****
    unsigned abstract_virtual : 1;
    unsigned permanent_attr : 1 ;
    unsigned constructor_for_vbase_attr : 1;
-   unsigned mutable_flag : 1;
    unsigned is_default_implementation : 1;
    unsigned saved_inline : 1;
    unsigned use_template : 2;
--- 946,951 ----
***************
*** 955,961 ****
    unsigned nonconverting : 1;
    unsigned declared_inline : 1;
    unsigned not_really_extern : 1;
!   unsigned dummy : 4;
  
    tree access;
    tree context;
--- 954,960 ----
    unsigned nonconverting : 1;
    unsigned declared_inline : 1;
    unsigned not_really_extern : 1;
!   unsigned dummy : 5;
  
    tree access;
    tree context;
***************
*** 1038,1044 ****
  
  /* Nonzero for _DECL means that this member object type
     is mutable.  */
! #define DECL_MUTABLE_P(NODE) (DECL_LANG_SPECIFIC(NODE)->decl_flags.mutable_flag)
  
  /* Nonzero for _DECL means that this constructor is a non-converting
     constructor.  */
--- 1037,1043 ----
  
  /* Nonzero for _DECL means that this member object type
     is mutable.  */
! #define DECL_MUTABLE_P(NODE) (TREE_MUTABLE(NODE))
  
  /* Nonzero for _DECL means that this constructor is a non-converting
     constructor.  */
diff -cr release-gcc-2.7.2/cp/decl.c new-gcc-2.7.2/cp/decl.c
*** release-gcc-2.7.2/cp/decl.c	Thu Jan 18 02:03:04 1996
--- new-gcc-2.7.2/cp/decl.c	Thu Jan 18 02:45:28 1996
***************
*** 9611,9619 ****
--- 9611,9622 ----
  		decl = build_lang_field_decl (FIELD_DECL, declarator, type);
  		if (RIDBIT_SETP (RID_MUTABLE, specbits))
  		  {
+ 		    TYPE_FIELDS_MUTABLE (current_class_type) = 1;
  		    DECL_MUTABLE_P (decl) = 1;
  		    RIDBIT_RESET (RID_MUTABLE, specbits);
  		  }
+ 		TYPE_FIELDS_MUTABLE (current_class_type) |=
+ 		  TYPE_FIELDS_MUTABLE (type);
  	      }
  
  	    bad_specifiers (decl, "field", virtualp, quals != NULL_TREE,
***************
*** 10758,10763 ****
--- 10761,10767 ----
  	  TYPE_GETS_NEW (ref) |= TYPE_GETS_NEW (basetype);
  	  TYPE_GETS_DELETE (ref) |= TYPE_GETS_DELETE (basetype);
  	  CLASSTYPE_LOCAL_TYPEDECLS (ref) |= CLASSTYPE_LOCAL_TYPEDECLS (basetype);
+ 	  TYPE_FIELDS_MUTABLE (ref) |= TYPE_FIELDS_MUTABLE (basetype);
  	  i += 1;
  	}
      }
diff -cr release-gcc-2.7.2/cp/ptree.c new-gcc-2.7.2/cp/ptree.c
*** release-gcc-2.7.2/cp/ptree.c	Thu Jan 18 02:03:10 1996
--- new-gcc-2.7.2/cp/ptree.c	Wed Jan 17 23:45:31 1996
***************
*** 131,136 ****
--- 131,138 ----
      fputs (" op[]", file);
    if (TYPE_OVERLOADS_ARROW (node))
      fputs (" op->", file);
+   if (TYPE_FIELDS_MUTABLE (node))
+     fputs (" fields-mutable", file);
    if (TYPE_USES_MULTIPLE_INHERITANCE (node))
      fputs (" uses-multiple-inheritance", file);
  
diff -cr release-gcc-2.7.2/cp/typeck.c new-gcc-2.7.2/cp/typeck.c
*** release-gcc-2.7.2/cp/typeck.c	Thu Jan 18 02:03:12 1996
--- new-gcc-2.7.2/cp/typeck.c	Wed Jan 17 23:49:05 1996
***************
*** 1612,1618 ****
    if (TREE_THIS_VOLATILE (datum) || TREE_THIS_VOLATILE (field))
      TREE_THIS_VOLATILE (ref) = 1;
    if (DECL_MUTABLE_P (field))
!     TREE_READONLY (ref) = 0;
  
    return ref;
  }
--- 1612,1621 ----
    if (TREE_THIS_VOLATILE (datum) || TREE_THIS_VOLATILE (field))
      TREE_THIS_VOLATILE (ref) = 1;
    if (DECL_MUTABLE_P (field))
!     {
!       TREE_READONLY (ref) = 0;
!       TREE_MUTABLE (ref) = 1;
!     }
  
    return ref;
  }
***************
*** 1837,1843 ****
    if (TREE_THIS_VOLATILE (datum) || TREE_THIS_VOLATILE (field))
      TREE_THIS_VOLATILE (ref) = 1;
    if (DECL_MUTABLE_P (field))
!     TREE_READONLY (ref) = 0;
  
    return ref;
  }
--- 1840,1849 ----
    if (TREE_THIS_VOLATILE (datum) || TREE_THIS_VOLATILE (field))
      TREE_THIS_VOLATILE (ref) = 1;
    if (DECL_MUTABLE_P (field))
!     {
!       TREE_READONLY (ref) = 0;
!       TREE_MUTABLE (ref) = 1;
!     }
  
    return ref;
  }
diff -cr release-gcc-2.7.2/expr.c new-gcc-2.7.2/expr.c
*** release-gcc-2.7.2/expr.c	Thu Jan 18 02:04:08 1996
--- new-gcc-2.7.2/expr.c	Thu Jan 18 01:48:55 1996
***************
*** 4582,4588 ****
  	   through a pointer to const does not mean that the value there can
  	   never change.  Languages where it can never change should
  	   also set TREE_STATIC.  */
! 	RTX_UNCHANGING_P (temp) = TREE_READONLY (exp) | TREE_STATIC (exp);
  	return temp;
        }
  
--- 4582,4596 ----
  	   through a pointer to const does not mean that the value there can
  	   never change.  Languages where it can never change should
  	   also set TREE_STATIC.  */
! 	/* The above has been disregarded for some reason.  Someone
! 	   probably decided that this is an ok assumption for the
! 	   compiler to make.  Regardless, if EXP is a const aggregate
! 	   that contains mutable fields, RTX_UNCHANGING_P must definitely
! 	   not be set. */
! 	if ((TREE_READONLY (exp) | TREE_STATIC (exp))
! 	    && !(TYPE_FIELDS_MUTABLE (TREE_TYPE (exp))))
! 	  RTX_UNCHANGING_P (temp) = 1;
! 
  	return temp;
        }
  
***************
*** 4906,4912 ****
  #if 0 /* It is incorrect to set RTX_UNCHANGING_P here, because the fact that
  	 a location is accessed through a pointer to const does not mean
  	 that the value there can never change.  */
! 	RTX_UNCHANGING_P (temp) = TREE_READONLY (exp);
  #endif
  	return temp;
        }
--- 4914,4924 ----
  #if 0 /* It is incorrect to set RTX_UNCHANGING_P here, because the fact that
  	 a location is accessed through a pointer to const does not mean
  	 that the value there can never change.  */
! 	/* If EXP is a const aggregate that contains mutable fields,
! 	   it isn't unchanging. */
! 	if (TREE_READONLY (exp)
! 	    && !(TYPE_FIELDS_MUTABLE (TREE_TYPE (exp))))
! 	  RTX_UNCHANGING_P (temp) = 1;
  #endif
  	return temp;
        }
diff -cr release-gcc-2.7.2/function.c new-gcc-2.7.2/function.c
*** release-gcc-2.7.2/function.c	Thu Jan 18 02:04:09 1996
--- new-gcc-2.7.2/function.c	Thu Jan 18 01:28:55 1996
***************
*** 3591,3597 ****
  	      else if (PARM_BOUNDARY % BITS_PER_WORD != 0)
  		abort ();
  
! 	      if (TREE_READONLY (parm))
  		RTX_UNCHANGING_P (stack_parm) = 1;
  
  	      move_block_from_reg (REGNO (entry_parm),
--- 3591,3603 ----
  	      else if (PARM_BOUNDARY % BITS_PER_WORD != 0)
  		abort ();
  
! 	      /* If PARM is a const aggregate that contains mutable
! 		 fields, it isn't unchanging.  Actually a const
! 		 parameter can be changed anyway, by taking its address
! 		 and casting away the constness.  It seems fair for the
! 		 compiler to assume this won't happen. */
! 	      if (TREE_READONLY (parm)
! 		  && !(TYPE_FIELDS_MUTABLE (TREE_TYPE (parm))))
  		RTX_UNCHANGING_P (stack_parm) = 1;
  
  	      move_block_from_reg (REGNO (entry_parm),
***************
*** 3900,3906 ****
  
        if (TREE_THIS_VOLATILE (parm))
  	MEM_VOLATILE_P (DECL_RTL (parm)) = 1;
!       if (TREE_READONLY (parm))
  	RTX_UNCHANGING_P (DECL_RTL (parm)) = 1;
      }
  
--- 3906,3918 ----
  
        if (TREE_THIS_VOLATILE (parm))
  	MEM_VOLATILE_P (DECL_RTL (parm)) = 1;
! 
!       /* If PARM is a const aggregate that contains mutable fields, it
! 	 isn't unchanging.  Actually a const parameter can be changed
! 	 anyway, by taking its address and casting away the constness.
! 	 It seems fair for the compiler to assume this won't happen. */
!       if (TREE_READONLY (parm)
! 	  && !(TYPE_FIELDS_MUTABLE (TREE_TYPE (parm))))
  	RTX_UNCHANGING_P (DECL_RTL (parm)) = 1;
      }
  
diff -cr release-gcc-2.7.2/print-tree.c new-gcc-2.7.2/print-tree.c
*** release-gcc-2.7.2/print-tree.c	Thu Jun 15 12:56:40 1995
--- new-gcc-2.7.2/print-tree.c	Thu Jan 18 00:49:16 1996
***************
*** 311,316 ****
--- 311,318 ----
      fputs (" side-effects", file);
    if (TREE_READONLY (node))
      fputs (" readonly", file);
+   if (TREE_MUTABLE (node))
+     fputs (" mutable", file);
    if (TREE_CONSTANT (node))
      fputs (" constant", file);
    if (TREE_ADDRESSABLE (node))
diff -cr release-gcc-2.7.2/tree.h new-gcc-2.7.2/tree.h
*** release-gcc-2.7.2/tree.h	Thu Jan 18 02:03:42 1996
--- new-gcc-2.7.2/tree.h	Wed Jan 17 23:44:11 1996
***************
*** 151,156 ****
--- 151,157 ----
    unsigned readonly_flag : 1;
    unsigned unsigned_flag : 1;
    unsigned asm_written_flag: 1;
+   unsigned mutable_flag : 1;
  
    unsigned used_flag : 1;
    unsigned raises_flag : 1;
***************
*** 166,172 ****
    unsigned lang_flag_4 : 1;
    unsigned lang_flag_5 : 1;
    unsigned lang_flag_6 : 1;
!   /* There is room for two more flags.  */
  };
  
  /* Define accessors for the fields that all tree nodes have
--- 167,173 ----
    unsigned lang_flag_4 : 1;
    unsigned lang_flag_5 : 1;
    unsigned lang_flag_6 : 1;
!   /* There is room for one more flag.  */
  };
  
  /* Define accessors for the fields that all tree nodes have
***************
*** 331,336 ****
--- 332,343 ----
     when the node is a type).  */
  #define TREE_READONLY(NODE) ((NODE)->common.readonly_flag)
  
+ /* In a FIELD_DECL, nonzero means it may be the lhs of an assigment
+    even if the containing structure is read-only.  If TREE_READONLY
+    is set though, it may not be the lhs of an assignment after all.
+    This is for C++ `mutable' fields. */
+ #define TREE_MUTABLE(NODE) ((NODE)->common.mutable_flag)
+ 
  /* Value of expression is constant.
     Always appears in all ..._CST nodes.
     May also appear in an arithmetic expression, an ADDR_EXPR or a CONSTRUCTOR
***************
*** 605,610 ****
--- 612,621 ----
  
  /* Means this type is const-qualified.  */
  #define TYPE_READONLY(NODE) ((NODE)->common.readonly_flag)
+ 
+ /* In an aggregate type, means that at least one of the FIELD_DECLs
+    in the aggregate has TREE_MUTABLE set.  Otherwise leave unset. */
+ #define TYPE_FIELDS_MUTABLE(NODE) ((NODE)->common.mutable_flag)
  
  /* These flags are available for each language front end to use internally.  */
  #define TYPE_LANG_FLAG_0(NODE) ((NODE)->type.lang_flag_0)
diff -cr release-gcc-2.7.2/varasm.c new-gcc-2.7.2/varasm.c
*** release-gcc-2.7.2/varasm.c	Thu Jan 18 02:59:59 1996
--- new-gcc-2.7.2/varasm.c	Thu Jan 18 03:31:43 1996
***************
*** 598,604 ****
  	  if (TREE_SIDE_EFFECTS (decl))
  	    MEM_VOLATILE_P (DECL_RTL (decl)) = 1;
  
! 	  if (TREE_READONLY (decl))
  	    RTX_UNCHANGING_P (DECL_RTL (decl)) = 1;
  	  MEM_IN_STRUCT_P (DECL_RTL (decl))
  	    = AGGREGATE_TYPE_P (TREE_TYPE (decl));
--- 598,607 ----
  	  if (TREE_SIDE_EFFECTS (decl))
  	    MEM_VOLATILE_P (DECL_RTL (decl)) = 1;
  
! 	  /* If DECL is a const aggregate that contains mutable fields,
! 	     it isn't unchanging. */
! 	  if (TREE_READONLY (decl)
! 	      && !(TYPE_FIELDS_MUTABLE (TREE_TYPE (decl))))
  	    RTX_UNCHANGING_P (DECL_RTL (decl)) = 1;
  	  MEM_IN_STRUCT_P (DECL_RTL (decl))
  	    = AGGREGATE_TYPE_P (TREE_TYPE (decl));
***************
*** 1233,1238 ****
--- 1236,1242 ----
  #else
        if (TREE_READONLY (decl)
  	  && ! TREE_THIS_VOLATILE (decl)
+ 	  && ! TYPE_FIELDS_MUTABLE (TREE_TYPE (decl))
  	  && DECL_INITIAL (decl)
  	  && (DECL_INITIAL (decl) == error_mark_node
  	      || TREE_CONSTANT (DECL_INITIAL (decl)))
***************
*** 1283,1288 ****
--- 1287,1293 ----
  #else
        if (TREE_READONLY (decl)
  	  && ! TREE_THIS_VOLATILE (decl)
+ 	  && ! TYPE_FIELDS_MUTABLE (TREE_TYPE (decl))
  	  && DECL_INITIAL (decl)
  	  && (DECL_INITIAL (decl) == error_mark_node
  	      || TREE_CONSTANT (DECL_INITIAL (decl)))
***************
*** 1380,1385 ****
--- 1385,1391 ----
  #else
  	  if (TREE_READONLY (decl)
  	      && ! TREE_THIS_VOLATILE (decl)
+ 	      && ! TYPE_FIELDS_MUTABLE (TREE_TYPE (decl))
  	      && DECL_INITIAL (decl)
  	      && (DECL_INITIAL (decl) == error_mark_node
  		  || TREE_CONSTANT (DECL_INITIAL (decl)))


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]