DRTVWR-589: Add tests for LLSD-to-Lua round-trip conversions.

Add from_lua() function to run a small Lua script that constructs a specified
Lua object and posts it back to the test program via a temporary LLEventPump.
Call this with a variety of Lua objects, comparing to the expected LLSD.

Add round_trip() function to run another small Lua script that listens for
incoming LLEventPump events and, for each, posts the received Lua data back to
the test program as LLSD. Call this with a variety of LLSD objects, comparing
to the expected LLSD. Also collect these objects into an LLSD array and send
that for a round trip; also collect into an LLSD map and send that.

Sadly, tests currently drive an access violation when trying to convert a
nested Lua table to LLSD.

Add verbose debug logging to lua_tollsd() to identify the context at which we
hit the access violation.

Add comments describing further exceptions to LLSD-to-Lua round trip identity.

Add lua_what() iostream manipulator to stream whatever we can readily
discover about a value at a specified Lua stack index.

Add lua_stack() to report the contents of the Lua stack. Since the stack is
created anew for every call to a C function, this shouldn't usually be
enormous.

Add hexdump.h with iostream manipulators to dump a byte range as hex digits,
or to produce readable text from a mix of printing and nonprinting ASCII
characters.

3 files changed, 465 insertions, 8 deletions

diff --git a/indra/llcommon/hexdump.h b/indra/llcommon/hexdump.h
new file mode 100755
index 0000000000..234168cd61
--- /dev/null
+++ b/indra/llcommon/hexdump.h
@@ -0,0 +1,106 @@
+/**
+ * @file   hexdump.h
+ * @author Nat Goodspeed
+ * @date   2023-10-03
+ * @brief  iostream manipulators to stream hex, or string with nonprinting chars
+ * 
+ * $LicenseInfo:firstyear=2023&license=viewerlgpl$
+ * Copyright (c) 2023, Linden Research, Inc.
+ * $/LicenseInfo$
+ */
+
+#if ! defined(LL_HEXDUMP_H)
+#define LL_HEXDUMP_H
+
+#include <cctype>
+#include <iomanip>
+#include <iostream>
+#include <string_view>
+
+namespace LL
+{
+
+// Format a given byte string as 2-digit hex values, no separators
+// Usage: std::cout << hexdump(somestring) << ...
+class hexdump
+{
+public:
+    hexdump(const std::string_view& data):
+        hexdump(data.data(), data.length())
+    {}
+
+    hexdump(const char* data, size_t len):
+        hexdump(reinterpret_cast<const unsigned char*>(data), len)
+    {}
+
+    hexdump(const std::vector<unsigned char>& data):
+        hexdump(data.data(), data.size())
+    {}
+
+    hexdump(const unsigned char* data, size_t len):
+        mData(data, data + len)
+    {}
+
+    friend std::ostream& operator<<(std::ostream& out, const hexdump& self)
+    {
+        auto oldfmt{ out.flags() };
+        auto oldfill{ out.fill() };
+        out.setf(std::ios_base::hex, std::ios_base::basefield);
+        out.fill('0');
+        for (auto c : self.mData)
+        {
+            out << std::setw(2) << unsigned(c);
+        }
+        out.setf(oldfmt, std::ios_base::basefield);
+        out.fill(oldfill);
+        return out;
+    }
+
+private:
+    std::vector<unsigned char> mData;
+};
+
+// Format a given byte string as a mix of printable characters and, for each
+// non-printable character, "\xnn"
+// Usage: std::cout << hexmix(somestring) << ...
+class hexmix
+{
+public:
+    hexmix(const std::string_view& data):
+        mData(data)
+    {}
+
+    hexmix(const char* data, size_t len):
+        mData(data, len)
+    {}
+
+    friend std::ostream& operator<<(std::ostream& out, const hexmix& self)
+    {
+        auto oldfmt{ out.flags() };
+        auto oldfill{ out.fill() };
+        out.setf(std::ios_base::hex, std::ios_base::basefield);
+        out.fill('0');
+        for (auto c : self.mData)
+        {
+            // std::isprint() must be passed an unsigned char!
+            if (std::isprint(static_cast<unsigned char>(c)))
+            {
+                out << c;
+            }
+            else
+            {
+                out << "\\x" << std::setw(2) << unsigned(c);
+            }
+        }
+        out.setf(oldfmt, std::ios_base::basefield);
+        out.fill(oldfill);
+        return out;
+    }
+
+private:
+    std::string mData;
+};
+
+} // namespace LL
+
+#endif /* ! defined(LL_HEXDUMP_H) */
diff --git a/indra/newview/llluamanager.cpp b/indra/newview/llluamanager.cpp
index 61c0ecf1ed..5efe4ac7a6 100644
--- a/indra/newview/llluamanager.cpp
+++ b/indra/newview/llluamanager.cpp
@@ -28,6 +28,7 @@
 #include "llviewerprecompiledheaders.h"
 #include "llluamanager.h"
 
