From a8d96078a1cc42fe32fcda7a4bae5020618a0152 Mon Sep 17 00:00:00 2001
From: Chris Jefferson <caj21@st-andrews.ac.uk>
Date: Mon, 15 Jul 2019 18:38:04 +0100
Subject: [PATCH] Clear stored inverse when calling TrimPerm

This is required if TrimPerm changes the TNUM of the permutation,
as a permutation and its stored inverse must have the same TNUM.
This is also required if TrimPerm changes the permutation (although
this would break many other things if anyone did it)
---
 src/permutat.cc                         |  1 +
 src/permutat.h                          | 19 +++++++++++-
 tst/testbugfix/2019-07-15-StoredInv.tst | 40 +++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 tst/testbugfix/2019-07-15-StoredInv.tst

diff --git a/src/permutat.cc b/src/permutat.cc
index 0f190a6d8c..dfa28005e6 100644
--- a/src/permutat.cc
+++ b/src/permutat.cc
@@ -2238,6 +2238,7 @@ Obj Array2Perm (
 
 void TrimPerm(Obj perm, UInt m)
 {
+    CLEAR_STOREDINV_PERM(perm);
     if (TNUM_OBJ(perm) == T_PERM2) {
         GAP_ASSERT(m <= DEG_PERM2(perm));
         ResizeBag(perm, SIZEBAG_PERM2(m));
diff --git a/src/permutat.h b/src/permutat.h
index 85204d4cef..879b5b3a59 100644
--- a/src/permutat.h
+++ b/src/permutat.h
@@ -76,7 +76,12 @@ EXPORT_INLINE const UInt4 * CONST_ADDR_PERM4(Obj perm)
 
 EXPORT_INLINE Obj STOREDINV_PERM(Obj perm)
 {
-    return ADDR_OBJ(perm)[0];
+    Obj inv = ADDR_OBJ(perm)[0];
+    /* Check inv has the same TNAM as perm
+      This is checked below in SET_STOREDINV_PERM, but could
+      be invalidated if either perm or inv is changed in-place */
+    GAP_ASSERT(!inv || (TNUM_OBJ(perm) == TNUM_OBJ(inv)));
+    return inv;
 }
 
 /* SET_STOREDINV_PERM should only be used in neither perm, nor inv has
@@ -98,6 +103,18 @@ EXPORT_INLINE void SET_STOREDINV_PERM(Obj perm, Obj inv)
     }
 }
 
+/* Clear the stored inverse. This is required if 'perm' changes TNUM.
+   Also clears the stored inverse of the stored inverse (which should be
+   perm). */
+EXPORT_INLINE void CLEAR_STOREDINV_PERM(Obj perm)
+{
+    Obj inv = ADDR_OBJ(perm)[0];
+    if (inv) {
+        ADDR_OBJ(inv)[0] = 0;
+        ADDR_OBJ(perm)[0] = 0;
+    }
+}
+
 
 #define IMAGE(i,pt,dg)  (((i) < (dg)) ? (pt)[(i)] : (i))
 
diff --git a/tst/testbugfix/2019-07-15-StoredInv.tst b/tst/testbugfix/2019-07-15-StoredInv.tst
new file mode 100644
index 0000000000..ce748445ee
--- /dev/null
+++ b/tst/testbugfix/2019-07-15-StoredInv.tst
@@ -0,0 +1,40 @@
+# When TRIM_PERM causes a permutation 'p' to change TNAM, the stored inverse
+# must be cleared, as the stored inverse of 'p' must have the
+# same TNAM as 'p'
+
+# Make p a Perm4
+gap> p := (1,2,3,4)*(2^16,2^16+1)*(2^16,2^16+1);
+(1,2,3,4)
+gap> IsPerm4Rep(p);
+true
+
+# Force inverse calculation
+gap> q := p^-1;
+(1,4,3,2)
+gap> IsPerm4Rep(q);
+true
+
+# Now trim
+gap> TRIM_PERM(p, 4);
+gap> IsPerm2Rep(p);
+true
+
+# Check inverse is also a perm2
+gap> IsPerm2Rep(p^-1);
+true
+
+# But this has not changed q
+gap> IsPerm4Rep(q);
+true
+
+# and it's inverse is the correct type
+gap> IsPerm4Rep(q^-1);
+true
+
+# Check some calculations that use the inverse
+gap> List([1..5], x -> x/p);
+[ 4, 1, 2, 3, 5 ]
+
+# And on q (to ensure it's inverse is not still 'p')
+gap> List([1..5], x -> x/q);
+[ 2, 3, 4, 1, 5 ]