From 3b2fffa448b1682797448743805d4f5f5e12df38 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:25 +0200 Subject: [PATCH 1/8] kernel: add code to debug subbag marking in GASMAN If DEBUG_GASMAN_MARKING is #defined, then we increment a counter whenever `MarkBag` is called on something that isn't either zero, or a valid bag ref, or an immediate integer or FFE. By setting a break point on the line incrementing BadMarksCounter, one can quickly find out which bags are affected. --- src/gasman.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/gasman.c b/src/gasman.c index 3dc4d492e6..ebc6bb87ad 100644 --- a/src/gasman.c +++ b/src/gasman.c @@ -660,6 +660,12 @@ void MarkAllSubBagsDefault(Bag bag) MarkArrayOfBags(CONST_PTR_BAG(bag), SIZE_BAG(bag) / sizeof(Bag)); } +#ifdef DEBUG_GASMAN_MARKING +UInt BadMarksCounter = 0; +Int DisableMarkBagValidation = 0; +#endif + + // We define MarkBag as a inline function here so that // the compiler can optimize the marking functions using it in the // "current translation unit", i.e. inside gasman.c. @@ -676,6 +682,13 @@ inline void MarkBag(Bag bag) LINK_BAG(bag) = MarkedBags; MarkedBags = bag; } +#ifdef DEBUG_GASMAN_MARKING + else if (!DisableMarkBagValidation) { + if (bag != 0 && !((UInt)bag & 3) && !IS_BAG_ID(bag)) { + BadMarksCounter++; + } + } +#endif } void MarkBagWeakly(Bag bag) @@ -1755,6 +1768,10 @@ void GenStackFuncBags ( void ) Bag * p; /* loop variable */ UInt i; /* loop variable */ +#ifdef DEBUG_GASMAN_MARKING + DisableMarkBagValidation = 1; +#endif + top = (Bag*)((void*)&top); if ( StackBottomBags < top ) { for ( i = 0; i < sizeof(Bag*); i += StackAlignBags ) { @@ -1775,6 +1792,9 @@ void GenStackFuncBags ( void ) p++ ) MarkBag( *p ); +#ifdef DEBUG_GASMAN_MARKING + DisableMarkBagValidation = 0; +#endif } UInt FullBags; From 7fe50d065182825dca90ca3c1ad10f7f1ee5fd2e Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:28 +0200 Subject: [PATCH 2/8] kernel: use custom GC marking for T_FUNCTION Not all slots of a T_FUNCTION bag are filled with bag refs, yet when marking the slots of such a bag during garbage collection, we still used MarkAllSubBags. This is usually no problem with GASMAN, as it detects if a pointer isn't a master pointer, and can then simply ignore it. But other garbage collections can't as easily verify master pointers. So let's try to be accurate about what we mark as a bag and what not: skip the first eight slots of every T_FUNCTION bag for marking (they contain pointers to C functions), and also ensure that the rest of any T_FUNCTION bag only contains bag refs. Also fix a bug in saving/restoring operations, where the 'enabled' field was stored as an UInt, even though it now is an Obj (though we currently only store immediate ints in it, hence there was no functional problem). --- src/calls.c | 29 +++++++++++++++++++++++------ src/calls.h | 12 ++++++------ src/opers.c | 49 ++++++++++++++++++++++++------------------------- 3 files changed, 53 insertions(+), 37 deletions(-) diff --git a/src/calls.c b/src/calls.c index 830f2ba28a..be94820394 100644 --- a/src/calls.c +++ b/src/calls.c @@ -957,6 +957,7 @@ Obj NewFunctionT ( SET_NAME_FUNC(func, ConvImmString(name)); SET_NARG_FUNC(func, narg); SET_NAMS_FUNC(func, nams); + SET_NLOC_FUNC(func, 0); if (nams) MakeBagPublic(nams); CHANGED_BAG(func); @@ -1547,6 +1548,7 @@ Obj FuncPROFILE_FUNC( SET_NARG_FUNC(copy, NARG_FUNC(func)); SET_NAMS_FUNC(copy, NAMS_FUNC(func)); SET_PROF_FUNC(copy, PROF_FUNC(func)); + SET_NLOC_FUNC(copy, NLOC_FUNC(func)); SET_HDLR_FUNC(func,0, DoProf0args); SET_HDLR_FUNC(func,1, DoProf1args); SET_HDLR_FUNC(func,2, DoProf2args); @@ -1730,14 +1732,14 @@ static ObjFunc LoadHandler( void ) */ void SaveFunction ( Obj func ) { - FuncBag * header = FUNC(func); + const FuncBag * header = CONST_FUNC(func); for (UInt i = 0; i <= 7; i++) SaveHandler(header->handlers[i]); SaveSubObj(header->name); - SaveUInt(header->nargs); + SaveSubObj(header->nargs); SaveSubObj(header->namesOfLocals); SaveSubObj(header->prof); - SaveUInt(header->nloc); + SaveSubObj(header->nloc); SaveSubObj(header->body); SaveSubObj(header->envi); SaveSubObj(header->fexs); @@ -1756,10 +1758,10 @@ void LoadFunction ( Obj func ) for (UInt i = 0; i <= 7; i++) header->handlers[i] = LoadHandler(); header->name = LoadSubObj(); - header->nargs = LoadUInt(); + header->nargs = LoadSubObj(); header->namesOfLocals = LoadSubObj(); header->prof = LoadSubObj(); - header->nloc = LoadUInt(); + header->nloc = LoadSubObj(); header->body = LoadSubObj(); header->envi = LoadSubObj(); header->fexs = LoadSubObj(); @@ -1769,6 +1771,21 @@ void LoadFunction ( Obj func ) #endif +/**************************************************************************** +** +*F MarkFunctionSubBags( ) . . . . . . . marking function for functions +** +** 'MarkFunctionSubBags' is the marking function for bags of type 'T_FUNCTION'. +*/ +void MarkFunctionSubBags(Obj func) +{ + // the first eight slots are pointers to C functions, so we need + // to skip those for marking + UInt size = SIZE_BAG(func) / sizeof(Obj) - 8; + const Bag * data = CONST_PTR_BAG(func) + 8; + MarkArrayOfBags(data, size); +} + /**************************************************************************** ** @@ -1836,7 +1853,7 @@ static Int InitKernel ( /* install the marking functions */ InfoBags[ T_FUNCTION ].name = "function"; - InitMarkFuncBags( T_FUNCTION , MarkAllSubBags ); + InitMarkFuncBags(T_FUNCTION, MarkFunctionSubBags); /* Allocate functions in the public region */ MakeBagTypePublic(T_FUNCTION); diff --git a/src/calls.h b/src/calls.h index 593f048877..1b44eeed2d 100644 --- a/src/calls.h +++ b/src/calls.h @@ -107,10 +107,10 @@ typedef Obj (* ObjFunc_6ARGS) (Obj self, Obj a1, Obj a2, Obj a3, Obj a4, Obj a5, typedef struct { ObjFunc handlers[8]; Obj name; - Int nargs; + Obj nargs; Obj namesOfLocals; Obj prof; - UInt nloc; + Obj nloc; Obj body; Obj envi; Obj fexs; @@ -146,7 +146,7 @@ static inline Obj NAME_FUNC(Obj func) static inline Int NARG_FUNC(Obj func) { - return CONST_FUNC(func)->nargs; + return INT_INTOBJ(CONST_FUNC(func)->nargs); } static inline Obj NAMS_FUNC(Obj func) @@ -163,7 +163,7 @@ static inline Obj PROF_FUNC(Obj func) static inline UInt NLOC_FUNC(Obj func) { - return CONST_FUNC(func)->nloc; + return INT_INTOBJ(CONST_FUNC(func)->nloc); } static inline Obj BODY_FUNC(Obj func) @@ -199,7 +199,7 @@ extern void SET_NAME_FUNC(Obj func, Obj name); static inline void SET_NARG_FUNC(Obj func, Int nargs) { - FUNC(func)->nargs = nargs; + FUNC(func)->nargs = INTOBJ_INT(nargs); } static inline void SET_NAMS_FUNC(Obj func, Obj namesOfLocals) @@ -214,7 +214,7 @@ static inline void SET_PROF_FUNC(Obj func, Obj prof) static inline void SET_NLOC_FUNC(Obj func, UInt nloc) { - FUNC(func)->nloc = nloc; + FUNC(func)->nloc = INTOBJ_INT(nloc); } static inline void SET_BODY_FUNC(Obj func, Obj body) diff --git a/src/opers.c b/src/opers.c index 9470c70d64..e9acf8429f 100644 --- a/src/opers.c +++ b/src/opers.c @@ -3313,22 +3313,22 @@ void InstallGlobalFunction ( void SaveOperationExtras ( Obj oper ) { - UInt i; - - SaveSubObj(FLAG1_FILT(oper)); - SaveSubObj(FLAG2_FILT(oper)); - SaveSubObj(FLAGS_FILT(oper)); - SaveSubObj(SETTR_FILT(oper)); - SaveSubObj(TESTR_FILT(oper)); - SaveUInt(ENABLED_ATTR(oper)); - for (i = 0; i <= 7; i++) - SaveSubObj(METHS_OPER(oper,i)); + const OperBag * header = CONST_OPER(oper); + + SaveSubObj(header->flag1); + SaveSubObj(header->flag2); + SaveSubObj(header->flags); + SaveSubObj(header->setter); + SaveSubObj(header->tester); + SaveSubObj(header->enabled); + for (UInt i = 0; i <= 7; i++) + SaveSubObj(header->methods[i]); #ifdef HPCGAP // FIXME: We probably don't want to save/restore the cache? // (and that would include "normal" GAP, too...) #else - for (i = 0; i <= 7; i++) - SaveSubObj(CACHE_OPER(oper,i)); + for (UInt i = 0; i <= 7; i++) + SaveSubObj(header->cache[i]); #endif } @@ -3344,23 +3344,22 @@ void SaveOperationExtras ( void LoadOperationExtras ( Obj oper ) { - UInt i; - - SET_FLAG1_FILT(oper, LoadSubObj()); - SET_FLAG2_FILT(oper, LoadSubObj()); - SET_FLAGS_FILT(oper, LoadSubObj()); - SET_SETTR_FILT(oper, LoadSubObj()); - SET_TESTR_FILT(oper, LoadSubObj()); - i = LoadUInt(); - SET_ENABLED_ATTR(oper,i); - for (i = 0; i <= 7; i++) - SET_METHS_OPER(oper, i, LoadSubObj()); + OperBag * header = OPER(oper); + + header->flag1 = LoadSubObj(); + header->flag2 = LoadSubObj(); + header->flags = LoadSubObj(); + header->setter = LoadSubObj(); + header->tester = LoadSubObj(); + header->enabled = LoadSubObj(); + for (UInt i = 0; i <= 7; i++) + header->methods[i] = LoadSubObj(); #ifdef HPCGAP // FIXME: We probably don't want to save/restore the cache? // (and that would include "normal" GAP, too...) #else - for (i = 0; i <= 7; i++) - SET_CACHE_OPER(oper, i, LoadSubObj()); + for (UInt i = 0; i <= 7; i++) + header->cache[i] = LoadSubObj(); #endif } From 9888677df2a196f0dd944159e41e9eea036ff62d Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:30 +0200 Subject: [PATCH 3/8] kernel: unify implementation of MarkNNNSubBags They now all call MarkArrayOfBags, but thanks to inlining, this produces identical machine code. --- src/gasman.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/gasman.c b/src/gasman.c index ebc6bb87ad..abe09cb1ec 100644 --- a/src/gasman.c +++ b/src/gasman.c @@ -613,41 +613,35 @@ static inline Bag UNMARKED_HALFDEAD(Bag x) } +inline void MarkArrayOfBags(const Bag array[], UInt count) +{ + for (UInt i = 0; i < count; i++) { + MarkBag(array[i]); + } +} + void MarkNoSubBags(Bag bag) { } void MarkOneSubBags(Bag bag) { - MarkBag(CONST_PTR_BAG(bag)[0]); + MarkArrayOfBags(CONST_PTR_BAG(bag), 1); } void MarkTwoSubBags(Bag bag) { - MarkBag(CONST_PTR_BAG(bag)[0]); - MarkBag(CONST_PTR_BAG(bag)[1]); + MarkArrayOfBags(CONST_PTR_BAG(bag), 2); } void MarkThreeSubBags(Bag bag) { - MarkBag(CONST_PTR_BAG(bag)[0]); - MarkBag(CONST_PTR_BAG(bag)[1]); - MarkBag(CONST_PTR_BAG(bag)[2]); + MarkArrayOfBags(CONST_PTR_BAG(bag), 3); } void MarkFourSubBags(Bag bag) { - MarkBag(CONST_PTR_BAG(bag)[0]); - MarkBag(CONST_PTR_BAG(bag)[1]); - MarkBag(CONST_PTR_BAG(bag)[2]); - MarkBag(CONST_PTR_BAG(bag)[3]); -} - -inline void MarkArrayOfBags(const Bag array[], UInt count) -{ - for (UInt i = 0; i < count; i++) { - MarkBag(array[i]); - } + MarkArrayOfBags(CONST_PTR_BAG(bag), 4); } void MarkAllSubBags(Bag bag) From be15779a9c389530ec59a10443fcdd62c095bece Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:31 +0200 Subject: [PATCH 4/8] kernel: refactor StatStack/ExprStack code --- src/code.c | 57 +++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/src/code.c b/src/code.c index 0b04352fc2..ea77bdac6f 100644 --- a/src/code.c +++ b/src/code.c @@ -294,9 +294,10 @@ Expr NewExpr ( ** 'PopStat' returns the top statement from the statements stack and pops ** it. It is an error if the stack is empty. */ -/* TL: Bag StackStat; */ - -/* TL: Int CountStat; */ +static inline UInt CapacityStatStack(void) +{ + return SIZE_BAG(STATE(StackStat))/sizeof(Stat); +} static void PushStat ( Stat stat ) @@ -304,14 +305,17 @@ static void PushStat ( /* there must be a stack, it must not be underfull or overfull */ assert( STATE(StackStat) != 0 ); assert( 0 <= STATE(CountStat) ); - assert( STATE(CountStat) <= SIZE_BAG(STATE(StackStat))/sizeof(Stat) ); + assert( STATE(CountStat) <= CapacityStatStack() ); assert( stat != 0 ); - /* count up and put the statement onto the stack */ - if ( STATE(CountStat) == SIZE_BAG(STATE(StackStat))/sizeof(Stat) ) { + // count up and put the statement onto the stack + if ( STATE(CountStat) == CapacityStatStack() ) { ResizeBag( STATE(StackStat), 2*STATE(CountStat)*sizeof(Stat) ); } - ((Stat*)PTR_BAG(STATE(StackStat)))[STATE(CountStat)] = stat; + + // put + Stat * data = (Stat *)PTR_BAG(STATE(StackStat)); + data[STATE(CountStat)] = stat; STATE(CountStat)++; } @@ -322,11 +326,12 @@ static Stat PopStat ( void ) /* there must be a stack, it must not be underfull/empty or overfull */ assert( STATE(StackStat) != 0 ); assert( 1 <= STATE(CountStat) ); - assert( STATE(CountStat) <= SIZE_BAG(STATE(StackStat))/sizeof(Stat) ); + assert( STATE(CountStat) <= CapacityStatStack() ); /* get the top statement from the stack, and count down */ STATE(CountStat)--; - stat = ((Stat*)PTR_BAG(STATE(StackStat)))[STATE(CountStat)]; + Stat * data = (Stat *)PTR_BAG(STATE(StackStat)); + stat = data[STATE(CountStat)]; /* return the popped statement */ return stat; @@ -415,39 +420,42 @@ static inline Stat PopLoopStat(UInt baseType, UInt extra, UInt nr) ** 'PopExpr' returns the top expressions from the expressions stack and pops ** it. It is an error if the stack is empty. */ -/* TL: Bag StackExpr; */ - -/* TL: Int CountExpr; */ +static inline UInt CapacityStackExpr(void) +{ + return SIZE_BAG(STATE(StackExpr))/sizeof(Expr); +} -void PushExpr ( - Expr expr ) +static void PushExpr(Expr expr) { /* there must be a stack, it must not be underfull or overfull */ assert( STATE(StackExpr) != 0 ); assert( 0 <= STATE(CountExpr) ); - assert( STATE(CountExpr) <= SIZE_BAG(STATE(StackExpr))/sizeof(Expr) ); + assert( STATE(CountExpr) <= CapacityStackExpr() ); assert( expr != 0 ); /* count up and put the expression onto the stack */ - if ( STATE(CountExpr) == SIZE_BAG(STATE(StackExpr))/sizeof(Expr) ) { + if ( STATE(CountExpr) == CapacityStackExpr() ) { ResizeBag( STATE(StackExpr), 2*STATE(CountExpr)*sizeof(Expr) ); } - ((Expr*)PTR_BAG(STATE(StackExpr)))[STATE(CountExpr)] = expr; + + Expr * data = (Expr *)PTR_BAG(STATE(StackExpr)); + data[STATE(CountExpr)] = expr; STATE(CountExpr)++; } -Expr PopExpr ( void ) +static Expr PopExpr(void) { Expr expr; /* there must be a stack, it must not be underfull/empty or overfull */ assert( STATE(StackExpr) != 0 ); assert( 1 <= STATE(CountExpr) ); - assert( STATE(CountExpr) <= SIZE_BAG(STATE(StackExpr))/sizeof(Expr) ); + assert( STATE(CountExpr) <= CapacityStackExpr() ); /* get the top expression from the stack, and count down */ STATE(CountExpr)--; - expr = ((Expr*)PTR_BAG(STATE(StackExpr)))[STATE(CountExpr)]; + Expr * data = (Expr *)PTR_BAG(STATE(StackExpr)); + expr = data[STATE(CountExpr)]; /* return the popped expression */ return expr; @@ -3401,8 +3409,6 @@ static Int PostRestore ( static Int PreSave ( StructInitInfo * module ) { - UInt i; - /* Can't save in mid-parsing */ if (STATE(CountExpr) || STATE(CountStat)) return 1; @@ -3411,10 +3417,9 @@ static Int PreSave ( AssGVar(GVarName("SavedFloatIndex"), INTOBJ_INT(NextFloatExprNumber)); /* clean any old data out of the statement and expression stacks */ - for (i = 0; i < SIZE_BAG(STATE(StackStat))/sizeof(UInt); i++) - ADDR_OBJ(STATE(StackStat))[i] = (Obj)0; - for (i = 0; i < SIZE_BAG(STATE(StackExpr))/sizeof(UInt); i++) - ADDR_OBJ(STATE(StackExpr))[i] = (Obj)0; + memset(ADDR_OBJ(STATE(StackStat)), 0, SIZE_BAG(STATE(StackStat))); + memset(ADDR_OBJ(STATE(StackExpr)), 0, SIZE_BAG(STATE(StackExpr))); + /* return success */ return 0; } From b3a2362133948255ce1574ae638eccc8b67c8044 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:32 +0200 Subject: [PATCH 5/8] kernel: use T_DATOBJ for StackStat and StackExpr ... instead of a T_BODY, as we don't really create a T_BODY here. (and this can lead to confusion in GC marking) --- src/code.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/code.c b/src/code.c index ea77bdac6f..ddc4cd8fe3 100644 --- a/src/code.c +++ b/src/code.c @@ -43,6 +43,10 @@ GAP_STATIC_ASSERT(sizeof(StatHeader) == 8, "StatHeader has wrong size"); + +static Obj TYPE_KERNEL_OBJECT; + + /**************************************************************************** ** *V PtrBody . . . . . . . . . . . . . . . . . . . . . pointer to current body @@ -296,7 +300,7 @@ Expr NewExpr ( */ static inline UInt CapacityStatStack(void) { - return SIZE_BAG(STATE(StackStat))/sizeof(Stat); + return SIZE_BAG(STATE(StackStat))/sizeof(Stat) - 1; } static void PushStat ( @@ -310,11 +314,11 @@ static void PushStat ( // count up and put the statement onto the stack if ( STATE(CountStat) == CapacityStatStack() ) { - ResizeBag( STATE(StackStat), 2*STATE(CountStat)*sizeof(Stat) ); + ResizeBag( STATE(StackStat), (2*STATE(CountStat) + 1)*sizeof(Stat) ); } // put - Stat * data = (Stat *)PTR_BAG(STATE(StackStat)); + Stat * data = (Stat *)PTR_BAG(STATE(StackStat)) + 1; data[STATE(CountStat)] = stat; STATE(CountStat)++; } @@ -330,7 +334,7 @@ static Stat PopStat ( void ) /* get the top statement from the stack, and count down */ STATE(CountStat)--; - Stat * data = (Stat *)PTR_BAG(STATE(StackStat)); + Stat * data = (Stat *)PTR_BAG(STATE(StackStat)) + 1; stat = data[STATE(CountStat)]; /* return the popped statement */ @@ -422,7 +426,7 @@ static inline Stat PopLoopStat(UInt baseType, UInt extra, UInt nr) */ static inline UInt CapacityStackExpr(void) { - return SIZE_BAG(STATE(StackExpr))/sizeof(Expr); + return SIZE_BAG(STATE(StackExpr))/sizeof(Expr) - 1; } static void PushExpr(Expr expr) @@ -435,10 +439,10 @@ static void PushExpr(Expr expr) /* count up and put the expression onto the stack */ if ( STATE(CountExpr) == CapacityStackExpr() ) { - ResizeBag( STATE(StackExpr), 2*STATE(CountExpr)*sizeof(Expr) ); + ResizeBag( STATE(StackExpr), (2*STATE(CountExpr) + 1)*sizeof(Expr) ); } - Expr * data = (Expr *)PTR_BAG(STATE(StackExpr)); + Expr * data = (Expr *)PTR_BAG(STATE(StackExpr)) + 1; data[STATE(CountExpr)] = expr; STATE(CountExpr)++; } @@ -454,7 +458,7 @@ static Expr PopExpr(void) /* get the top expression from the stack, and count down */ STATE(CountExpr)--; - Expr * data = (Expr *)PTR_BAG(STATE(StackExpr)); + Expr * data = (Expr *)PTR_BAG(STATE(StackExpr)) + 1; expr = data[STATE(CountExpr)]; /* return the popped expression */ @@ -3355,6 +3359,8 @@ static Int InitKernel ( InitHdlrFuncsFromTable( GVarFuncs ); + ImportGVarFromLibrary( "TYPE_KERNEL_OBJECT", &TYPE_KERNEL_OBJECT ); + /* return success */ return 0; } @@ -3416,9 +3422,10 @@ static Int PreSave ( /* push the FP cache index out into a GAP Variable */ AssGVar(GVarName("SavedFloatIndex"), INTOBJ_INT(NextFloatExprNumber)); - /* clean any old data out of the statement and expression stacks */ - memset(ADDR_OBJ(STATE(StackStat)), 0, SIZE_BAG(STATE(StackStat))); - memset(ADDR_OBJ(STATE(StackExpr)), 0, SIZE_BAG(STATE(StackExpr))); + // clean any old data out of the statement and expression stacks, + // but leave the type field alone + memset(ADDR_OBJ(STATE(StackStat)) + 1, 0, SIZE_BAG(STATE(StackStat)) - sizeof(Obj)); + memset(ADDR_OBJ(STATE(StackExpr)) + 1, 0, SIZE_BAG(STATE(StackExpr)) - sizeof(Obj)); /* return success */ return 0; @@ -3430,9 +3437,11 @@ static void InitModuleState(ModuleStateOffset offset) STATE(LoopNesting) = 0; STATE(LoopStackCount) = 0; - /* allocate the statements and expressions stacks */ - STATE(StackStat) = NewBag( T_BODY, 64*sizeof(Stat) ); - STATE(StackExpr) = NewBag( T_BODY, 64*sizeof(Expr) ); + // allocate the statements and expressions stacks + STATE(StackStat) = NewBag(T_DATOBJ, sizeof(Obj) + 64*sizeof(Stat)); + STATE(StackExpr) = NewBag(T_DATOBJ, sizeof(Obj) + 64*sizeof(Expr)); + SET_TYPE_DATOBJ(STATE(StackStat), TYPE_KERNEL_OBJECT); + SET_TYPE_DATOBJ(STATE(StackExpr), TYPE_KERNEL_OBJECT); #ifdef HPCGAP STATE(OffsBodyStack) = AllocateMemoryBlock(MAX_FUNC_EXPR_NESTING*sizeof(Stat)); From f6993c28cff3e4b719639c182de241ed6b87c9d2 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:34 +0200 Subject: [PATCH 6/8] kernel: fix GC marking of plists --- src/boehm_gc.c | 4 ++++ src/gasman.c | 5 +++++ src/gasman.h | 2 ++ src/plist.c | 4 ++-- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/boehm_gc.c b/src/boehm_gc.c index 0b651cef81..aedbbc9d20 100644 --- a/src/boehm_gc.c +++ b/src/boehm_gc.c @@ -625,6 +625,10 @@ void MarkAllSubBags(Bag bag) { } +void MarkAllButFirstSubBags(Bag bag) +{ +} + void MarkArrayOfBags(const Bag array[], UInt count) { } diff --git a/src/gasman.c b/src/gasman.c index abe09cb1ec..ea9460b518 100644 --- a/src/gasman.c +++ b/src/gasman.c @@ -649,6 +649,11 @@ void MarkAllSubBags(Bag bag) MarkArrayOfBags(CONST_PTR_BAG(bag), SIZE_BAG(bag) / sizeof(Bag)); } +void MarkAllButFirstSubBags(Bag bag) +{ + MarkArrayOfBags(CONST_PTR_BAG(bag) + 1, SIZE_BAG(bag) / sizeof(Bag) - 1); +} + void MarkAllSubBagsDefault(Bag bag) { MarkArrayOfBags(CONST_PTR_BAG(bag), SIZE_BAG(bag) / sizeof(Bag)); diff --git a/src/gasman.h b/src/gasman.h index 712f83be02..1eb8a69773 100644 --- a/src/gasman.h +++ b/src/gasman.h @@ -806,6 +806,8 @@ extern void MarkAllSubBags( Bag bag ); extern void MarkAllSubBagsDefault ( Bag ); +extern void MarkAllButFirstSubBags( Bag bag ); + /**************************************************************************** ** *F MarkBag() . . . . . . . . . . . . . . . . . . . mark a bag as live diff --git a/src/plist.c b/src/plist.c index 97df1b8eec..abc894cfcb 100644 --- a/src/plist.c +++ b/src/plist.c @@ -3644,8 +3644,8 @@ static Int InitKernel ( InitBagNamesFromTable( BagNames ); for ( t1 = T_PLIST; t1 < T_PLIST_FFE ; t1 += 2 ) { - InitMarkFuncBags( t1 , MarkAllSubBags ); - InitMarkFuncBags( t1 +IMMUTABLE , MarkAllSubBags ); + InitMarkFuncBags( t1 , MarkAllButFirstSubBags ); + InitMarkFuncBags( t1 +IMMUTABLE , MarkAllButFirstSubBags ); #if !defined(USE_THREADSAFE_COPYING) InitMarkFuncBags( t1 +COPYING , MarkAllSubBags ); InitMarkFuncBags( t1 +IMMUTABLE +COPYING , MarkAllSubBags ); From e87de2c584229784cf0f51d944ace2a3b0292875 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:35 +0200 Subject: [PATCH 7/8] kernel: fix marking of precords and component objects --- src/objects.c | 4 ++-- src/precord.c | 31 +++++++++++++++++++++++++++---- src/precord.h | 4 ++++ 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/objects.c b/src/objects.c index 8463ead514..5a5076a0d3 100644 --- a/src/objects.c +++ b/src/objects.c @@ -2009,14 +2009,14 @@ static Int InitKernel ( /* install the marking methods */ InfoBags[ T_COMOBJ ].name = "object (component)"; - InitMarkFuncBags( T_COMOBJ , MarkAllSubBags ); + InitMarkFuncBags( T_COMOBJ , MarkPRecSubBags ); InfoBags[ T_POSOBJ ].name = "object (positional)"; InitMarkFuncBags( T_POSOBJ , MarkAllSubBags ); InfoBags[ T_DATOBJ ].name = "object (data)"; InitMarkFuncBags( T_DATOBJ , MarkOneSubBags ); #if !defined(USE_THREADSAFE_COPYING) InfoBags[ T_COMOBJ +COPYING ].name = "object (component,copied)"; - InitMarkFuncBags( T_COMOBJ +COPYING , MarkAllSubBags ); + InitMarkFuncBags( T_COMOBJ +COPYING , MarkPRecSubBags ); InfoBags[ T_POSOBJ +COPYING ].name = "object (positional,copied)"; InitMarkFuncBags( T_POSOBJ +COPYING , MarkAllSubBags ); InfoBags[ T_DATOBJ +COPYING ].name = "object (data,copied)"; diff --git a/src/precord.c b/src/precord.c index c10c42ce24..ee3558a57e 100644 --- a/src/precord.c +++ b/src/precord.c @@ -821,6 +821,29 @@ void LoadPRec( Obj prec ) } } +/**************************************************************************** +** +*F MarkPRecSubBags( ) . . . . marking function for precs and com. objs +** +** 'MarkPRecSubBags' is the marking function for bags of type 'T_PREC' or +** 'T_COMOBJ'. +*/ +void MarkPRecSubBags(Obj bag) +{ + const Bag * data = CONST_PTR_BAG(bag); + const UInt count = SIZE_BAG(bag) / sizeof(Bag); + + // while data[0] is unused for regular precords, it used during copying + // to store a pointer to the copy; moreover, this mark function is also + // used for component objects, which store their type in slot 0 + MarkBag(data[0]); + + for (UInt i = 3; i < count; i += 2) { + MarkBag(data[i]); + } +} + + /**************************************************************************** ** *F * * * * * * * * * * * * * initialize module * * * * * * * * * * * * * * * @@ -864,11 +887,11 @@ static Int InitKernel ( /* GASMAN marking functions and GASMAN names */ InitBagNamesFromTable( BagNames ); - InitMarkFuncBags( T_PREC , MarkAllSubBags ); - InitMarkFuncBags( T_PREC +IMMUTABLE , MarkAllSubBags ); + InitMarkFuncBags( T_PREC , MarkPRecSubBags ); + InitMarkFuncBags( T_PREC +IMMUTABLE , MarkPRecSubBags ); #if !defined(USE_THREADSAFE_COPYING) - InitMarkFuncBags( T_PREC +COPYING , MarkAllSubBags ); - InitMarkFuncBags( T_PREC +IMMUTABLE +COPYING , MarkAllSubBags ); + InitMarkFuncBags( T_PREC +COPYING , MarkPRecSubBags ); + InitMarkFuncBags( T_PREC +IMMUTABLE +COPYING , MarkPRecSubBags ); #endif /* Immutable records are public */ diff --git a/src/precord.h b/src/precord.h index e753f5c376..f2d23e9769 100644 --- a/src/precord.h +++ b/src/precord.h @@ -260,6 +260,10 @@ extern void CopyPRecord(Obj copy, Obj original); #endif + +extern void MarkPRecSubBags(Obj bag); + + /**************************************************************************** ** *F * * * * * * * * * * * * * initialize module * * * * * * * * * * * * * * * From cf2c8c1987b94585609a5f31dbfbb2f642de055a Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 27 Apr 2018 00:59:37 +0200 Subject: [PATCH 8/8] kernel: fix marking of LVars/HVars --- src/vars.c | 62 ++++++++++++++++++++++++++++++++++++++---------------- src/vars.h | 2 +- 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/src/vars.c b/src/vars.c index b45e1886e9..c028fd911f 100644 --- a/src/vars.c +++ b/src/vars.c @@ -102,25 +102,51 @@ Obj ObjLVar ( return val; } -Bag NewLVarsBag(UInt slots) { - Bag result; - if (slots < ARRAY_SIZE(STATE(LVarsPool))) { - result = STATE(LVarsPool)[slots]; - if (result) { - STATE(LVarsPool)[slots] = CONST_ADDR_OBJ(result)[0]; - return result; + +/**************************************************************************** +** +*F NewLVarsBag() . . . . . . . . . . . . . . allocate a new LVars bag +** +** 'NewLVarsBag' allocates a new 'T_LVAR' bag, with the given number of +** local variable . It tries to satisfy the request from a pool of +** available LVars with up to 16 slots. If the request cannot be satisfied +** from a pool, a new bag is allocated instead. +** +** The pools are stored as single linked lists, for which 'PARENT_LVARS' +** is abused. +*/ +Bag NewLVarsBag(UInt slots) +{ + Bag result; + if (slots < ARRAY_SIZE(STATE(LVarsPool))) { + result = STATE(LVarsPool)[slots]; + if (result) { + STATE(LVarsPool)[slots] = PARENT_LVARS(result); + return result; + } } - } - return NewBag(T_LVARS, sizeof(Obj) * ( 3 + slots ) ); + return NewBag(T_LVARS, sizeof(LVarsHeader) + sizeof(Obj) * slots); } -void FreeLVarsBag(Bag bag) { - UInt slots = SIZE_BAG(bag) / sizeof(Obj) - 3; - if (slots < ARRAY_SIZE(STATE(LVarsPool))) { - memset(PTR_BAG(bag), 0, SIZE_BAG(bag)); - ADDR_OBJ(bag)[0] = STATE(LVarsPool)[slots]; - STATE(LVarsPool)[slots] = bag; - } + +/**************************************************************************** +** +*F FreeLVarsBag() . . . . . . . . . . . . . . . . . free an LVars bag +** +** 'FreeLVarsBag' returns an unused 'T_LVAR' bag to one of the 'LVarsPool', +** assuming its size (resp. number of local variable slots) is not too big. +*/ +void FreeLVarsBag(Bag bag) +{ + GAP_ASSERT(TNUM_OBJ(STATE(CurrLVars)) == T_LVARS); + UInt slots = (SIZE_BAG(bag) - sizeof(LVarsHeader)) / sizeof(Obj); + if (slots < ARRAY_SIZE(STATE(LVarsPool))) { + // clean the bag + memset(PTR_BAG(bag), 0, SIZE_BAG(bag)); + // put it into the linked list of available LVars bags + PARENT_LVARS(bag) = STATE(LVarsPool)[slots]; + STATE(LVarsPool)[slots] = bag; + } } @@ -2590,9 +2616,9 @@ static Int InitKernel ( /* install the marking functions for local variables bag */ InfoBags[ T_LVARS ].name = "values bag"; - InitMarkFuncBags( T_LVARS, MarkAllSubBags ); + InitMarkFuncBags( T_LVARS, MarkAllButFirstSubBags ); InfoBags[ T_HVARS ].name = "high variables bag"; - InitMarkFuncBags( T_HVARS, MarkAllSubBags ); + InitMarkFuncBags( T_HVARS, MarkAllButFirstSubBags ); #ifdef HPCGAP /* Make T_LVARS bags public */ diff --git a/src/vars.h b/src/vars.h index 5067e3eada..1d79b784e3 100644 --- a/src/vars.h +++ b/src/vars.h @@ -77,8 +77,8 @@ static inline int IS_LVARS_OR_HVARS(Obj obj) typedef struct { - Obj func; Expr stat; + Obj func; Obj parent; } LVarsHeader;