+#include "hexdump.h"
 #include "llerror.h"
 #include "lleventcoro.h"
 #include "lleventfilter.h"
@@ -1070,6 +1071,122 @@ void lua_pushstdstring(lua_State* L, const std::string& str)
     lua_pushlstring(L, str.c_str(), str.length());
 }
 
+// Usage:  std::cout << lua_what(L, stackindex) << ...;
+// Reports on the Lua value found at the passed stackindex.
+// If cast to std::string, returns the corresponding string value.
+class lua_what
+{
+public:
+    lua_what(lua_State* state, int idx):
+        L(state),
+        index(idx)
+    {}
+
+    friend std::ostream& operator<<(std::ostream& out, const lua_what& self)
+    {
+        switch (lua_type(self.L, self.index))
+        {
+        case LUA_TNONE:
+            // distinguish acceptable but non-valid index
+            out << "none";
+            break;
+
+        case LUA_TNIL:
+            out << "nil";
+            break;
+
+        case LUA_TBOOLEAN:
+        {
+            auto oldflags { out.flags() };
+            out << std::boolalpha << lua_toboolean(self.L, self.index);
+            out.flags(oldflags);
+            break;
+        }
+
+        case LUA_TNUMBER:
+            out << lua_tonumber(self.L, self.index);
+            break;
+
+        case LUA_TSTRING:
+            out << std::quoted(lua_tostdstring(self.L, self.index));
+            break;
+
+        case LUA_TUSERDATA:
+        {
+            const size_t maxlen = 20;
+            auto binlen{ lua_rawlen(self.L, self.index) };
+            LLSD::Binary binary(std::min(maxlen, binlen));
+            std::memcpy(binary.data(), lua_touserdata(self.L, self.index), binary.size());
+            out << LL::hexdump(binary);
+            if (binlen > maxlen)
+            {
+                out << "...(" << (binlen - maxlen) << " more)";
+            }
+            break;
+        }
+
+        case LUA_TLIGHTUSERDATA:
+            out << lua_touserdata(self.L, self.index);
+            break;
+
+        default:
+            // anything else, don't bother trying to report value, just type
+            out << lua_typename(self.L, lua_type(self.L, self.index));
+            break;
+        }
+        return out;
+    }
+
+    operator std::string() const { return stringize(*this); }
+
+private:
+    lua_State* L;
+    int index;
+};
+
+// Usage:  std::cout << lua_stack(L) << ...;
+// Reports on the contents of the Lua stack.
+// If cast to std::string, returns the corresponding string value.
+class lua_stack
+{
+public:
+    lua_stack(lua_State* state):
+        L(state)
+    {}
+
+    friend std::ostream& operator<<(std::ostream& out, const lua_stack& self)
+    {
+        const char* sep = "stack: [";
+        for (int index = 1; index <= lua_gettop(self.L); ++index)
+        {
+            out << sep << lua_what(self.L, index);
+            sep = ", ";
+        }
+        out << ']';
+        return out;
+    }
+
+    operator std::string() const { return stringize(*this); }
+
+private:
+    lua_State* L;
+};
+
+// log exit from any block declaring an instance of DebugExit, regardless of
+// how control leaves that block
+struct DebugExit
+{
+    DebugExit(const std::string& name): mName(name) {}
+    DebugExit(const DebugExit&) = delete;
+    DebugExit& operator=(const DebugExit&) = delete;
+    ~DebugExit()
+    {
+        LL_DEBUGS("Lua") << "exit " << mName << LL_ENDL;
+    }
+
+    std::string mName;
+};
+
 // By analogy with existing lua_tomumble() functions, return an LLSD object
 // corresponding to the Lua object at stack index 'index' in state L.
 // This function assumes that a Lua caller is fully aware that they're trying
