bug: __newindex metamethod may not work if metatable is its own

metatable + luaV_settable does not create entry when there is a metamethod (and therefore entry is useless)
author: Roberto Ierusalimschy <roberto@inf.puc-rio.br> 2011-08-17 17:26:47 -0300
committer: Roberto Ierusalimschy <roberto@inf.puc-rio.br> 2011-08-17 17:26:47 -0300
commit: 89b59eee7386379e87200a86215fc6de91e49036 (patch)
tree: 7c82d2f7f98680787e3b235d94defef00e87133a
parent: 166dd0261a0fc012808a6448a1c46030c8db4806 (diff)
download: lua-89b59eee7386379e87200a86215fc6de91e49036.tar.gz
lua-89b59eee7386379e87200a86215fc6de91e49036.tar.bz2
lua-89b59eee7386379e87200a86215fc6de91e49036.zip
4 files changed, 44 insertions, 26 deletions
diff --git a/lapi.c b/lapi.c
index 29f639a6..b1da9951 100644
--- a/lapi.c
+++ b/lapi.c
@@ -1,5 +1,5 @@
 /*
-** $Id: lapi.c,v 2.149 2011/06/13 14:13:06 roberto Exp roberto $
+** $Id: lapi.c,v 2.150 2011/08/09 20:58:29 roberto Exp roberto $
 ** Lua API
 ** See Copyright Notice in lua.h
 */
