#25 6.4.2 a pattern's own fact address read as a slot number

[Batch Job 1]

What happened. A rule that names a pattern's fact address inside a constraint on that same pattern crashed CLIPS.

(deftemplate av (slot attribute) (slot value))
(defrule r1
   ?f <- (av (attribute ?a&:(> (fact-slot-value ?f value) ?a)))
   =>)
(deffacts initial
   (av (attribute 1) (value 2))
   (av (attribute 2) (value 1)))
(reset)

Seven lines, and the process dies on the (reset) with a signal. Under AddressSanitizer it is a bus error from dereferencing a high address, at factrete.c in FactPNGetVar2.

Why it is a fix and not a preference. Section 5.4.1 documents ?variable <- as binding the address of the fact matching the pattern, and section 5.4.6.5 documents : as taking a function call that may use variables bound earlier in the pattern. Nothing says the address may not be one of them, and CLIPS 7.0.0 accepts the rule and activates it. A segmentation fault is not a reading of any of that.

The rule is the CLIPS 7.0.0 test suite's own, in tmplv700.bat, under a comment that reads ; Crash.

Where it came from. FactReplaceGetfield in clips_core_source_642/core/factgen.c chooses which of three routines will fetch a variable during pattern matching:

   if (theNode->withinMultifieldSlot == false)
     {
      theItem->type = FACT_PN_VAR2;
      theItem->value = FactGetVarPN2(theEnv,theNode);
      return;
     }

FactGetVarPN2 stores one number and nothing else:

   hack.whichSlot = theNode->slotNumber - 1;

and FactPNGetVar2 indexes the fact's slots with it:

   fieldPtr = &factPtr->theProposition.contents[hack->whichSlot];

A fact address variable names no slot, so its slotNumber is zero. whichSlot is an unsigned short, so 0 - 1 is 65535, and the read is of slot 65535 of a two-slot fact.

The test that was missing is twenty lines earlier in the same file. FactGenGetfield chooses between the same routines and asks for a slot first:

   if ((theNode->slotNumber > 0) && (theNode->slotNumber != UNSPECIFIED_SLOT) && (theNode->withinMultifieldSlot == false))
     { return(GenConstant(theEnv,FACT_PN_VAR2,FactGetVarPN2(theEnv,theNode))); }

The change. FactReplaceGetfield asks the same question, and a node that names no slot goes to FactGetVarPN1 instead — which already handles exactly that case:

   if ((theNode->slotNumber == 0) ||
       (theNode->slotNumber == UNSPECIFIED_SLOT))
     {
      hack.factAddress = 1;
      …
     }

FactPNGetVar1 then returns FactData(theEnv)->CurrentPatternFact, which is the fact being matched — the thing ?f stands for. The fix is to route the node to the routine that was already written for it.

What was checked. tests/self-reference.clp gives output identical to CLIPS 7.0.0, line for line, across seven shapes: the suite's own rule, which activates on the fact whose attribute is below its value and not the other; the comparison reversed, which activates on the other fact; the address used in a predicate without reading a slot through it; the address passed to fact-relation; a multislot read through the address with member$, matching one fact and not the other; two patterns where the second one's address is used in its own test, so the join network is involved as well; and the address used on the right-hand side, which goes through the join network and was never affected.

Unmodified 6.4.2 produces no output at all against that script: it dies on the first (reset).

What it changed in the 7.0.0 suite. tmplv700.tst ran to completion rather than dying part way: its dribble output had been cut off at exactly 8192 bytes, one buffer, and is now 9474. drtest11.tst, which crashes for the other reason recorded in ../\FactPNGetVar1` read past the end of a multifield slot`, also completes once both are fixed, and moves from 76 differing lines against the 7.0.0 expectation to 16 — the remainder being 7.0.0 features 6.4.2 does not have.

Fixed upstream. CLIPS 7.0.0 accepts the rule and gives the answers this now gives. The suite ships the rule as a regression test, so it was a known crash there.