Page MenuHomeFreeBSD

Fix s" word in Ficl to account for memory used by the string literal
AcceptedPublic

Authored by stevek on Aug 20 2019, 2:08 AM.

Details

Reviewers
imp
Summary

Prior to this fix, using successive s" words in interpret mode would
cause them to use the same address. This was due to the fact that s" was
not moving "here" to account for the space used by the string literal.

Test Plan

Tested interactively with userboot.

Before changes...
OK s" first" s" second" .s cr
34378971689 5 34378971689 6

After changes...
OK s" first" s" second" .s cr
34378971758 5 34378971764 6

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 25971
Build 24527: arc lint + arc unit

Event Timeline

stevek created this revision.Aug 20 2019, 2:08 AM
imp added a comment.Aug 20 2019, 2:53 AM

Won't this slowly eat all memory as we run through loops with s" in them?

The same thing happens currently for cstringQuoteIm(), which is where this
change comes from:

static void cstringQuoteIm(FICL_VM *pVM)
{

FICL_DICT *dp = vmGetDict(pVM);

if (pVM->state == INTERPRET)
{
    FICL_STRING *sp = (FICL_STRING *) dp->here;
    vmGetString(pVM, sp, '\"');
    stackPushPtr(pVM->pStack, sp);
            /* move HERE past string so it doesn't get overwritten.

--lch *
/

dictAllot(dp, sp->count + sizeof(FICL_COUNT));

Note the last couple of lines above.

Yes, we eat space fairly quickly if done in a loop. But the same is true
for number of things that allot.

Most other Forth implementations only allow s" for compile mode. The
inclusion of it in interpret mode adds some complications.

I wonder even in the cstringQuoteIm() case if the dictAllot() alone is
correct. Note that compilation mode would align the dict by pointer size.

I suppose the question is what should really happen in the case of
interpret mode? Should one be able to do s" back-to-back and have it work
the same as compile mode does? Or should one know that space is not alloted
for it and one would need to do so if that were desired?

It looks like GNU Forth has a s( word for interpret mode and it acts the
same way that s" does currently in Ficl for interpret mode (does not allot
space for the string.)

I'm okay with abandoning the change in this case. I wonder if Ficl should
have just had s( word and not allowed s" in interpret mode. This would
match the .( vs. ." words and what GNU Forth does.

imp accepted this revision.Aug 20 2019, 10:40 PM

If we do it for other things, I'm cool with this going in...

This revision is now accepted and ready to land.Aug 20 2019, 10:40 PM