Segfault after installing a silly method for a synonym attribute

[wilfwilson]

In the current master and stable-4.11 branches, and in 4.11.1 (I believe), I can cause a segfault in the following way:

gap> DeclareAttribute("Wilf", IsGroup);
gap> DeclareSynonymAttr("Wilson", Wilf);
gap> InstallMethod(Wilf, [IsGroup], x -> fail);
gap> InstallMethod(Wilson, [IsGroup], Wilf);
gap> Wilf(SymmetricGroup(3));
Segmentation fault: 11

Calling Wilson(SymmetricGroup(3)); instead on the final line also gives the a segfault.

The specific group involved, and indeed even the use of IsGroup as the filter rather than some other object, doesn't seem to be important.

Obviously I'm not using things correctly – the declaration as a synonym means that the line InstallMethod(Wilson, [IsGroup], Wilf); is completely unnecessary. But whatever the case, this shouldn't lead to a crash.

This shows that it is most likely the redundant method installation that causes the problem:

gap> DeclareAttribute("Wilf", IsGroup);
gap> InstallMethod(Wilf, [IsGroup], x -> fail);
gap> Wilf(SymmetricGroup(3)); # All fine
fail
gap> DeclareSynonymAttr("Wilson", Wilf);
gap> Wilf(SymmetricGroup(3)); # All fine
fail
gap> InstallMethod(Wilson, [IsGroup], Wilf);
gap> Wilf(SymmetricGroup(3));
Segmentation fault: 11

Note that doing the method installation the other way round (i.e. InstallMethod(Wilf, [IsGroup], Wilson);) also gives the segfault.

[fingolfin]

I suspect this has nothing to do with synonyms; just do InstallMethod(Foo, ..., Foo); and then any call to Foo triggering this "method" will result in infinite recursion and a crash . Of course you can also do with 2, 3, ... operations calling each other in a circle

[fingolfin]

As such this seems to be a duplicate of issue #1286 ? Which we closed but the fix was reverted...?

[wilfwilson]

Ah, interesting, thanks for remembering this @fingolfin.

1286 was fixed in #3221 but unfixed in #3578, because the fix was too expensive.

If that's how things are going to stay, then we should close this issue but re-open #1286, and add the 'status: wontfix' label (and perhaps give it a more descriptive title).

[fingolfin]

Well, one thing that we definitely could do is to at least catch the "obvious" cases, were someone installs an operation as a method for itself, which is essentially what's happening here, and also in #1286:

gap> DeclareAttribute("Wilf", IsGroup);
gap> DeclareSynonymAttr("Wilson", Wilf);
gap> Wilson;
<Attribute "Wilf">
gap> IsIdenticalObj(Wilson, Wilf);
true

While that may not catch all and every potential problem, it seems that at least two people ran into this independently, so even such a limited but fast patch would still be useful. I've now made PR #4349. Unfortunately it triggers for me when loading with the default set of autoloaded packages :joy:. Details in that PR.