Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when ApplicableMethod is called incorrectly #3151

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/opers.c
Original file line number Diff line number Diff line change
Expand Up @@ -3160,13 +3160,13 @@ void SaveOperationExtras (
SaveSubObj(header->setter);
SaveSubObj(header->tester);
SaveSubObj(header->extra);
for (UInt i = 0; i <= 7; i++)
for (UInt i = 0; i <= MAX_OPER_ARGS; 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 (UInt i = 0; i <= 7; i++)
for (UInt i = 0; i <= MAX_OPER_ARGS; i++)
SaveSubObj(header->cache[i]);
#endif
}
Expand All @@ -3191,13 +3191,13 @@ void LoadOperationExtras (
header->setter = LoadSubObj();
header->tester = LoadSubObj();
header->extra = LoadSubObj();
for (UInt i = 0; i <= 7; i++)
for (UInt i = 0; i <= MAX_OPER_ARGS; 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 (UInt i = 0; i <= 7; i++)
for (UInt i = 0; i <= MAX_OPER_ARGS; i++)
header->cache[i] = LoadSubObj();
#endif
}
Expand Down Expand Up @@ -3446,8 +3446,10 @@ Obj FuncMETHODS_OPERATION (
Obj meth;

RequireOperation(oper);
RequireNonnegativeSmallInt("METHODS_OPERATION", narg);
n = INT_INTOBJ( narg );
n = IS_INTOBJ(narg) ? INT_INTOBJ(narg) : -1;
if (n < 0 || n > MAX_OPER_ARGS)
RequireArgument("METHODS_OPERATION", narg, "narg",
"must be an integer between 0 and 6");
meth = MethsOper( oper, (UInt)n );
#ifdef HPCGAP
MEMBAR_READ();
Expand All @@ -3471,13 +3473,15 @@ Obj FuncCHANGED_METHODS_OPERATION (
Int i;

RequireOperation(oper);
RequireNonnegativeSmallInt("CHANGED_METHODS_OPERATION", narg);
n = IS_INTOBJ(narg) ? INT_INTOBJ(narg) : -1;
if (n < 0 || n > MAX_OPER_ARGS)
RequireArgument("CHANGED_METHODS_OPERATION", narg, "narg",
"must be an integer between 0 and 6");
#ifdef HPCGAP
if (!PreThreadCreation) {
ErrorQuit("Methods may only be changed before thread creation",0L,0L);
}
#endif
n = INT_INTOBJ( narg );
cacheBag = CacheOper( oper, (UInt) n );
cache = ADDR_OBJ( cacheBag );
for ( i = 1; i < SIZE_OBJ(cacheBag) / sizeof(Obj); i++ ) {
Expand All @@ -3500,8 +3504,10 @@ Obj FuncSET_METHODS_OPERATION (
Int n;

RequireOperation(oper);
RequireNonnegativeSmallInt("SET_METHODS_OPERATION", narg);
n = INT_INTOBJ( narg );
n = IS_INTOBJ(narg) ? INT_INTOBJ(narg) : -1;
if (n < 0 || n > MAX_OPER_ARGS)
RequireArgument("SET_METHODS_OPERATION", narg, "narg",
"must be an integer between 0 and 6");
#ifdef HPCGAP
MEMBAR_WRITE();
#endif
Expand Down
17 changes: 11 additions & 6 deletions src/opers.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
#include "calls.h"
#include "system.h"


enum {
MAX_OPER_ARGS = 6
};

/****************************************************************************
**
**
Expand All @@ -42,10 +47,10 @@ typedef struct {
Obj tester;

// method list of an operation
Obj methods[8];
Obj methods[MAX_OPER_ARGS+1];

// cache of an operation
Obj cache[8];
Obj cache[MAX_OPER_ARGS+1];

// small integer encoding a set of bit flags with information about the
// operation, see OperExtras below
Expand Down Expand Up @@ -177,13 +182,13 @@ static inline void SET_TESTR_FILT(Obj oper, Obj x)
*/
static inline Obj METHS_OPER(Obj oper, Int i)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i <= MAX_OPER_ARGS);
return CONST_OPER(oper)->methods[i];
}

static inline void SET_METHS_OPER(Obj oper, Int i, Obj x)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i <= MAX_OPER_ARGS);
OPER(oper)->methods[i] = x;
}

Expand All @@ -194,13 +199,13 @@ static inline void SET_METHS_OPER(Obj oper, Int i, Obj x)
*/
static inline Obj CACHE_OPER(Obj oper, Int i)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i <= MAX_OPER_ARGS);
return CONST_OPER(oper)->cache[i];
}

static inline void SET_CACHE_OPER(Obj oper, Int i, Obj x)
{
GAP_ASSERT(0 <= i && i < 8);
GAP_ASSERT(0 <= i && i <= MAX_OPER_ARGS);
OPER(oper)->cache[i] = x;
}

Expand Down
25 changes: 19 additions & 6 deletions tst/testinstall/kernel/opers.tst
Original file line number Diff line number Diff line change
Expand Up @@ -214,28 +214,41 @@ Error, operation already installed
gap> METHODS_OPERATION(fail,1);
Error, METHODS_OPERATION: <oper> must be an operation (not the value 'fail')
gap> METHODS_OPERATION(Size,-1);
Error, METHODS_OPERATION: <narg> must be a non-negative small integer (not the\
integer -1)
Error, METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not the i\
nteger -1)
gap> METHODS_OPERATION(Size,0);
[ ]
gap> METHODS_OPERATION(Size,6);
[ ]
gap> METHODS_OPERATION(Size,7);
Error, METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not the i\
nteger 7)

# note: CHANGED_METHODS_OPERATION is not usable on HPC-GAP
gap> CHANGED_METHODS_OPERATION(fail,1);
Error, CHANGED_METHODS_OPERATION: <oper> must be an operation (not the value '\
fail')
gap> CHANGED_METHODS_OPERATION(Size,-1);
Error, CHANGED_METHODS_OPERATION: <narg> must be a non-negative small integer \
(not the integer -1)
Error, CHANGED_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (n\
ot the integer -1)
gap> if not IsHPCGAP then CHANGED_METHODS_OPERATION(Size,0); fi;
gap> if not IsHPCGAP then CHANGED_METHODS_OPERATION(Size,6); fi;
gap> CHANGED_METHODS_OPERATION(Size,7);
Error, CHANGED_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (n\
ot the integer 7)

#
gap> SET_METHODS_OPERATION (fail,1,[]);
Error, SET_METHODS_OPERATION: <oper> must be an operation (not the value 'fail\
')
gap> SET_METHODS_OPERATION (Size,-1,[]);
Error, SET_METHODS_OPERATION: <narg> must be a non-negative small integer (not\
the integer -1)
Error, SET_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not t\
he integer -1)
gap> SET_METHODS_OPERATION (Size,0,[]);
gap> SET_METHODS_OPERATION (Size,6,[]);
gap> SET_METHODS_OPERATION (Size,7,[]);
Error, SET_METHODS_OPERATION: <narg> must be an integer between 0 and 6 (not t\
he integer 7)

#
gap> f:=SETTER_FUNCTION("foobar", IsPGroup);;
Expand Down