Crashes with table.sort

[sirinsidiator]

I always thought it was normal that eso crashes if I mess up my sort function in some bad way, but now that haggen had a similar problem I went to the trouble of actually reading the Lua source code and saw that it should not happen that way.

local myTable = {1,2,3,4}
table.sort(myTable, function(a, b)
   return true
end)

This should throw a Lua error according to the source code of auxsort, but instead it becomes an infinite loop and hangs the process until it is killed.

I suspect it has to do with this piece here:

   /* repeat ++i until a[i] >= P */
      while (lua_rawgeti(L, 1, ++i), sort_comp(L, -1, -2)) {
        if (i>u) luaL_error(L, "invalid order function for sorting");
        lua_pop(L, 1);  /* remove a[i] */
      }

For some reason luaL_error does not get fired or simply does not stop code execution.

[merlight]

To me it looks quite suspicious that they check indices one-behind:

// auxsort 5.1
 
      while (lua_rawgeti(L, 1, ++i), sort_comp(L, -1, -2)) {
        if (i>u) luaL_error(L, "invalid order function for sorting");
        lua_pop(L, 1);  /* remove a[i] */
      }
      /* repeat --j until a[j] <= P */
      while (lua_rawgeti(L, 1, --j), sort_comp(L, -3, -1)) {
        if (j<l) luaL_error(L, "invalid order function for sorting");
        lua_pop(L, 1);  /* remove a[j] */
      }

(i>u) or (j<l) is checked after t[i] or t[j] has been accessed and compared, and it may happen that (i>#t) or (j<1). Although luaH_getnum does check the index, and goes into the hashtable if it's out of range. Perhaps ESO Lua removed these checks in auxsort completely.

You can see in Lua 5.2 they actually fixed these checks auxsort 5.2

[sirinsidiator]

Haggen did mention that sometimes the sort function got passed a nil even when the table only contained numbers, so that might be the case.

[Solinur]

It looks like this is behind the recent crashes caused by Combat Metrics. It's really hard to find a bug like this

[sirinsidiator]

Yes. And it would be literally solved by adding 2 characters.

[ZOS_ChipHilseberg]

I have a fix in for this.

[sirinsidiator]