From 6a6f3760ba3fc416096c0bae85f3044528584025 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Tue, 15 May 2018 23:12:16 +0200 Subject: [PATCH 1/3] kernel: call InitGlobalBag(bag) before using Otherwise, if a garbage collection happens after we assign a new bag to , but before calling InitGlobalBag on it, the new bag may be garbage collected, and we end up with a bad reference. --- src/objscoll.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/objscoll.c b/src/objscoll.c index 68749f64bf..4e2b6225fe 100644 --- a/src/objscoll.c +++ b/src/objscoll.c @@ -799,16 +799,6 @@ static Int InitLibrary ( static void InitModuleState(ModuleStateOffset offset) { - const UInt maxStackSize = 256; - STATE(SC_NW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_LW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_PW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_EW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_GE_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_CW_VECTOR) = NEW_STRING(0); - STATE(SC_CW2_VECTOR) = NEW_STRING(0); - STATE(SC_MAX_STACK_SIZE) = maxStackSize; - #ifndef HPCGAP InitGlobalBag( &STATE(SC_NW_STACK), "SC_NW_STACK" ); InitGlobalBag( &STATE(SC_LW_STACK), "SC_LW_STACK" ); @@ -818,6 +808,16 @@ static void InitModuleState(ModuleStateOffset offset) InitGlobalBag( &STATE(SC_CW_VECTOR), "SC_CW_VECTOR" ); InitGlobalBag( &STATE(SC_CW2_VECTOR), "SC_CW2_VECTOR" ); #endif + + const UInt maxStackSize = 256; + STATE(SC_NW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); + STATE(SC_LW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); + STATE(SC_PW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); + STATE(SC_EW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); + STATE(SC_GE_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); + STATE(SC_CW_VECTOR) = NEW_STRING(0); + STATE(SC_CW2_VECTOR) = NEW_STRING(0); + STATE(SC_MAX_STACK_SIZE) = maxStackSize; } /**************************************************************************** From f591206f310c160c43373f37d0da5099aa99f43d Mon Sep 17 00:00:00 2001 From: Max Horn Date: Tue, 15 May 2018 23:12:16 +0200 Subject: [PATCH 2/3] kernel: fix stacks for pc group collectors These stacks were created as T_PLIST_EMPTY, but then used as if they were T_STRING (but with the length field overwritten with arbitrary data), then were resized into T_STRING. Now they are T_DATOBJ, and we make sure not to overwrite the type slot. --- src/objccoll-impl.h | 36 ++++++++++++++++-------------------- src/objscoll-impl.h | 38 +++++++++++++++++--------------------- src/objscoll.c | 35 +++++++++++++++++------------------ 3 files changed, 50 insertions(+), 59 deletions(-) diff --git a/src/objccoll-impl.h b/src/objccoll-impl.h index 529b7ea06f..f1393791d8 100644 --- a/src/objccoll-impl.h +++ b/src/objccoll-impl.h @@ -204,40 +204,36 @@ Int CombiCollectWord ( Obj sc, Obj vv, Obj w ) max = STATE(SC_MAX_STACK_SIZE); /* ensure that the stacks are large enough */ - if ( SIZE_OBJ(vnw)/sizeof(Obj) < max+1 ) { - ResizeBag( vnw, sizeof(Obj)*(max+1) ); - RetypeBag( vnw, T_STRING ); + const UInt desiredStackSize = sizeof(Obj) * (max + 2); + if ( SIZE_OBJ(vnw) < desiredStackSize ) { + ResizeBag( vnw, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vlw)/sizeof(Obj) < max+1 ) { - ResizeBag( vlw, sizeof(Obj)*(max+1) ); - RetypeBag( vlw, T_STRING ); + if ( SIZE_OBJ(vlw) < desiredStackSize ) { + ResizeBag( vlw, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vpw)/sizeof(Obj) < max+1 ) { - ResizeBag( vpw, sizeof(Obj)*(max+1) ); - RetypeBag( vpw, T_STRING ); + if ( SIZE_OBJ(vpw) < desiredStackSize ) { + ResizeBag( vpw, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vew)/sizeof(Obj) < max+1 ) { - ResizeBag( vew, sizeof(Obj)*(max+1) ); - RetypeBag( vew, T_STRING ); + if ( SIZE_OBJ(vew) < desiredStackSize ) { + ResizeBag( vew, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vge)/sizeof(Obj) < max+1 ) { - ResizeBag( vge, sizeof(Obj)*(max+1) ); - RetypeBag( vge, T_STRING ); + if ( SIZE_OBJ(vge) < desiredStackSize ) { + ResizeBag( vge, desiredStackSize ); resized = 1; } if( resized ) return -1; /* from now on we use addresses instead of handles most of the time */ v = (Int*)ADDR_OBJ(vv); - nw = (UIntN**)ADDR_OBJ(vnw); - lw = (UIntN**)ADDR_OBJ(vlw); - pw = (UIntN**)ADDR_OBJ(vpw); - ew = (UIntN*)ADDR_OBJ(vew); - ge = (Int*)ADDR_OBJ(vge); + nw = (UIntN**)(ADDR_OBJ(vnw)+1); + lw = (UIntN**)(ADDR_OBJ(vlw)+1); + pw = (UIntN**)(ADDR_OBJ(vpw)+1); + ew = (UIntN*)(ADDR_OBJ(vew)+1); + ge = (Int*)(ADDR_OBJ(vge)+1); /* conjugates, powers, order, generators, avector, inverses */ vpow = SC_POWERS(sc); diff --git a/src/objscoll-impl.h b/src/objscoll-impl.h index 2f96ceddb8..da208fce48 100644 --- a/src/objscoll-impl.h +++ b/src/objscoll-impl.h @@ -268,42 +268,38 @@ Int SingleCollectWord ( Obj sc, Obj vv, Obj w ) max = STATE(SC_MAX_STACK_SIZE); /* ensure that the stacks are large enough */ - if ( SIZE_OBJ(vnw)/sizeof(Obj) < max+1 ) { - ResizeBag( vnw, sizeof(Obj)*(max+1) ); - RetypeBag( vnw, T_STRING ); + const UInt desiredStackSize = sizeof(Obj) * (max + 2); + if ( SIZE_OBJ(vnw) < desiredStackSize ) { + ResizeBag( vnw, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vlw)/sizeof(Obj) < max+1 ) { - ResizeBag( vlw, sizeof(Obj)*(max+1) ); - RetypeBag( vlw, T_STRING ); + if ( SIZE_OBJ(vlw) < desiredStackSize ) { + ResizeBag( vlw, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vpw)/sizeof(Obj) < max+1 ) { - ResizeBag( vpw, sizeof(Obj)*(max+1) ); - RetypeBag( vpw, T_STRING ); + if ( SIZE_OBJ(vpw) < desiredStackSize ) { + ResizeBag( vpw, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vew)/sizeof(Obj) < max+1 ) { - ResizeBag( vew, sizeof(Obj)*(max+1) ); - RetypeBag( vew, T_STRING ); + if ( SIZE_OBJ(vew) < desiredStackSize ) { + ResizeBag( vew, desiredStackSize ); resized = 1; } - if ( SIZE_OBJ(vge)/sizeof(Obj) < max+1 ) { - ResizeBag( vge, sizeof(Obj)*(max+1) ); - RetypeBag( vge, T_STRING ); + if ( SIZE_OBJ(vge) < desiredStackSize ) { + ResizeBag( vge, desiredStackSize ); resized = 1; } if( resized ) return -1; /* from now on we use addresses instead of handles most of the time */ v = (Int*)ADDR_OBJ(vv); - nw = (UIntN**)ADDR_OBJ(vnw); - lw = (UIntN**)ADDR_OBJ(vlw); - pw = (UIntN**)ADDR_OBJ(vpw); - ew = (UIntN*)ADDR_OBJ(vew); - ge = (Int*)ADDR_OBJ(vge); + nw = (UIntN**)(ADDR_OBJ(vnw)+1); + lw = (UIntN**)(ADDR_OBJ(vlw)+1); + pw = (UIntN**)(ADDR_OBJ(vpw)+1); + ew = (UIntN*)(ADDR_OBJ(vew)+1); + ge = (Int*)(ADDR_OBJ(vge)+1); - /* conjujagtes, powers, order, generators, avector, inverses */ + /* conjuagtes, powers, order, generators, avector, inverses */ vpow = SC_POWERS(sc); lpow = LEN_PLIST(vpow); pow = CONST_ADDR_OBJ(vpow); diff --git a/src/objscoll.c b/src/objscoll.c index 4e2b6225fe..878c2104c5 100644 --- a/src/objscoll.c +++ b/src/objscoll.c @@ -40,6 +40,8 @@ *F * * * * * * * * * * * * local defines and typedefs * * * * * * * * * * * * */ +static Obj TYPE_KERNEL_OBJECT; + /**************************************************************************** ** *T FinPowConjCol @@ -724,19 +726,6 @@ static StructGVarFunc GVarFuncs [] = { }; -/* - * Allocate a Plist of the given length, pre-allocating - * the number of entries given by 'reserved'. - */ -static inline Obj NewPlist( UInt tnum, UInt len, UInt reserved ) -{ - Obj obj; - obj = NEW_PLIST( tnum, reserved ); - SET_LEN_PLIST( obj, len ); - return obj; -} - - /**************************************************************************** ** *F InitKernel( ) . . . . . . . . initialise kernel data structures @@ -747,6 +736,8 @@ static Int InitKernel ( /* init filters and functions */ InitHdlrFuncsFromTable( GVarFuncs ); + ImportGVarFromLibrary( "TYPE_KERNEL_OBJECT", &TYPE_KERNEL_OBJECT ); + /* return success */ return 0; } @@ -810,11 +801,19 @@ static void InitModuleState(ModuleStateOffset offset) #endif const UInt maxStackSize = 256; - STATE(SC_NW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_LW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_PW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_EW_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); - STATE(SC_GE_STACK) = NewPlist(T_PLIST_EMPTY, 0, maxStackSize); + const UInt desiredStackSize = sizeof(Obj) * (maxStackSize + 2); + STATE(SC_NW_STACK) = NewBag(T_DATOBJ, desiredStackSize); + STATE(SC_LW_STACK) = NewBag(T_DATOBJ, desiredStackSize); + STATE(SC_PW_STACK) = NewBag(T_DATOBJ, desiredStackSize); + STATE(SC_EW_STACK) = NewBag(T_DATOBJ, desiredStackSize); + STATE(SC_GE_STACK) = NewBag(T_DATOBJ, desiredStackSize); + + SET_TYPE_DATOBJ(STATE(SC_NW_STACK), TYPE_KERNEL_OBJECT); + SET_TYPE_DATOBJ(STATE(SC_LW_STACK), TYPE_KERNEL_OBJECT); + SET_TYPE_DATOBJ(STATE(SC_PW_STACK), TYPE_KERNEL_OBJECT); + SET_TYPE_DATOBJ(STATE(SC_EW_STACK), TYPE_KERNEL_OBJECT); + SET_TYPE_DATOBJ(STATE(SC_GE_STACK), TYPE_KERNEL_OBJECT); + STATE(SC_CW_VECTOR) = NEW_STRING(0); STATE(SC_CW2_VECTOR) = NEW_STRING(0); STATE(SC_MAX_STACK_SIZE) = maxStackSize; From c8665163fc187bd9f6dd3c81dcd4eed892141b78 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Tue, 15 May 2018 23:12:16 +0200 Subject: [PATCH 3/3] kernel: change pcp collector to use their own stacks ... instead of using the stack objects in the pcp collector objects provided by the polycyclic package. This will make it possible to remove the latter in a future version of polycyclic, thus reducing the size of these objects considerably. --- src/objcftl.c | 57 ++++++++++++++++++++++++++++++++++++++++++--------- src/objcftl.h | 21 +++++++++++++++++++ 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/objcftl.c b/src/objcftl.c index 5f1c5bc8f6..09e63f15c8 100644 --- a/src/objcftl.c +++ b/src/objcftl.c @@ -18,13 +18,30 @@ #include "objcftl.h" #include "ariths.h" +#include "bool.h" #include "error.h" +#include "gapstate.h" #include "gvars.h" #include "integer.h" #include "modules.h" #include "plist.h" +static ModuleStateOffset CFTLStateOffset = -1; + +struct CFTLModuleState { + Obj WORD_STACK; + Obj WORD_EXPONENT_STACK; + Obj SYLLABLE_STACK; + Obj EXPONENT_STACK; +}; + +extern inline struct CFTLModuleState *CFTLState(void) +{ + return (struct CFTLModuleState *)StateSlotsAtOffset(CFTLStateOffset); +} + + #define IS_INT_ZERO( n ) ((n) == INTOBJ_INT(0)) #define GET_COMMUTE( g ) INT_INTOBJ(ELM_PLIST(commute,(g))) @@ -46,6 +63,10 @@ #define PUSH_STACK( word, exp ) { \ st++; \ + GROW_PLIST( wst, st ); \ + GROW_PLIST( west, st ); \ + GROW_PLIST( sst, st ); \ + GROW_PLIST( est, st ); \ SET_ELM_PLIST( wst, st, word ); \ SET_ELM_PLIST( west, st, exp ); \ SET_ELM_PLIST( sst, st, INTOBJ_INT(1) ); \ @@ -87,14 +108,14 @@ Obj CollectPolycyc ( Obj ipow = ADDR_OBJ(pcp)[ PC_INVERSEPOWERS ]; Obj exp = ADDR_OBJ(pcp)[ PC_EXPONENTS ]; - Obj wst = ADDR_OBJ(pcp)[ PC_WORD_STACK ]; - Obj west = ADDR_OBJ(pcp)[ PC_WORD_EXPONENT_STACK ]; - Obj sst = ADDR_OBJ(pcp)[ PC_SYLLABLE_STACK ]; - Obj est = ADDR_OBJ(pcp)[ PC_EXPONENT_STACK ]; + Obj wst = CFTLState()->WORD_STACK; + Obj west = CFTLState()->WORD_EXPONENT_STACK; + Obj sst = CFTLState()->SYLLABLE_STACK; + Obj est = CFTLState()->EXPONENT_STACK; Obj conj=0, iconj=0; /*QQ initialize to please compiler */ - Int st, bottom = INT_INTOBJ( ADDR_OBJ(pcp)[ PC_STACK_POINTER ] ); + Int st; Int g, syl, h, hh; @@ -113,16 +134,16 @@ Obj CollectPolycyc ( return (Obj)0; } - st = bottom; + st = 0; PUSH_STACK( word, INTOBJ_INT(1) ); - while( st > bottom ) { + while( st > 0 ) { w = ELM_PLIST( wst, st ); syl = INT_INTOBJ( ELM_PLIST( sst, st ) ); g = INT_INTOBJ( ELM_PLIST( w, syl ) ); - if( st > bottom+1 && syl==1 && g == GET_COMMUTE(g) ) { + if( st > 1 && syl==1 && g == GET_COMMUTE(g) ) { /* Collect word^exponent in one go. */ e = ELM_PLIST( west, st ); @@ -275,7 +296,7 @@ Obj CollectPolycyc ( if( y != (Obj)0 ) PUSH_STACK( y, ge ); } - while( st > bottom && IS_INT_ZERO( ELM_PLIST( est, st ) ) ) { + while( st > 0 && IS_INT_ZERO( ELM_PLIST( est, st ) ) ) { w = ELM_PLIST( wst, st ); syl = INT_INTOBJ( ELM_PLIST( sst, st ) ) + 2; if( syl > LEN_PLIST( w ) ) { @@ -296,7 +317,6 @@ Obj CollectPolycyc ( } } - ADDR_OBJ(pcp)[ PC_STACK_POINTER ] = INTOBJ_INT( bottom ); return (Obj)0; } @@ -387,6 +407,10 @@ static Int InitLibrary ( ExportAsConstantGVar(PC_STACK_POINTER); ExportAsConstantGVar(PC_DEFAULT_TYPE); + // signal to polycyclic that 'CollectPolycyclic' does not use resp. + // require stacks inside the collector objects + AssConstantGVar(GVarName("NO_STACKS_INSIDE_COLLECTORS"), True); + /* init filters and functions */ InitGVarFuncsFromTable( GVarFuncs ); @@ -394,6 +418,18 @@ static Int InitLibrary ( return 0; } +static void InitModuleState(ModuleStateOffset offset) +{ + InitGlobalBag( &CFTLState()->WORD_STACK, "WORD_STACK" ); + InitGlobalBag( &CFTLState()->WORD_EXPONENT_STACK, "WORD_EXPONENT_STACK" ); + InitGlobalBag( &CFTLState()->SYLLABLE_STACK, "SYLLABLE_STACK" ); + InitGlobalBag( &CFTLState()->EXPONENT_STACK, "EXPONENT_STACK" ); + + CFTLState()->WORD_STACK = NEW_PLIST( T_PLIST, 4096 ); + CFTLState()->WORD_EXPONENT_STACK = NEW_PLIST( T_PLIST, 4096 ); + CFTLState()->SYLLABLE_STACK = NEW_PLIST( T_PLIST, 4096 ); + CFTLState()->EXPONENT_STACK = NEW_PLIST( T_PLIST, 4096 ); +} /**************************************************************************** ** @@ -411,5 +447,6 @@ static StructInitInfo module = { StructInitInfo * InitInfoPcc ( void ) { + CFTLStateOffset = RegisterModuleState(sizeof(struct CFTLModuleState), InitModuleState, 0); return &module; } diff --git a/src/objcftl.h b/src/objcftl.h index ae3d4d2085..e5f588c52e 100644 --- a/src/objcftl.h +++ b/src/objcftl.h @@ -27,14 +27,35 @@ #define PC_DEEP_THOUGHT_BOUND 13 #define PC_ORDERS 14 +// TODO: the following stack related positions are not +// used anymore by CollectPolycyc, and will not be used +// anymore in future releases of the polycyclic package. +// We should phase them out for GAP 4.10. #define PC_WORD_STACK 15 #define PC_STACK_SIZE 16 #define PC_WORD_EXPONENT_STACK 17 #define PC_SYLLABLE_STACK 18 #define PC_EXPONENT_STACK 19 #define PC_STACK_POINTER 20 +// TODO: end obsolete + #define PC_DEFAULT_TYPE 21 +/* the following are defined in polycyclic: + +#define PC_PCP_ELEMENTS_FAMILY 22 +#define PC_PCP_ELEMENTS_TYPE 23 + +#define PC_COMMUTATORS 24 +#define PC_INVERSECOMMUTATORS 25 +#define PC_COMMUTATORSINVERSE 26 +#define PC_INVERSECOMMUTATORSINVERSE 27 + +#define PC_NILPOTENT_COMMUTE 28 +#define PC_WEIGHTS 29 +#define PC_ABELIAN_START 30 +*/ + /**************************************************************************** ** *F * * * * * * * * * * * * * initialize module * * * * * * * * * * * * * * *