@@ -1087,6 +1204,9 @@ void lua_pushstdstring(lua_State* L, const std::string& str)
 // reached by that block raises a Lua error.
 LLSD lua_tollsd(lua_State* L, int index)
 {
+    LL_DEBUGS("Lua") << "lua_tollsd(" << index << ") of " << lua_gettop(L) << " stack entries: "
+                     << lua_what(L, index) << LL_ENDL;
+    DebugExit log_exit("lua_tollsd()");
     switch (lua_type(L, index))
     {
     case LUA_TNONE:
@@ -1158,17 +1278,36 @@ LLSD lua_tollsd(lua_State* L, int index)
         // - LLSD::Real with integer value returns as LLSD::Integer.
         // - LLSD::UUID, LLSD::Date and LLSD::URI all convert to Lua string,
         //   and so return as LLSD::String.
+        // - Lua does not store any table key whose value is nil. An LLSD
+        //   array with isUndefined() entries produces a Lua table with
+        //   "holes" in the int key sequence; this converts back to an LLSD
+        //   array containing corresponding isUndefined() entries -- except
+        //   when one or more of the final entries isUndefined(). These are
+        //   simply dropped, producing a shorter LLSD array than the original.
+        // - For the same reason, any keys in an LLSD map whose value
+        //   isUndefined() are simply discarded in the converted Lua table.
+        //   This converts back to an LLSD map lacking those keys.
+        // - If it's important to preserve the original length of an LLSD
+        //   array whose final entries are undefined, or the full set of keys
+        //   for an LLSD map some of whose values are undefined, store an
+        //   LLSD::emptyArray() or emptyMap() instead. These will be
+        //   represented in Lua as empty table, which should convert back to
+        //   undefined LLSD. Naturally, though, those won't survive a second
+        //   round trip.
 
         // This is the most important of the luaL_checkstack() calls because a
         // deeply nested Lua structure will enter this case at each level, and
         // we'll need another 2 stack slots to traverse each nested table.
         luaL_checkstack(L, 2, nullptr);
+        LL_DEBUGS("Lua") << "checking for empty table" << LL_ENDL;
         lua_pushnil(L);             // first key
+        LL_DEBUGS("Lua") << lua_stack(L) << LL_ENDL;
         if (! lua_next(L, index))
         {
             // it's a table, but the table is empty -- no idea if it should be
             // modeled as empty array or empty map -- return isUndefined(),
             // which can be consumed as either
+            LL_DEBUGS("Lua") << "empty table" << LL_ENDL;
             return {};
         }
         // key is at stack index -2, value at index -1
@@ -1177,6 +1316,8 @@ LLSD lua_tollsd(lua_State* L, int index)
         LuaPopper popper(L, 2);
         // Remember the type of the first key
         auto firstkeytype{ lua_type(L, -2) };
+        LL_DEBUGS("Lua") << "table not empty, first key type " << lua_typename(L, firstkeytype)
+                         << LL_ENDL;
         switch (firstkeytype)
         {
         case LUA_TNUMBER:
@@ -1254,6 +1395,7 @@ LLSD lua_tollsd(lua_State* L, int index)
                 // crazy key territory.
                 return luaL_error(L, "Gaps in Lua table too large for conversion to LLSD array");
             }
+            LL_DEBUGS("Lua") << "collected " << keys.size() << " keys, max " << highkey << LL_ENDL;
             // right away expand the result array to the size we'll need
             LLSD result{ LLSD::emptyArray() };
             result[highkey - 1] = LLSD();
@@ -1263,8 +1405,10 @@ LLSD lua_tollsd(lua_State* L, int index)
             {
                 // key at stack index -2, value at index -1
                 // We've already validated lua_tointegerx() for each key.
+                auto key{ lua_tointeger(L, -2) };
+                LL_DEBUGS("Lua") << "key " << key << ':' << LL_ENDL;
                 // Don't forget to subtract 1 from Lua key for LLSD subscript!
-                result[LLSD::Integer(lua_tointeger(L, -2)) - 1] = lua_tollsd(L, -1);
+                result[LLSD::Integer(key) - 1] = lua_tollsd(L, -1);
                 // remove value, keep key for next iteration
                 lua_pop(L, 1);
             }
@@ -1283,8 +1427,10 @@ LLSD lua_tollsd(lua_State* L, int index)
                     return luaL_error(L, "Cannot convert %s map key to LLSD",
                                       lua_typename(L, mapkeytype));
                 }
-                
-                result[lua_tostdstring(L, -2)] = lua_tollsd(L, -1);
+
+                auto key{ lua_tostdstring(L, -2) };
+                LL_DEBUGS("Lua") << "map key " << std::quoted(key) << ':' << LL_ENDL;
+                result[key] = lua_tollsd(L, -1);
                 // remove value, keep key for next iteration
                 lua_pop(L, 1);
             } while (lua_next(L, index) != 0);