@@ -732,6 +732,7 @@ LUA_API void lua_rawset (lua_State *L, int idx) {
  t = index2addr(L, idx);
  api_check(L, ttistable(t), "table expected");
  setobj2t(L, luaH_set(L, hvalue(t), L->top-2), L->top-1);
+  invalidateTMcache(hvalue(t));
  luaC_barrierback(L, gcvalue(t), L->top-1);
  L->top -= 2;
  lua_unlock(L);
diff --git a/ltable.c b/ltable.c
index b2f40083..1bfe616c 100644
--- a/ltable.c
+++ b/ltable.c
@@ -1,5 +1,5 @@
 /*
-** $Id: ltable.c,v 2.60 2011/06/16 14:14:31 roberto Exp roberto $
+** $Id: ltable.c,v 2.61 2011/08/09 20:58:29 roberto Exp roberto $
 ** Lua tables (hash)
 ** See Copyright Notice in lua.h
 */
@@ -322,8 +322,11 @@ void luaH_resize (lua_State *L, Table *t, int nasize, int nhsize) {
  /* re-insert elements from hash part */
  for (i = twoto(oldhsize) - 1; i >= 0; i--) {
    Node *old = nold+i;
-    if (!ttisnil(gval(old)))
+    if (!ttisnil(gval(old))) {
+      /* doesn't need barrier/invalidate cache, as entry was
+         already present in the table */
      setobjt2t(L, luaH_set(L, t, gkey(old)), gval(old));
+    }
  }
  if (!isdummy(nold))
    luaM_freearray(L, nold, twoto(oldhsize));  /* free old array */
@@ -398,14 +401,18 @@ static Node *getfreepos (Table *t) {
 ** put new key in its main position; otherwise (colliding node is in its main
 ** position), new key goes to an empty position.
 */
-static TValue *newkey (lua_State *L, Table *t, const TValue *key) {
+TValue *luaH_newkey (lua_State *L, Table *t, const TValue *key) {
  Node *mp = mainposition(t, key);
+  if (ttisnil(key)) luaG_runerror(L, "table index is nil");
+  else if (ttisnumber(key) && luai_numisnan(L, nvalue(key)))
+    luaG_runerror(L, "table index is NaN");
  if (!ttisnil(gval(mp)) || isdummy(mp)) {  /* main position is taken? */
    Node *othern;
    Node *n = getfreepos(t);  /* get a free place */
    if (n == NULL) {  /* cannot find a free place? */
      rehash(L, t, key);  /* grow table */
-      return luaH_set(L, t, key);  /* re-insert key into grown table */
+      /* whatever called 'newkey' take care of TM cache and GC barrier */
+      return luaH_set(L, t, key);  /* insert key into grown table */
    }
    lua_assert(!isdummy(n));
    othern = mainposition(t, gkey(mp));
@@ -493,17 +500,15 @@ const TValue *luaH_get (Table *t, const TValue *key) {
 }
+/*
+** beware: when using this function you probably need to check a GC
+** barrier and invalidate the TM cache.
+*/
 TValue *luaH_set (lua_State *L, Table *t, const TValue *key) {
  const TValue *p = luaH_get(t, key);
-  t->flags = 0;
  if (p != luaO_nilobject)
    return cast(TValue *, p);
-  else {
+  else return luaH_newkey(L, t, key);
-    if (ttisnil(key)) luaG_runerror(L, "table index is nil");
-    else if (ttisnumber(key) && luai_numisnan(L, nvalue(key)))
-      luaG_runerror(L, "table index is NaN");
-    return newkey(L, t, key);
-  }
 }
@@ -515,7 +520,7 @@ void luaH_setint (lua_State *L, Table *t, int key, TValue *value) {
  else {
    TValue k;
    setnvalue(&k, cast_num(key));
-    cell = newkey(L, t, &k);
+    cell = luaH_newkey(L, t, &k);
  }
  setobj2t(L, cell, value);
 }
diff --git a/ltable.h b/ltable.h
index ad13c969..9088b9e8 100644
--- a/ltable.h
+++ b/ltable.h
@@ -1,5 +1,5 @@
 /*
-** $Id: ltable.h,v 2.14 2010/06/25 12:18:10 roberto Exp roberto $
+** $Id: ltable.h,v 2.15 2011/08/09 20:58:29 roberto Exp roberto $
 ** Lua tables (hash)
 ** See Copyright Notice in lua.h
 */
@@ -15,11 +15,14 @@
 #define gval(n)         (&(n)->i_val)
 #define gnext(n)        ((n)->i_key.nk.next)
+#define invalidateTMcache(t)    ((t)->flags = 0)
 LUAI_FUNC const TValue *luaH_getint (Table *t, int key);
 LUAI_FUNC void luaH_setint (lua_State *L, Table *t, int key, TValue *value);
 LUAI_FUNC const TValue *luaH_getstr (Table *t, TString *key);
 LUAI_FUNC const TValue *luaH_get (Table *t, const TValue *key);
+LUAI_FUNC TValue *luaH_newkey (lua_State *L, Table *t, const TValue *key);
 LUAI_FUNC TValue *luaH_set (lua_State *L, Table *t, const TValue *key);
 LUAI_FUNC Table *luaH_new (lua_State *L);
 LUAI_FUNC void luaH_resize (lua_State *L, Table *t, int nasize, int nhsize);
diff --git a/lvm.c b/lvm.c
index 680b1055..5b816239 100644
--- a/lvm.c
+++ b/lvm.c
@@ -1,5 +1,5 @@
 /*
-** $Id: lvm.c,v 2.141 2011/06/09 18:24:22 roberto Exp roberto $
+** $Id: lvm.c,v 2.142 2011/08/09 20:58:29 roberto Exp roberto $
 ** Lua virtual machine
 ** See Copyright Notice in lua.h
 */
@@ -127,29 +127,38 @@ void luaV_gettable (lua_State *L, const TValue *t, TValue *key, StkId val) {
 void luaV_settable (lua_State *L, const TValue *t, TValue *key, StkId val) {
  int loop;
-  TValue temp;
  for (loop = 0; loop < MAXTAGLOOP; loop++) {
    const TValue *tm;
    if (ttistable(t)) {  /* `t' is a table? */
      Table *h = hvalue(t);
-      TValue *oldval = luaH_set(L, h, key); /* do a primitive set */
+      TValue *oldval = cast(TValue *, luaH_get(h, key));
-      if (!ttisnil(oldval) ||  /* result is not nil? */
+      /* if previous value is not nil, there must be a previous entry
-          (tm = fasttm(L, h->metatable, TM_NEWINDEX)) == NULL) { /* or no TM? */
+         in the table; moreover, a metamethod has no relevance */
-        setobj2t(L, oldval, val);
+      if (!ttisnil(oldval) ||
+         /* previous value is nil; must check the metamethod */
+         ((tm = fasttm(L, h->metatable, TM_NEWINDEX)) == NULL &&
+         /* no metamethod; is there a previous entry in the table? */
+         (oldval != luaO_nilobject ||
+         /* no previous entry; must create one. (The next test is
+            always true; we only need the assignment.) */
+         (oldval = luaH_newkey(L, h, key), 1)))) {
+        /* no metamethod and (now) there is an entry with given key */
+        setobj2t(L, oldval, val);  /* assign new value to that entry */
+        invalidateTMcache(h);
        luaC_barrierback(L, obj2gco(h), val);
        return;
      }
-      /* else will try the tag method */
+      /* else will try the metamethod */
    }
-    else if (ttisnil(tm = luaT_gettmbyobj(L, t, TM_NEWINDEX)))
+    else  /* not a table; check metamethod */
-      luaG_typeerror(L, t, "index");
+      if (ttisnil(tm = luaT_gettmbyobj(L, t, TM_NEWINDEX)))
+        luaG_typeerror(L, t, "index");
+    /* there is a metamethod */
    if (ttisfunction(tm)) {
      callTM(L, tm, t, key, val, 0);
      return;
    }
-    /* else repeat with 'tm' */
+    t = tm;  /* else repeat with 'tm' */
-    setobj(L, &temp, tm);  /* avoid pointing inside table (may rehash) */
-    t = &temp;
  }
  luaG_runerror(L, "loop in settable");
 }
author	Roberto Ierusalimschy <roberto@inf.puc-rio.br>	2011-08-17 17:26:47 -0300
committer	Roberto Ierusalimschy <roberto@inf.puc-rio.br>	2011-08-17 17:26:47 -0300
commit	89b59eee7386379e87200a86215fc6de91e49036 (patch)
tree	7c82d2f7f98680787e3b235d94defef00e87133a
parent	166dd0261a0fc012808a6448a1c46030c8db4806 (diff)
download	lua-89b59eee7386379e87200a86215fc6de91e49036.tar.gz lua-89b59eee7386379e87200a86215fc6de91e49036.tar.bz2 lua-89b59eee7386379e87200a86215fc6de91e49036.zip