Fix crash in MakeImmutable(rec(x:=~));

src/plist.c

@@ -2628,19 +2628,22 @@ Obj FuncASS_PLIST_DEFAULT (
 **  (or immutable, but MakeImmutable will have caught that case before we get here)
 */
 
-void MakeImmutablePlistInHom( Obj list )
+void MakeImmutablePlistInHom(Obj list)
 {
-  UInt i;
-  Obj elm;
-  RetypeBag( list, IMMUTABLE_TNUM(TNUM_OBJ(list)));
-  for (i = 1; i <= LEN_PLIST(list); i++)
-    {
-      elm = ELM_PLIST( list, i);
-      if (elm != 0)
-	{
-	  MakeImmutable( elm );
-	  CHANGED_BAG(list);
-	}
+    // change the tnum first, to avoid infinite recursion for objects that
+    // contain themselves
+    RetypeBag(list, IMMUTABLE_TNUM(TNUM_OBJ(list)));
+
+    // FIXME HPC-GAP: there is a potential race here: <list> becomes public
+    // the moment we change its type, but it's not ready for public access
+    // until the following code completed.
+
+    UInt len = LEN_PLIST(list);
+    for (UInt i = 1; i <= len; i++) {
+        Obj elm = ELM_PLIST(list, i);
+        if (elm != 0) {
+            MakeImmutable(elm);
+        }
     }
 }
 

src/precord.c

@@ -240,19 +240,24 @@ void CleanPRecCopy (
 *F  MakeImmutablePRec( <rec> )
 */
 
-void MakeImmutablePRec( Obj rec)
+void MakeImmutablePRec(Obj rec)
 {
-  UInt len;
-  UInt i;
-  len = LEN_PREC( rec );
-  for ( i = 1; i <= len; i++ )
-    MakeImmutable(GET_ELM_PREC(rec,i));
-
-  /* Sort the record at this point.
-     This can never hurt, unless the record will never be accessed again anyway
-     for HPCGAP it's essential so that immutable records are actually binary unchanging */
-  SortPRecRNam(rec, 1); 
-  RetypeBag(rec, IMMUTABLE_TNUM(TNUM_OBJ(rec)));
+    // change the tnum first, to avoid infinite recursion for objects that
+    // contain themselves
+    RetypeBag(rec, IMMUTABLE_TNUM(TNUM_OBJ(rec)));
+
+    // FIXME HPC-GAP: there is a potential race here: <rec> becomes public
+    // the moment we change its type, but it's not ready for public access
+    // until the following code completed.
+
+    UInt len = LEN_PREC(rec);
+    for (UInt i = 1; i <= len; i++)
+        MakeImmutable(GET_ELM_PREC(rec, i));
+
+    // Sort the record at this point. This can never hurt, unless the record
+    // will never be accessed again anyway. But for HPC-GAP it is essential so
+    // that immutable records are actually binary unchanging.
+    SortPRecRNam(rec, 1);
 }
 
 

tst/testbugfix/2018-07-02-MakeImmutablePRec.tst

@@ -0,0 +1,7 @@
+# the following used to crash GAP due to infinite recursion
+gap> MakeImmutable(rec(x:=~));
+rec( x := ~ )
+
+# just to be complete, also test this for plists
+gap> MakeImmutable([1,~]);
+[ 1, ~ ]