diff --git a/indra/newview/tests/llluamanager_test.cpp b/indra/newview/tests/llluamanager_test.cpp
index e5e06b095c..1363f114c1 100644
--- a/indra/newview/tests/llluamanager_test.cpp
+++ b/indra/newview/tests/llluamanager_test.cpp
@@ -15,12 +15,17 @@
 #include "../newview/llluamanager.h"
 // STL headers
 // std headers
+#include <vector>
 // external library headers
 // other Linden headers
 #include "../test/lltut.h"
 #include "llapp.h"
+#include "lldate.h"
 #include "llevents.h"
 #include "lleventcoro.h"
+#include "llsdutil.h"
+#include "lluri.h"
+#include "lluuid.h"
 #include "stringize.h"
 #include "../llcommon/tests/StringVec.h"
 
@@ -32,6 +37,17 @@ public:
     bool frame()   override { return true; }
 };
 
+template <typename CALLABLE>
+auto listener(CALLABLE&& callable)
+{
+    return [callable=std::forward<CALLABLE>(callable)]
+    (const LLSD& data)
+    {
+        callable(data);
+        return false;
+    };
+}
+
 /*****************************************************************************
 *   TUT
 *****************************************************************************/
@@ -56,11 +72,8 @@ namespace tut
         LLEventStream replypump("testpump");
         LLTempBoundListener conn(
             replypump.listen("test<1>",
-                             [&posts](const LLSD& post)
-                             {
-                                 posts.push_back(post.asString());
-                                 return false;
-                             }));
+                             listener([&posts](const LLSD& data)
+                             { posts.push_back(data.asString()); })));
         const std::string lua(
             "-- test post_on,listen_events,await_event\n"
             "post_on('testpump', 'entry')\n"
@@ -98,4 +111,196 @@ namespace tut
         llcoro::suspend();
         ensure_equals("post_on() sequence", posts, expected);
     }
