#36 6.4.2 `delete-member$` deleted the wrong field and wrote past its own block

[Batch Job 1]

delete-member$ deleted the wrong field and wrote past its own block

What happened. Deleting a multi-field sequence from a multifield that is a slice of another one removed the wrong fields and wrote one element beyond the block it had allocated.

(delete-member$ (rest$ (create$ a b c d)) (create$ b c))

answered (c). (rest$ (create$ a b c d)) is (b c d), and removing the sequence b c from it leaves (d).

Under AddressSanitizer the same call reports a heap-buffer-overflow: an 8-byte write 0 bytes after a 32-byte region, at multifun.c:440, in a block CreateMultifield had just allocated four slots for.

A slice is not an unusual thing to have. rest$ returns one, and so does a multifield pattern variable: (data ? $?x) matching (data a b c d) binds ?x to (b c d) as a slice of the stored fact's own multifield.

Why it is a fix and not a preference. Section 12.2.14 says delete-member$ returns the first multifield with every occurrence of each later argument removed, and that a multifield argument is removed where the whole sequence appears in order. Its own example is

(delete-member$ (create$ a b c c b a) (create$ b a))  →  (a b c c)

which is the same deletion this got wrong whenever the first argument began anywhere but at index 0. Writing outside an allocation needs no manual to be wrong.

Where it came from. FindValueInMultifield in clips_core_source_642/core/multifun.c answered two different kinds of number. For a single sought value it walked begin to begin + range and returned the loop variable — an index into contents. For a multifield sought value it walked 0 to range - soughtRange and returned that loop variable — an offset from the start of the slice, with begin not counted.

DeleteMemberFunction compares the answer with j, and j starts at resultValue.begin:

      while ((rs = FindValueInMultifield(&valueSought,&resultValue)) != VALUE_NOT_FOUND)
        {
         update = CreateMultifield(theEnv,resultValue.range - valueSoughtLength);
         re = rs + valueSoughtLength - 1;

         for (j = resultValue.begin, k = 0; j < (resultValue.begin + resultValue.range); j++)
            {
             if ((j >= rs) && (j <= re)) continue;

             update->contents[k++].value = resultValue.multifieldValue->contents[j].value;
            }

With begin at 1 and the sequence found at offset 0, rs is 0 and re is 1. The skip fires for j of 1 only, so one field is skipped where two should be — and k reaches 3 in a block sized for 2.

The two answers being different kinds of number is the defect. Each caller on its own is consistent with what it believes it was given.

The change. FindValueInMultifield answers an index into contents in both branches, and says so. The multifield branch adds the searched slice's begin before returning.

MemberFunction is the other caller, and member$ reports a position within the multifield it was handed, counting from one — which its own single-value branch already computed by subtracting begin. It now subtracts begin in the multifield branch too, so both branches of both functions agree about what the number means.

DeleteMemberFunction needed no change once the answer meant one thing.

What was checked. Section 12.2.14's example, the same deletion over a slice, a single sought value over a slice, and a search with no hit. Then member$ in five forms — a single value found and not found, a sequence, and both of those over a slice — because changing what the search returns could have moved what member$ reports, and it did not. Section 12.2.15's replace-member$ example and a replace-member$ over a slice, which take a different path and were unaffected.

[Batch Job 1]

Present in 7.0? Not checked.