perl11 / potion

_why the lucky stiff's little language (the official repo... until _why returns)
http://groups.google.com/group/potion-lang
Other
658 stars 90 forks source link

'eval' has problems, parse is not GC safe #9

Open ghost opened 14 years ago

ghost commented 14 years ago

In potion, eval has some problems that make it behave strangely sometimes, such as that loop: 'x = Object()' eval. segfaults while 'loop: x = Object().' eval doesn't.

I think this is due to the fact that potion_eval calls potion_parse which overwrites data in the P variable. One way to overcome this problem would be to create a new Potion object in potion_eval, but that would require using potion_create/potion_init, which overwrites global variables like PN_string that are vital to the operation of the original Potion object. The only way I can think of overcoming this is to use fork in potion_eval.

rurban commented 11 years ago

I tried to save and restore the overwritten P fields. parse should be re-entrant. source, yypos, input. skip pbuf

But it turned out the problem is that PNSource in the parser is not GC safe. all the intermediate t and lhs objects need to be moved also, and be checked for FWDs.

rurban commented 11 years ago

re-entrancy and testcase fixed with

commit d83d86947e44bff8a3eec37c203ac8f5cf471975 Author: Reini Urban rurban@cpanel.net Date: Mon Sep 16 12:32:35 2013 -0500

potion #9: fix parser is not re-entrant, but still not GC safe

All the intermediate t and lhs objects in the compiler need to
be moved also while in a eval, and be checked for FWDs.
robotii commented 5 years ago

I had some luck with trying to make the parser GC safe. I can't guarantee that this is the right solution, but it seems to work for me.

void potion_gc_minor_parser(PN parser) {
  if(parser == 0)
    return;

  struct _GREG *G = (struct _GREG *)parser;
  struct PNMemory *M = ((Potion *)G->data)->mem;
  Potion *P = G->data;

  if(PN_IS_PTR(G->ss)) {
    GC_MINOR_UPDATE(G->ss);
    potion_mark_minor(G->data, (const struct PNObject *) G->ss);
  }
  if(PN_IS_PTR(G->val[0])) {
    GC_MINOR_UPDATE(G->val[0]);
    potion_mark_minor(G->data, (const struct PNObject *) G->val[0]);
  }
  int c = G->val - G->vals;
  for(int i = 0; i < c; i++) {
    if(PN_IS_PTR(G->vals[i])) {
      GC_MINOR_UPDATE(G->vals[i]);
      potion_mark_minor(G->data, (const struct PNObject *) G->vals[i]);
    }
  }
}

void potion_gc_major_parser(PN parser) {
  if(parser == 0)
    return;

  struct _GREG *G = (struct _GREG *)parser;
  struct PNMemory *M = ((Potion *)G->data)->mem;
  Potion *P = G->data;

  if(PN_IS_PTR(G->ss)) {
    GC_MAJOR_UPDATE(G->ss);
    potion_mark_major(P, (const struct PNObject *) G->ss);
  }
  if(PN_IS_PTR(G->val[0])) {
    GC_MAJOR_UPDATE(G->val[0]);
    potion_mark_major(P, (const struct PNObject *) G->val[0]);
  }
  int c = G->val - G->vals;
  for(int i = 0; i < c; i++) {
    if(G->vals[i] != NULL && PN_IS_PTR(G->vals[i])) {
      GC_MAJOR_UPDATE(G->vals[i]);
      potion_mark_major(P, (const struct PNObject *) G->vals[i]);
    }
  }
}

I made the calls right after the GC calls to the strings (for both major and minor GC)

    GC_MAJOR_STRINGS();
    potion_gc_major_parser(P->parser); // ensure the parser is GC safe

Sorry I don't have time to make a proper pull request, as my copy of potion has diverged from this repository, as well as a lack of time. I think this should help with making the parser a bit safer though.

rurban commented 5 years ago

Great, I'll look at it