Skip to content

Commit

Permalink
Add memory debugging
Browse files Browse the repository at this point in the history
This adds a new configure option to GAP which enables an
extremely slow debug mode, which can find problems where
references to memory are incorrectly kept across
garbage collections.
  • Loading branch information
ChrisJefferson committed Mar 24, 2018
1 parent 4722aa5 commit 9bd83e8
Show file tree
Hide file tree
Showing 7 changed files with 306 additions and 43 deletions.
8 changes: 8 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,14 @@ AC_ARG_ENABLE([debug],
AC_MSG_CHECKING([whether to enable debug mode])
AC_MSG_RESULT([$enable_debug])

AC_ARG_ENABLE([memory-checking],
[AS_HELP_STRING([--enable-memory-checking], [enable memory checking])],
[AC_DEFINE([GAP_MEM_CHECK], [1], [define if building with memory checking])],
[enable_memory_checking=no]
)
AC_MSG_CHECKING([whether to enable memory checking])
AC_MSG_RESULT([$enable_memory_checking])

dnl
dnl User setting: Enable -Werror (off by default)
dnl
Expand Down
3 changes: 3 additions & 0 deletions lib/init.g
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,9 @@ BindGlobal( "ShowKernelInformation", function()
if GAPInfo.KernelInfo.KernelDebug then
Add( config, "KernelDebug" );
fi;
if GAPInfo.KernelInfo.MemCheck then
Add(config, "MemCheck (EXPERIMENTAL)");
fi;
if config <> [] then
print_info( " Configuration: ", config, "\n" );
fi;
Expand Down
3 changes: 2 additions & 1 deletion lib/system.g
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ BIND_GLOBAL( "GAPInfo", rec(
help := [ "Run ProfileLineByLine(<filename>) with recordMem := true on GAP start"] ),
rec( long := "cover", default := "", arg := "<file>",
help := [ "Run CoverageLineByLine(<filename>) on GAP start"] ),
],
rec( long := "enableMemCheck", default := false)
],
) );


Expand Down
27 changes: 25 additions & 2 deletions src/gap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1880,6 +1880,18 @@ Obj FuncGASMAN_LIMITS( Obj self )
return list;
}

#ifdef GAP_MEM_CHECK

extern Int EnableMemCheck;

Obj FuncGASMAN_MEM_CHECK(Obj self, Obj newval)
{
EnableMemCheck = INT_INTOBJ(newval);
return True;
}

#endif