+
+    void from_lua(const std::string& desc, const std::string_view& construct, const LLSD& expect)
+    {
+        LLSD fromlua;
+        LLEventStream replypump("testpump");
+        LLTempBoundListener conn(
+            replypump.listen("llluamanager_test",
+                             listener([&fromlua](const LLSD& data){ fromlua = data; })));
+        const std::string lua(stringize(
+            "-- test LLSD synthesized by Lua\n",
+            // we expect the caller's Lua snippet to construct a Lua object
+            // called 'data'
+            construct, "\n"
+            "post_on('testpump', data)\n"
+        ));
+        LLLUAmanager::runScriptLine(lua);
+        // At this point LLLUAmanager::runScriptLine() has launched a new C++
+        // coroutine to run the passed Lua snippet, but that coroutine hasn't
+        // yet had a chance to run. Poke the coroutine scheduler until the Lua
+        // script has sent its data.
+        for (int i = 0; i < 10 && fromlua.isUndefined(); ++i)
+        {
+            llcoro::suspend();
+        }
+        // We woke up again ourselves because the coroutine running Lua has
+        // finished.
+        ensure_equals(desc, fromlua, expect);
+    }
+
+    template<> template<>
+    void object::test<2>()
+    {
+        set_test_name("LLSD from Lua");
+        from_lua("nil", "data = nil", LLSD());
+        from_lua("true", "data = true", true);
+        from_lua("false", "data = false", false);
+        from_lua("int", "data = 17", 17);
+        from_lua("real", "data = 3.14", 3.14);
+        from_lua("string", "data = 'string'", "string");
+        // can't synthesize Lua userdata in Lua code: that can only be
+        // constructed by a C function
+        from_lua("empty table", "data = {}", LLSD());
+        from_lua("nested empty table", "data = { 1, 2, 3, {}, 5 }",
+                 llsd::array(1, 2, 3, LLSD(), 5));
+        from_lua("nested non-empty table", "data = { 1, 2, 3, {a=0, b=1}, 5 }",
+                 llsd::array(1, 2, 3, llsd::map("a", 0, "b", 1), 5));
+    }
+
+    void round_trip(const std::string& desc, const LLSD& send, const LLSD& expect)
+    {
+        LLSD reply;
+        LLEventStream replypump("testpump");
+        LLTempBoundListener conn(
+            replypump.listen("llluamanager_test",
+                             listener([&reply](const LLSD& post){ reply = post; })));
+        const std::string lua(
+            "-- test LLSD round trip\n"
+            "callback = function(pump, data)\n"
+            "    -- just echo the data we received\n"
+            "    post_on('testpump', data)\n"
+            "end\n"
+            "replypump, cmdpump = listen_events(callback)\n"
+            "post_on('testpump', replypump)\n"
+            "await_event(replypump)\n"
+        );
+        LLLUAmanager::runScriptLine(lua);
+        // At this point LLLUAmanager::runScriptLine() has launched a new C++
+        // coroutine to run the passed Lua snippet, but that coroutine hasn't
+        // yet had a chance to run. Poke the coroutine scheduler until the Lua
+        // script has sent its reply pump name.
+        for (int i = 0; i < 10 && reply.isUndefined(); ++i)
+        {
+            llcoro::suspend();
+        }
+        // We woke up again ourselves because the coroutine running Lua has
+        // reached the await_event() call, which suspends the calling C++
+        // coroutine (including the Lua code running on it) until we post
+        // something to that reply pump.
+        auto luapump{ reply.asString() };
+        reply.clear();
+        LLEventPumps::instance().post(luapump, send);
+        // The C++ coroutine running the Lua script is now ready to run. Run
+        // it so it will echo the LLSD back to us.
+        llcoro::suspend();
+        ensure_equals(desc, reply, expect);
+    }
+
+    // Define an RTItem to be used for round-trip LLSD testing: what it is,
+    // what we send to Lua, what we expect to get back. They could be the
+    // same.
+    struct RTItem
+    {
+        RTItem(const std::string& name, const LLSD& send, const LLSD& expect):
+            mName(name),
+            mSend(send),
+            mExpect(expect)
+        {}
+        RTItem(const std::string& name, const LLSD& both):
+            mName(name),
+            mSend(both),
+            mExpect(both)
+        {}
+
+        std::string mName;
+        LLSD mSend, mExpect;
+    };
+
+    bool will_be_nil(const LLSD& data)
+    {
+        // undefined LLSD converts to Lua nil.
+        // empty LLSD array or empty LLSD map converts to nil.
+        return (data.isUndefined() ||
+                ((data.isArray() || data.isMap()) && data.size() == 0));
+    }
+
+    template<> template<>
+    void object::test<3>()
+    {
+        set_test_name("LLSD round trip");
+        LLSD::Binary binary{ 3, 1, 4, 1, 5, 9, 2, 6, 5 };
+        const char* uuid{ "01234567-abcd-0123-4567-0123456789ab" };
+        const char* date{ "2023-10-04T21:06:00Z" };
+        const char* uri{ "https://secondlife.com/index.html" };
+        std::vector<RTItem> items{
+            RTItem("undefined", LLSD()),
+            RTItem("true", true),
+            RTItem("false", false),
+            RTItem("int", 17),
+            RTItem("real", 3.14),
+            RTItem("int real", 27.0, 27),
+            RTItem("string", "string"),
+            RTItem("binary", binary),
+            RTItem("empty array", LLSD::emptyArray(), LLSD()),
+            RTItem("empty map", LLSD::emptyMap(), LLSD()),
+            RTItem("UUID", LLUUID(uuid), uuid),
+            RTItem("date", LLDate(date), date),
+            RTItem("uri", LLURI(uri), uri)
+            };
+        // scalars
+        for (const auto& item: items)
+        {
+            round_trip(item.mName, item.mSend, item.mExpect);
+        }
+
+        // array
+        LLSD send_array{ LLSD::emptyArray() }, expect_array{ LLSD::emptyArray() };
+        for (const auto& item: items)
+        {
+            // BUG AVOIDANCE:
+            // As of 2023-10-04, lua_tollsd() hits access violation in
+            // lua_next() when handed a table nested in another table.
+            if (! (item.mSend.isArray() || item.mSend.isMap()))
+            {
+                send_array.append(item.mSend);
+                expect_array.append(item.mExpect);
+            }
+        }
+        // Lua takes a table value of nil to mean: don't store this key. An
+        // LLSD array containing undefined entries (converted to nil) leaves
+        // "holes" in the Lua table. These will be converted back to undefined
+        // LLSD entries -- except at the end. Trailing undefined entries are
+        // simply omitted from the table -- so the table converts back to a
+        // shorter LLSD array. We've constructed send_array and expect_array
+        // according to 'items' above -- but truncate from expect_array any
+        // trailing entries that will map to Lua nil.
+        while (expect_array.size() > 0 &&
+               will_be_nil(expect_array[expect_array.size() - 1]))
+        {
+            expect_array.erase(expect_array.size() - 1);
+        }
+        round_trip("array", send_array, expect_array);
+
+        // map
+        LLSD send_map{ LLSD::emptyMap() }, expect_map{ LLSD::emptyMap() };
+        for (const auto& item: items)
+        {
+            // BUG AVOIDANCE:
+            // see comment in the send_array construction loop above
+            if (! (item.mSend.isArray() || item.mSend.isMap()))
+            {
+                send_map[item.mName] = item.mSend;
+                // see comment in the expect_array truncation loop above --
+                // keep this test because Lua never stores table entries with
+                // nil values
+                if (! will_be_nil(item.mExpect))
+                {
+                    expect_map[item.mName] = item.mExpect;
+                }
+            }
+        }
+        round_trip("map", send_map, expect_map);
+    }
 } // namespace tut