Skip to content

Commit

Permalink
Fix overflow bug in CYCLE_STRUCT_PERM
Browse files Browse the repository at this point in the history
CycleStructPerm overflowed the length of a cycle of length
exactly 2^16, so store the size-1 (which is what we output anyway)
  • Loading branch information
ChrisJefferson committed Nov 12, 2019
1 parent 71194d3 commit 82a5c29
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 15 deletions.
24 changes: 9 additions & 15 deletions src/permutat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1326,14 +1326,14 @@ static inline Obj CYCLE_STRUCT_PERM(Obj perm)
max = 0;
for (pnt = 0; pnt < deg; pnt++) {
if (clr[pnt] == 0) {
len = 1;
len = 0;
clr[pnt] = 1;
for (p = ptPerm[pnt]; p != pnt; p = ptPerm[p]) {
clr[p] = 1;
len++;
}

if (len > 1) {
if (len > 0) {
offset[cnt] = (T)len;
cnt++;
if (len > max) {
Expand All @@ -1346,27 +1346,21 @@ static inline Obj CYCLE_STRUCT_PERM(Obj perm)
ende = cnt;

/* create the list */
list = NEW_PLIST(T_PLIST, max - 1);
SET_LEN_PLIST(list, max - 1);
list = NEW_PLIST(T_PLIST, max);
SET_LEN_PLIST(list, max);
ptList = ADDR_OBJ(list);

/* Recalculate after possible GC */
scratch = ADDR_TMP_PERM<T>();
offset = (T *)((UInt)scratch + (bytes));

for (pnt = 1; pnt < max; pnt++) {
ptList[pnt] = 0;
} /* clean out */

for (cnt = 0; cnt < ende; cnt++) {
pnt = (UInt)offset[cnt];
pnt--;
ptList[pnt] = (Obj)((UInt)ptList[pnt] + 1);
}

for (pnt = 1; pnt < max; pnt++) {
if (ptList[pnt] != 0) {
ptList[pnt] = INTOBJ_INT((UInt)ptList[pnt]);
if(ptList[pnt] == 0) {
ptList[pnt] = INTOBJ_INT(1);
}
else {
ptList[pnt] = INTOBJ_INT(INT_INTOBJ(ptList[pnt]) + 1);
}
}

Expand Down
103 changes: 103 additions & 0 deletions tst/testinstall/kernel/permutat.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#
# Tests for functions defined in src/permutat.cc
#
gap> START_TEST("kernel/permutat.tst");

# Reduce the amount printed by CYCLE_STRUCT_PERM by skipping unbound values
gap> CycleStructPermShort := function(p)
> local ret, i, l;
> l := CYCLE_STRUCT_PERM(p);
> ret := [];
> for i in [1..Length(l)] do
> if IsBound(l[i]) then
> Add(ret, [i,l[i]]);
> fi;
> od;
> return ret;
> end;;
gap> permprops := function(p)
> local name;
> for name in ["LARGEST_MOVED_POINT_PERM",
> "ORDER_PERM", "SIGN_PERM"] do
> PrintFormatted("{}:{}\n", name, ValueGlobal(name)(p));
> od;
> PrintFormatted("CYCLE_SHORT_PERM (short output): {}\n", CycleStructPermShort(p));
> end;;
gap> permprops(());
LARGEST_MOVED_POINT_PERM:0
ORDER_PERM:1
SIGN_PERM:1
CYCLE_SHORT_PERM (short output): [ ]
gap> permprops((1,2));
LARGEST_MOVED_POINT_PERM:2
ORDER_PERM:2
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 1, 1 ] ]
gap> permprops((1,2)(3,4));
LARGEST_MOVED_POINT_PERM:4
ORDER_PERM:2
SIGN_PERM:1
CYCLE_SHORT_PERM (short output): [ [ 1, 2 ] ]
gap> permprops((1,5,4,3,2));
LARGEST_MOVED_POINT_PERM:5
ORDER_PERM:5
SIGN_PERM:1
CYCLE_SHORT_PERM (short output): [ [ 4, 1 ] ]
gap> permprops((1,2)(3,4)(5,6));
LARGEST_MOVED_POINT_PERM:6
ORDER_PERM:2
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 1, 3 ] ]
gap> permprops((1,2^20));
LARGEST_MOVED_POINT_PERM:1048576
ORDER_PERM:2
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 1, 1 ] ]
gap> permprops((1,2,3,4,5)^5);
LARGEST_MOVED_POINT_PERM:0
ORDER_PERM:1
SIGN_PERM:1
CYCLE_SHORT_PERM (short output): [ ]
gap> permprops(PermList(Concatenation([2^17], [1..2^17-1])));
LARGEST_MOVED_POINT_PERM:131072
ORDER_PERM:131072
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 131071, 1 ] ]

# Test the boundary between PERM2 and PERM4
gap> permprops((2^16-1, 2^16));
LARGEST_MOVED_POINT_PERM:65536
ORDER_PERM:2
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 1, 1 ] ]
gap> permprops((2^16-2, 2^16-1));
LARGEST_MOVED_POINT_PERM:65535
ORDER_PERM:2
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 1, 1 ] ]
gap> permprops((2^16-2, 2^16-3));
LARGEST_MOVED_POINT_PERM:65534
ORDER_PERM:2
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 1, 1 ] ]
gap> permprops(PermList(Concatenation([2^16+1], [1..2^16])));
LARGEST_MOVED_POINT_PERM:65537
ORDER_PERM:65537
SIGN_PERM:1
CYCLE_SHORT_PERM (short output): [ [ 65536, 1 ] ]
gap> permprops(PermList(Concatenation([2^16], [1..2^16-1])));
LARGEST_MOVED_POINT_PERM:65536
ORDER_PERM:65536
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 65535, 1 ] ]
gap> permprops(PermList(Concatenation([2^16-1], [1..2^16-2])));
LARGEST_MOVED_POINT_PERM:65535
ORDER_PERM:65535
SIGN_PERM:1
CYCLE_SHORT_PERM (short output): [ [ 65534, 1 ] ]
gap> permprops(PermList(Concatenation([2^16-2], [1..2^16-3])));
LARGEST_MOVED_POINT_PERM:65534
ORDER_PERM:65534
SIGN_PERM:-1
CYCLE_SHORT_PERM (short output): [ [ 65533, 1 ] ]
gap> STOP_TEST("kernel/permutat.tst", 1);

0 comments on commit 82a5c29

Please sign in to comment.