HPC-GAP: missing guards

[fingolfin]

Right now, HPC-GAP is missing almost all read and write guards, due to us switching lots of macros to static inline functions, see gap-system/ward#46

This is what should happen (recorded on the hpcgap-default branch):

gap> WRAPPER_OPERATIONS;
<protected object in shared region 0x1031ea900 (id: 4348491888)>
gap> WRAPPER_OPERATIONS[1];
Error, Attempt to read object 4348491888 of type list (plain,hom) without having read access
not in any function at line 3 of stream
brk>
gap> IsPNilpotent in WRAPPER_OPERATIONS;
Error, Attempt to read object 4348491888 of type list (plain,hom) without having read access
not in any function at line 3 of stream

But this is what currently does happen:

gap> WRAPPER_OPERATIONS;
<protected object in shared region 0x1025ac900 (id: 4335663856)>
gap> WRAPPER_OPERATIONS[1];
<Operation "IsPNilpotent">
gap> IsPNilpotent in WRAPPER_OPERATIONS;
true

This is a quite serious problem, and we must address it in some way.

[fingolfin]

One way to fix this is to fix ward, as described in gap-system/ward#46. As an alternative, we could also try to get rid of ward, by inserting guards into all relevant accessor functions, and hoping for the compiler to get rid of redundant ones -- and @rbehrends had some ideas on how to (ab)use __attribute__((pure)) to help the compiler do that. Also, in some experiments he made, using -O3 instead of -O2 in `CFLAGS helped with this considerably.

However, there are some obstacles to this. One is that we can't simply #include <src/hpc/guards.h> in gasman.h (where we'd put guards into TNUM_BAG, SIZE_BAG, etc.). The problem is that guards.h needs tls.h, and that includes gapstate.h and ultimates objects.h, which uses TNUM_BAG... so we have a circular dependency.

[rbehrends]

Well, TNUM_BAG() and SIZE_BAG() should not have guards, anyway. Guards were only ever inserted for operations that read or modified the contents of a bag; the header could be read and modified concurrently (accordingly, special tests are inserted where that happens in an unsafe fashion, such as in KTNumPlist().

This does not necessarily fix the issue for other inline functions, but in principle the only thing we would need is a write guard for ADDR_OBJ() and PTR_BAG() as well as a read guard for CONST_ADDR_OBJ() and CONST_PTR_BAG() [1]. We'd also need something like UNGUARDED_ADDR_OBJ() or UNGUARDED_PTR_BAG() for when we want to avoid guards for low-level code, such as gasman.c or aobjects.c (ideally, we may even want a #define for headers to remove guards entirely for a file).

[1] In practice, there are a few other functions, such as ELM_BAG() that calculate PTR_BAG() directly. These can generally be found by searching for (Bag **) (alternatively, without a space and/or with Obj instead of Bag) in the source code.

[fingolfin]

Well, TNUM_BAG() and SIZE_BAG() should not have guards, anyway. Guards were only ever inserted for operations that read or modified the contents of a bag

Are you sure about that? I am seeing e.g. this in hpcgap-default:

    if (!CheckWriteAccess(list)) {
      ReadGuard(list);
#line 431 "src/plist.c"
      return TNUM_OBJ(list);
    }

I'm also not concerned about ELM_BAG, as that is a gasman-only function which usually does not even get compiled in. Indeed, unless I am doing something wrong, I think all occurrences of (Bag **), with or without space, are in gasman.c, with PTR_BAG being the only exception. Or perhaps you meant something else?

[fingolfin]

I am not sure if it is realistic, but I really would love if we could get an alpha version of this ready for GAP 4.9. The alternative is to ship a severely broken HPC-GAP to people :-/.
If we could implement this, even in a sub-performant version, we could also get rid of ward, making it that much easier to build HPC-GAP...

[olexandr-konovalov]

I've reassigned this to 4.9.1 milestone.

[fingolfin]

Ward is now completely useless in master, I therefore posted PR #2870. See PR #2845 for work on adding back guards.