[I7-1984] [Mantis 2020] Fake ternary relations cause Glulxe to crash

[curiousdannii-testing]

Reported by : NYKevin

Description :

When using a relation-property to "fake" a ternary relation, Glulxe crashes with this error message:

Glulxe fatal error: Memory access out of range (-7FFFFFD2)

Have not tried on other Glulx implementations. Does not appear to happen on the Z-machine (which correctly displays 5, as one would expect).

Steps to reproduce :

A skill is a kind.  Every skill has a relation of people to numbers called the skill-score-relation.

To decide what number is the (S - a skill) score of (P - a person):
    If P relates to a number by the skill-score-relation of S:
        Decide on the number to which P relates by the skill-score-relation of S;
    Decide on zero.

To change the/-- (S - a skill) score of (P - a person) to (N - a number):
    If P relates to a number by the skill-score-relation of S:
        Now the skill-score-relation of S does not relate P to the S score of P;
    Now the skill-score-relation of S relates P to N.

Fishing is a skill.

There is a room.

When play begins:
    Change the fishing score of the player to 5;
    Showme the fishing score of the player.

Additional information :

Calling this "mild" because I doubt that people are doing this sort of thing regularly. However, given the lack of obvious methods for creating ternary relations, a solution would be Nice To Have.

imported from: [Mantis 2020] Fake ternary relations cause Glulxe to crash

• status: Closed
• resolution: Won't Do
• resolved: 2022-04-10T04:46:34+10:00
• imported: 2022/01/10

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by zarf :
Confirmed.

I made the routine more verbose for debugging purposes:

To change the/-- (S - a skill) score of (P - a person) to (N - a number):

    let R be the skill-score-relation of S;

    say "Point A.";

    If P relates to a number by R:

        say "Point B.";

        Now R does not relate P to the S score of P;

    say "Point C.";

    Now R relates P to N;

    say "Point D.";

This crashes on the line "Now R relates P to N;", which compiles as

((RlnGetF(tmp_0, RR_HANDLER))(tmp_0, RELS_ASSERT_TRUE, t_1, t_2));

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by zarf :
The first bit, (RlnGetF(tmp_0, RR_HANDLER)), returns DoubleHashSetRelationHandler.

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by NYKevin :
The conditionals documented in §13.13 of WI ("If P relates to a number by R") don't look very type safe to me (you have to tell it you're looking for a number, and it doesn't parameterize P and R on the relation's strong type even though that would make logical sense). Of course, for a conditional, you don't need to be type safe, you just have to make sure you return false if the types are wrong (e.g. because R doesn't relate people to numbers). But this suggests to me that the type safety of relations is intended to be mostly static (e.g. using an assertion verb other than "relate", so that the exact identity of the mutated relation is statically known). If that is the case, then dynamic type checking of relations might be implemented weakly or not at all.

It's also unclear to me whether using "relates" in a now phrase is even legal. WI doesn't explicitly endorse it, though it obviously compiles.

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by NYKevin :
Maintainers, please disregard my previous comment entirely. dfremont is right on all counts so far as I can tell.

Next time, maybe I'll look into things a little more carefully before opening my mouth.

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by dfremont :
The problem is with the way mutable copies are made of constant relations and lists, and causes more serious non-crashing errors - see below. The crash above happens through the following sequence of events (thanks again to zarf for the very helpful glulxe debugger!):

1) The initial value of the skill-score-relation of fishing is a constant "anonymous" relation
2) So when we try to modify it, Inform creates a mutable copy with BlkMakeMutable
3) The mutable copy is created using RELATION_TY_Create, which takes as an argument the strong kind of the relation to create (here "relation of people to numbers"), but BlkMakeMutable actually passes it the weak kind ("relation")
4) Then when Inform later tries to extract the left domain of the relation ("people"), it gets back UNKNOWN_TY since the weak kind doesn't contain that information, and this causes a crash.

With lists, you don't get a crash, but the runtime type of the mutable copy is wrongly set to UNKNOWN_TY. This leads to a serious problem: lists containing block values (like texts) aren't recognized as such, so the contained values are treated like word values, and only the pointers to them end up getting copied. This means that if you modify the copy, the original list gets modified too. The following example illustrates this: the list of numbers is copied correctly (so it doesn't change when we modify the copy), but the list of texts isn't.

Foo is a room.

Wob is initially 

{ 3 }
.

Blob is initially 
{ "good" }
.

When play begins:

    let wob-copy be wob;

    now entry 1 of wob-copy is 42;

    say "[wob] (should be 3).";

    let blob-copy be blob;

    now entry 1 of blob-copy is "bad";

    say "[blob] (should be good).".

Presumably this could be fixed by having BlkMakeMutable ask the KOV support function to look up the strong type of the block value (using the KINDDATA_KOVS operation, although this isn't implemented for relations for some reason).

[curiousdannii-testing]

557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:

Comment by dfremont :
They are typesafe: look at their definitions in the Standard Rules (section SR5/2/19). If you use a wrong kind of value, e.g. "if the Atrium relates to a number by the containment relation", you will get Problem messages (if you don't, that's a bug).

Using "relates" in a now phrase is definitely legal: it's the only way to modify temporary relations. See WI 13.15.