Skip to content

Commit

Permalink
kernel: use custom GC marking for T_FUNCTION
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
fingolfin committed Apr 28, 2018
1 parent ca11ea0 commit f2c53b5
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 37 deletions.
29 changes: 23 additions & 6 deletions src/calls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand All @@ -1769,6 +1771,21 @@ void LoadFunction ( Obj func )

#endif

/****************************************************************************
**
*F MarkFunctionSubBags( <bag> ) . . . . . . . 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);
}


/****************************************************************************
**
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions src/calls.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
49 changes: 24 additions & 25 deletions src/opers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down

0 comments on commit f2c53b5

Please sign in to comment.