Skip to content

Commit

Permalink
Forbid installing an operation as method for itself
Browse files Browse the repository at this point in the history
Triggering such a "method" would lead to a segfault due to a stack overflow.

Of course one can still trigger this by e.g. having two operations `foo` and
`bar, and installing `foo` as method for `bar` and vice-versa. But that is far
less likely to happen by accident. Also, it seems better to fix at least some
instances of this issue than to give up because we can't fix it completely.
  • Loading branch information
fingolfin committed Jun 7, 2021
1 parent 6032ea6 commit 4ba795c
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/oper1.g
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ BIND_GLOBAL( "INSTALL_METHOD_FLAGS",
function( opr, info, rel, flags, baserank, method )
local methods, narg, i, k, tmp, replace, match, j, lk, rank;

if IS_IDENTICAL_OBJ(opr, method) then
Error("Cannot install an operation as a method for itself");
fi;

if IsHPCGAP then
# TODO: once the GAP compiler supports 'atomic', use that
# to replace the explicit locking and unlocking here.
Expand Down
6 changes: 6 additions & 0 deletions tst/testbugfix/2021-03-25-InstallMethod.tst
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Installing an operation as a method for itself is forbidden (now, at least;
# it used to lead to a segfault if you ever managed to trigger that "method")
# Fixes GitHub issues #1286 and #4340.
gap> foo := NewOperation("foo", [IsObject]);;
gap> InstallMethod(foo, [IsInt], foo);
Error, Cannot install an operation as a method for itself

0 comments on commit 4ba795c

Please sign in to comment.