/****************************************************************************
**
*F FuncTotalMemoryAllocated( <self> ) .expert function 'TotalMemoryAllocated'
Expand Down Expand Up @@ -2672,6 +2684,13 @@ Obj FuncKERNEL_INFO(Obj self) {
AssPRec(res, r, False);
#endif

r = RNamName("MemCheck");
#ifdef GAP_MEM_CHECK
AssPRec(res, r, True);
#else
AssPRec(res, r, False);
#endif

MakeImmutable(res);

return res;
Expand Down Expand Up @@ -2767,7 +2786,7 @@ void ThreadedInterpreter(void *funcargs) {
**
*V GVarFuncs . . . . . . . . . . . . . . . . . . list of functions to export
*/
static StructGVarFunc GVarFuncs [] = {
static StructGVarFunc GVarFuncs[] = {

GVAR_FUNC(Runtime, 0, ""),
GVAR_FUNC(RUNTIMES, 0, ""),
Expand All @@ -2788,6 +2807,9 @@ static StructGVarFunc GVarFuncs [] = {
GVAR_FUNC(GASMAN_STATS, 0, ""),
GVAR_FUNC(GASMAN_MESSAGE_STATUS, 0, ""),
GVAR_FUNC(GASMAN_LIMITS, 0, ""),
#ifdef GAP_MEM_CHECK
GVAR_FUNC(GASMAN_MEM_CHECK, 1, "int"),
#endif
GVAR_FUNC(TotalMemoryAllocated, 0, ""),
GVAR_FUNC(SIZE_OBJ, 1, "object"),
GVAR_FUNC(TNUM_OBJ, 1, "object"),
Expand All @@ -2802,7 +2824,8 @@ static StructGVarFunc GVarFuncs [] = {
GVAR_FUNC(QUIT_GAP, -1, "args"),
GVAR_FUNC(FORCE_QUIT_GAP, -1, "args"),
GVAR_FUNC(SHOULD_QUIT_ON_BREAK, 0, ""),
GVAR_FUNC(SHELL, -1, "context, canReturnVoid, canReturnObj, lastDepth, setTime, prompt, promptHook, infile, outfile"),
GVAR_FUNC(SHELL, -1, "context, canReturnVoid, canReturnObj, lastDepth, "
"setTime, prompt, promptHook, infile, outfile"),
GVAR_FUNC(CALL_WITH_CATCH, 2, "func, args"),
GVAR_FUNC(JUMP_TO_CATCH, 1, "payload"),
GVAR_FUNC(KERNEL_INFO, 0, ""),
Expand Down
122 changes: 120 additions & 2 deletions src/gasman.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@

#include <stdio.h>

#ifdef GAP_MEM_CHECK
#include <sys/mman.h>
#endif

/****************************************************************************
**
*F WORDS_BAG( <size> ) . . . . . . . . . . words used by a bag of given size
Expand Down Expand Up @@ -973,6 +977,113 @@ void InitCollectFuncBags (
AfterCollectFuncBags = after_func;
}

/***************************************************************
* GAP_MEM_CHECK
*
* One of the hardest categories of bugs to fix in GAP are where
* a reference to the internals of a GAP object are kept across
* a garbage collection (which moves GAP objects around).
*
* MultiHeap provides a method of detecting such problems, at
* the cost of GREATLY decreased performance (Starting GAP in
* MultiHeap mode takes hours, rather than seconds).
*
* The fundamental idea behind MultiHeap is, whenever NewBag
* or ResizeBag is called, then the contents of every Bag in
* GAP is moved, and the memory previously being used is marked
* as not readable or writable using 'mprotect'.
*
* Actually copying all GAP's memory space would be extremely
* expensive, so instead we use 'mmap' to set up a set of copies
* of the GAP memory space, which are represented by the same
* underlying physical memory.
*
* The 0th such copy (which we also ensure is the one at the
* lowest memory address) is special -- this is where we
* reference the Master Pointers (which can't move). We do not
* 'mprotect' any of this memory.
*
* Every time we call 'NewBag' or 'ResizeBag', we change which
* copy of the GASMAN memory space the master pointers point
* to, disabling access to the previous copy, and enabling access
* to the new one.
*
* We never use the master pointers in any copy other than the
* 0th, and we never refer to the Bag Area in the 0th copy. However,
* it simplifies things to not try to seperate the master pointer
* and Bag areas, because the master pointer area can grow as GAP
* runs.
*
* Because this code is VERY slow, it can be turned on and off.
* At run time, call GASMAN_MEM_CHECK(1) to enable, and
* GASMAN_MEM_CHECK(0) to disable. Start GAP with --enableMemCheck
* to enable from GAP's start
*/

#ifdef GAP_MEM_CHECK

Int EnableMemCheck = 0;

Int enableMemCheck(Char ** argv, void * dummy)
{
EnableMemCheck = 1;
return 1;
}

void MoveBagMemory(char * oldbase, char * newbase)
{
Int moveSize = (newbase - oldbase) / sizeof(Bag);
/* update the masterpointers */
for (Bag * p = MptrBags; p < MptrEndBags; p++) {
if ((Bag)MptrEndBags <= *p)
*p += moveSize;
}

/* update 'OldBags', 'YoungBags', 'AllocBags', and 'EndBags' */
OldBags += moveSize;
YoungBags += moveSize;
AllocBags += moveSize;
EndBags += moveSize;
}


// From system.c

UInt GetMembufCount(void);
void * GetMembuf(UInt i);
UInt GetMembufSize(void);

void MaybeMoveBags(void)
{
static Int oldBase = 0;

if (!EnableMemCheck)
return;

Int newBase = oldBase + 1;
// 0 is special, skip it when cycling
if (newBase == GetMembufCount())
newBase = 1;

mprotect(GetMembuf(newBase), GetMembufSize(), PROT_READ | PROT_WRITE);

/* call the before function (if any) */
if (BeforeCollectFuncBags != 0)
(*BeforeCollectFuncBags)();

MoveBagMemory(GetMembuf(oldBase), GetMembuf(newBase));

/* call the after function (if any) */
if (AfterCollectFuncBags != 0)
(*AfterCollectFuncBags)();

if (oldBase != 0) {
mprotect(GetMembuf(oldBase), GetMembufSize(), PROT_NONE);
}

oldBase = newBase;
}
#endif

/****************************************************************************
**
Expand Down Expand Up @@ -1104,6 +1215,10 @@ Bag NewBag (
{
Bag bag; /* identifier of the new bag */

#ifdef GAP_MEM_CHECK
MaybeMoveBags();
#endif

#ifdef TREMBLE_HEAP
CollectBags(0,0);
#endif
Expand Down Expand Up @@ -1269,6 +1384,11 @@ UInt ResizeBag (
Bag bag,
UInt new_size )
{

#ifdef GAP_MEM_CHECK
MaybeMoveBags();
#endif

#ifdef TREMBLE_HEAP
CollectBags(0,0);
#endif
Expand Down Expand Up @@ -2011,7 +2131,6 @@ UInt CollectBags (
else {
done = 1;
}

}

/* if we already performed a full garbage collection */
Expand Down Expand Up @@ -2135,7 +2254,6 @@ UInt CollectBags (
return 1;
}


/****************************************************************************
**
*F CheckMasterPointers() . . . . do consistency checks on the masterpointers
Expand Down
20 changes: 12 additions & 8 deletions src/gasman.h
Original file line number Diff line number Diff line change
Expand Up @@ -594,11 +594,11 @@ extern void SwapMasterPoint (
** message.
**
** 'NrHalfDeadBags'
**
**
** 'NrHalfDeadBags' is the number of bags that have been found to be
** reachable only by way of weak pointers since the last garbage collection.
** The bodies of these bags are deleted, but their identifiers are marked so
** that weak pointer objects can recognize this situation.
** that weak pointer objects can recognize this situation.
*/

extern UInt NrAllBags;
Expand Down Expand Up @@ -894,15 +894,15 @@ extern Bag * AllocBags;
GC_unregister_disappearing_link((void **)(loc))

#endif

/****************************************************************************
**
*F InitSweepFuncBags(<type>,<sweep-func>) . . . . install sweeping function
**
** 'InitSweepFuncBags( <type>, <sweep-func> )'
**
** 'InitSweepFuncBags' installs the function <sweep-func> as sweeping
** function for bags of type <type>.
** function for bags of type <type>.
**
** A sweeping function is a function that takes two arguments src and dst of
** type Bag *, and a third, length of type UInt, and returns nothing. When
Expand All @@ -913,10 +913,10 @@ extern Bag * AllocBags;
** Those functions are applied during the garbage collection to each marked
** bag, i.e., bags that are assumed to be still live to move them to their
** new position. The intended use is for weak pointer bags, which must
** remove references to identifiers of any half-dead objects.
** remove references to identifiers of any half-dead objects.
**
** If no function is installed for a TNum, then the data is simply copied
** unchanged and this is done particularly quickly
** unchanged and this is done particularly quickly
*/

typedef void (* TNumSweepFuncBags ) (
Expand All @@ -927,7 +927,7 @@ typedef void (* TNumSweepFuncBags ) (
extern void InitSweepFuncBags (
UInt tnum,
TNumSweepFuncBags sweep_func );


/****************************************************************************
**
Expand Down Expand Up @@ -1133,7 +1133,7 @@ extern void FinishBags( void );
** This calls a C function on every bag, including ones that are not
** reachable from the root, and will be deleted at the next garbage
** collection, by simply walking the masterpointer area. Not terribly safe
**
**
*/

extern void CallbackForAllBags(
Expand All @@ -1144,4 +1144,8 @@ extern void CallbackForAllBags(
void *AllocateMemoryBlock(UInt size);
#endif

#ifdef GAP_MEM_CHECK
Int enableMemCheck(Char ** argv, void * dummy);
#endif

#endif // GAP_GASMAN_H
Loading

0 comments on commit 9bd83e8

Please sign in to comment.