From 394e5f18b6fb034b04fae0f35575dc24a4a1d620 Mon Sep 17 00:00:00 2001 From: Vas Crabb Date: Wed, 16 Mar 2022 04:13:18 +1100 Subject: Fixed some debugger memory view annoyances and cleaned up Lua bindings. Made the debugger memory view not depend on isprint which is affected by the global locale. Assume the OSD will display as ISO-8869-1 and replace problematic printable characters. Started changing Lua function bindings to use set_function to avoid potential issues related to ThePhD/sol2#608, and worked out what was causing problems with symbol table read_memory/write_memory. (They aren't really essential - you can do the same thing with the address space object itself, but they're easier to parameterise.) --- docs/source/techspecs/luareference.rst | 12 ++ src/emu/debug/dvmemory.cpp | 20 +++- src/frontend/mame/luaengine.h | 2 + src/frontend/mame/luaengine_debug.cpp | 177 +++++++++++++++--------------- src/frontend/mame/luaengine_render.cpp | 4 +- src/osd/modules/debugger/osx/debugview.mm | 2 +- 6 files changed, 124 insertions(+), 93 deletions(-) diff --git a/docs/source/techspecs/luareference.rst b/docs/source/techspecs/luareference.rst index 0ffbe310157..e2e33b362f0 100644 --- a/docs/source/techspecs/luareference.rst +++ b/docs/source/techspecs/luareference.rst @@ -3237,6 +3237,18 @@ symbols:set_memory_value(name, space, offset, value, size, disable_se) The access size is specified in bytes, and must be 1, 2, 4 or 8. The ``disable_se`` argument specifies whether memory access side effects should be disabled. +symbols:read_memory(space, address, size, apply_translation) + Read a value from an address space. The access size is specified in bytes, + and must be 1, 2, 4, or 8. If the ``apply_translation`` argument is true, + the address will be translated with debug read intention. Returns a value + of the requested size with all bits set if address translation fails. +symbols:write_memory(space, address, data, size, apply_translation) + Write a value to an address space. The access size is specified in bytes, + and must be 1, 2, 4, or 8. If the ``apply_translation`` argument is true, + the address will be translated with debug write intention. The symbol + table’s memory modified function will be called after the value is written. + The value will not be written and the symbol table’s memory modified + function will not be called if address translation fails. Properties ^^^^^^^^^^ diff --git a/src/emu/debug/dvmemory.cpp b/src/emu/debug/dvmemory.cpp index 02fcde9cbc3..4831023d479 100644 --- a/src/emu/debug/dvmemory.cpp +++ b/src/emu/debug/dvmemory.cpp @@ -19,6 +19,20 @@ #include +//************************************************************************** +// HELPER FUNCTIONS +//************************************************************************** + +namespace { + +constexpr u8 sanitise_character(u8 ch) +{ + // assume ISO-8859-1 (low 256 Unicode codepoints) - tab, soft hyphen, C0 and C1 cause problems + return ('\t' == ch) ? ' ' : (0xadU == ch) ? '-' : ((' ' > ch) || (('~' < ch) && (0xa0U > ch))) ? '.' : ch; +} + +} // anonymous namespace + //************************************************************************** // GLOBAL VARIABLES //************************************************************************** @@ -301,7 +315,7 @@ void debug_view_memory::generate_row(debug_view_char *destmin, debug_view_char * if (dest >= destmin && dest < destmax) dest->byte = addrtext[ch]; - // generate the data and the ascii string + // generate the data and the ASCII string std::string chunkascii; if (m_shift_bits != 0) { @@ -323,7 +337,7 @@ void debug_view_memory::generate_row(debug_view_char *destmin, debug_view_char * for (int i = 0; i < m_bytes_per_chunk; i++) { u8 chval = chunkdata >> (8 * (m_bytes_per_chunk - i - 1)); - chunkascii += char((ismapped && isprint(chval)) ? chval : '.'); + chunkascii += char(ismapped ? sanitise_character(chval) : '.'); } } } @@ -380,7 +394,7 @@ void debug_view_memory::generate_row(debug_view_char *destmin, debug_view_char * for (int i = 0; i < m_bytes_per_chunk; i++) { u8 chval = chunkdata >> (8 * (m_bytes_per_chunk - i - 1)); - chunkascii += char((ismapped && isprint(chval)) ? chval : '.'); + chunkascii += char(ismapped ? sanitise_character(chval) : '.'); } } } diff --git a/src/frontend/mame/luaengine.h b/src/frontend/mame/luaengine.h index 1aaa83bbf0b..b070969f424 100644 --- a/src/frontend/mame/luaengine.h +++ b/src/frontend/mame/luaengine.h @@ -123,6 +123,8 @@ private: struct addr_space; class tap_helper; class addr_space_change_notif; + class symbol_table_wrapper; + class expression_wrapper; struct save_item { void *base; diff --git a/src/frontend/mame/luaengine_debug.cpp b/src/frontend/mame/luaengine_debug.cpp index 749d4c34aa0..94748d5a897 100644 --- a/src/frontend/mame/luaengine_debug.cpp +++ b/src/frontend/mame/luaengine_debug.cpp @@ -30,7 +30,61 @@ struct wrap_textbuf }; -class symbol_table_wrapper +template +sol::object do_breakpoint_enable(device_debug &dev, sol::this_state s, sol::object index) +{ + if (index == sol::lua_nil) + { + dev.breakpoint_enable_all(Enable); + dev.device().machine().debug_view().update_all(DVT_DISASSEMBLY); + dev.device().machine().debug_view().update_all(DVT_BREAK_POINTS); + return sol::lua_nil; + } + else if (index.is()) + { + bool result(dev.breakpoint_enable(index.as(), Enable)); + if (result) + { + dev.device().machine().debug_view().update_all(DVT_DISASSEMBLY); + dev.device().machine().debug_view().update_all(DVT_BREAK_POINTS); + } + return sol::make_object(s, result); + } + else + { + osd_printf_error("[LUA ERROR] must call bpenable with integer or nil\n"); + return sol::lua_nil; + } +} + + +template +sol::object do_watchpoint_enable(device_debug &dev, sol::this_state s, sol::object index) +{ + if (index == sol::lua_nil) + { + dev.watchpoint_enable_all(Enable); + dev.device().machine().debug_view().update_all(DVT_WATCH_POINTS); + return sol::lua_nil; + } + else if (index.is()) + { + bool result(dev.watchpoint_enable(index.as(), Enable)); + if (result) + dev.device().machine().debug_view().update_all(DVT_WATCH_POINTS); + return sol::make_object(s, result); + } + else + { + osd_printf_error("[LUA ERROR] must call wpenable with integer or nil"); + return sol::lua_nil; + } +} + +} // anonymous namespace + + +class lua_engine::symbol_table_wrapper { public: symbol_table_wrapper(symbol_table_wrapper const &) = delete; @@ -53,13 +107,16 @@ public: u64 value(const char *symbol) { return m_table.value(symbol); } void set_value(const char *symbol, u64 value) { m_table.set_value(symbol, value); } + u64 read_memory(addr_space &space, offs_t address, int size, bool translate) { return m_table.read_memory(space.space, address, size, translate); } + void write_memory(addr_space &space, offs_t address, u64 data, int size, bool translate) { m_table.write_memory(space.space, address, data, size, translate); } + private: symbol_table m_table; std::shared_ptr const m_parent; }; -class expression_wrapper +class lua_engine::expression_wrapper { public: expression_wrapper(expression_wrapper const &) = delete; @@ -102,60 +159,6 @@ private: }; -template -sol::object do_breakpoint_enable(device_debug &dev, sol::this_state s, sol::object index) -{ - if (index == sol::lua_nil) - { - dev.breakpoint_enable_all(Enable); - dev.device().machine().debug_view().update_all(DVT_DISASSEMBLY); - dev.device().machine().debug_view().update_all(DVT_BREAK_POINTS); - return sol::lua_nil; - } - else if (index.is()) - { - bool result(dev.breakpoint_enable(index.as(), Enable)); - if (result) - { - dev.device().machine().debug_view().update_all(DVT_DISASSEMBLY); - dev.device().machine().debug_view().update_all(DVT_BREAK_POINTS); - } - return sol::make_object(s, result); - } - else - { - osd_printf_error("[LUA ERROR] must call bpenable with integer or nil\n"); - return sol::lua_nil; - } -} - - -template -sol::object do_watchpoint_enable(device_debug &dev, sol::this_state s, sol::object index) -{ - if (index == sol::lua_nil) - { - dev.watchpoint_enable_all(Enable); - dev.device().machine().debug_view().update_all(DVT_WATCH_POINTS); - return sol::lua_nil; - } - else if (index.is()) - { - bool result(dev.watchpoint_enable(index.as(), Enable)); - if (result) - dev.device().machine().debug_view().update_all(DVT_WATCH_POINTS); - return sol::make_object(s, result); - } - else - { - osd_printf_error("[LUA ERROR] must call wpenable with integer or nil"); - return sol::lua_nil; - } -} - -} // anonymous namespace - - void lua_engine::initialize_debug(sol::table &emu) { @@ -213,7 +216,7 @@ void lua_engine::initialize_debug(sol::table &emu) { return std::make_shared(parent->table().machine(), parent, nullptr); }, [] (device_t &device) { return std::make_shared(device.machine(), nullptr, &device); })); - symbol_table_type["set_memory_modified_func"] = + symbol_table_type.set_function("set_memory_modified_func", [this] (symbol_table_wrapper &st, sol::object cb) { if (cb == sol::lua_nil) @@ -222,7 +225,7 @@ void lua_engine::initialize_debug(sol::table &emu) st.table().set_memory_modified_func([this, cbfunc = cb.as()] () { invoke(cbfunc); }); else osd_printf_error("[LUA ERROR] must call set_memory_modified_func with function or nil\n"); - }; + }); symbol_table_type["add"] = sol::overload( static_cast(&symbol_table_wrapper::add), static_cast(&symbol_table_wrapper::add), @@ -261,24 +264,24 @@ void lua_engine::initialize_debug(sol::table &emu) return result ? *result : 0; }); }); - symbol_table_type["find"] = &symbol_table_wrapper::find; - symbol_table_type["find_deep"] = &symbol_table_wrapper::find_deep; - symbol_table_type["value"] = &symbol_table_wrapper::value; - symbol_table_type["set_value"] = &symbol_table_wrapper::set_value; - symbol_table_type["memory_value"] = + symbol_table_type.set_function("find", &symbol_table_wrapper::find); + symbol_table_type.set_function("find_deep", &symbol_table_wrapper::find_deep); + symbol_table_type.set_function("value", &symbol_table_wrapper::value); + symbol_table_type.set_function("set_value", &symbol_table_wrapper::set_value); + symbol_table_type.set_function("memory_value", [] (symbol_table_wrapper &st, char const *name, char const *space, u32 offset, int size, bool disable_se) { expression_space const es = s_expression_space_parser(space); return st.table().memory_value(name, es, offset, size, disable_se); - }; - symbol_table_type["set_memory_value"] = + }); + symbol_table_type.set_function("set_memory_value", [] (symbol_table_wrapper &st, char const *name, char const *space, u32 offset, int size, u64 value, bool disable_se) { expression_space const es = s_expression_space_parser(space); st.table().set_memory_value(name, es, offset, size, value, disable_se); - }; - //symbol_table_type["read_memory"] = &symbol_table::read_memory; crashes if you try to use it, need to work out why - //symbol_table_type["write_memory"] = &symbol_table::write_memory; crashes if you try to use it, need to work out why + }); + symbol_table_type.set_function("read_memory", &symbol_table_wrapper::read_memory); + symbol_table_type.set_function("write_memory", &symbol_table_wrapper::write_memory); symbol_table_type["entries"] = sol::property([] (symbol_table_wrapper const &st) { return standard_tag_object_ptr_map(st.table().entries()); }); symbol_table_type["parent"] = sol::property(&symbol_table_wrapper::parent); @@ -289,9 +292,9 @@ void lua_engine::initialize_debug(sol::table &emu) expression_wrapper(std::shared_ptr const &), expression_wrapper(std::shared_ptr const &, char const *, int), expression_wrapper(std::shared_ptr const &, char const *)>()); - parsed_expression_type["set_default_base"] = &expression_wrapper::set_default_base; - parsed_expression_type["parse"] = &expression_wrapper::parse; - parsed_expression_type["execute"] = &expression_wrapper::execute; + parsed_expression_type.set_function("set_default_base", &expression_wrapper::set_default_base); + parsed_expression_type.set_function("parse", &expression_wrapper::parse); + parsed_expression_type.set_function("execute", &expression_wrapper::execute); parsed_expression_type["is_empty"] = sol::property(&expression_wrapper::is_empty); parsed_expression_type["original_string"] = sol::property(&expression_wrapper::original_string); parsed_expression_type["symbols"] = sol::property(&expression_wrapper::symbols, &expression_wrapper::set_symbols); @@ -306,7 +309,7 @@ void lua_engine::initialize_debug(sol::table &emu) auto debugger_type = sol().registry().new_usertype("debugger", sol::no_constructor); - debugger_type["command"] = [] (debugger_manager &debug, std::string const &cmd) { debug.console().execute_command(cmd, false); }; + debugger_type.set_function("command", [] (debugger_manager &debug, std::string const &cmd) { debug.console().execute_command(cmd, false); }); debugger_type["consolelog"] = sol::property([] (debugger_manager &debug) { return wrap_textbuf(debug.console().get_console_textbuf()); }); debugger_type["errorlog"] = sol::property([](debugger_manager &debug) { return wrap_textbuf(debug.console().get_errorlog_textbuf()); }); debugger_type["visible_cpu"] = sol::property( @@ -340,23 +343,23 @@ void lua_engine::initialize_debug(sol::table &emu) auto device_debug_type = sol().registry().new_usertype("device_debug", sol::no_constructor); - device_debug_type["step"] = + device_debug_type.set_function("step", [] (device_debug &dev, sol::object num) { int steps = 1; if (num.is()) steps = num.as(); dev.single_step(steps); - }; - device_debug_type["go"] = &device_debug::go; - device_debug_type["bpset"] = + }); + device_debug_type.set_function("go", &device_debug::go); + device_debug_type.set_function("bpset", [] (device_debug &dev, offs_t address, char const *cond, char const *act) { int result(dev.breakpoint_set(address, cond, act)); dev.device().machine().debug_view().update_all(DVT_DISASSEMBLY); dev.device().machine().debug_view().update_all(DVT_BREAK_POINTS); return result; - }; + }); device_debug_type["bpclear"] = sol::overload( [] (device_debug &dev, int index) { @@ -374,24 +377,24 @@ void lua_engine::initialize_debug(sol::table &emu) dev.device().machine().debug_view().update_all(DVT_DISASSEMBLY); dev.device().machine().debug_view().update_all(DVT_BREAK_POINTS); }); - device_debug_type["bpenable"] = &do_breakpoint_enable; - device_debug_type["bpdisable"] = &do_breakpoint_enable; - device_debug_type["bplist"] = + device_debug_type.set_function("bpenable", &do_breakpoint_enable); + device_debug_type.set_function("bpdisable", &do_breakpoint_enable); + device_debug_type.set_function("bplist", [this] (device_debug &dev) { sol::table table = sol().create_table(); for (auto const &bpp : dev.breakpoint_list()) table[bpp.second->index()] = sol::make_reference(sol(), bpp.second.get()); return table; - }; - device_debug_type["wpset"] = + }); + device_debug_type.set_function("wpset", [] (device_debug &dev, addr_space &sp, std::string const &type, offs_t addr, offs_t len, char const *cond, char const *act) { read_or_write const wptype = s_read_or_write_parser(type); int result(dev.watchpoint_set(sp.space, wptype, addr, len, cond, act)); dev.device().machine().debug_view().update_all(DVT_WATCH_POINTS); return result; - }; + }); device_debug_type["wpclear"] = sol::overload( [] (device_debug &dev, int index) { @@ -405,16 +408,16 @@ void lua_engine::initialize_debug(sol::table &emu) dev.watchpoint_clear_all(); dev.device().machine().debug_view().update_all(DVT_WATCH_POINTS); }); - device_debug_type["wpenable"] = &do_watchpoint_enable; - device_debug_type["wpdisable"] = &do_watchpoint_enable; - device_debug_type["wplist"] = + device_debug_type.set_function("wpenable", &do_watchpoint_enable); + device_debug_type.set_function("wpdisable", &do_watchpoint_enable); + device_debug_type.set_function("wplist", [this] (device_debug &dev, addr_space &sp) { sol::table table = sol().create_table(); for (auto &wpp : dev.watchpoint_vector(sp.space.spacenum())) table[wpp->index()] = sol::make_reference(sol(), wpp.get()); return table; - }; + }); auto breakpoint_type = sol().registry().new_usertype("breakpoint", sol::no_constructor); diff --git a/src/frontend/mame/luaengine_render.cpp b/src/frontend/mame/luaengine_render.cpp index 21a68dc65cb..893412a4b30 100644 --- a/src/frontend/mame/luaengine_render.cpp +++ b/src/frontend/mame/luaengine_render.cpp @@ -418,7 +418,7 @@ void lua_engine::initialize_render(sol::table &emu) ctnr.add_line(x1, y1, x2, y2, UI_LINE_WIDTH, rgb_t(*color), PRIMFLAG_BLENDMODE(BLENDMODE_ALPHA)); }; render_container_type["draw_text"] = - [this] (render_container &ctnr, sol::object xobj, float y, char const *msg, std::optional fgcolor, std::optional bgcolor) + [] (render_container &ctnr, sol::this_state s, sol::object xobj, float y, char const *msg, std::optional fgcolor, std::optional bgcolor) { auto justify = ui::text_layout::text_justify::LEFT; float x = 0; @@ -438,7 +438,7 @@ void lua_engine::initialize_render(sol::table &emu) } else { - luaL_error(m_lua_state, "Error in param 1 to draw_text"); + luaL_error(s, "Error in param 1 to draw_text"); return; } y = std::clamp(y, 0.0f, 1.0f); diff --git a/src/osd/modules/debugger/osx/debugview.mm b/src/osd/modules/debugger/osx/debugview.mm index df7c4dbcc01..9d94d93e220 100644 --- a/src/osd/modules/debugger/osx/debugview.mm +++ b/src/osd/modules/debugger/osx/debugview.mm @@ -675,7 +675,7 @@ static void debugwin_view_update(debug_view &view, void *osdprivate) NSUInteger start = 0, length = 0; for (uint32_t col = origin.x; col < origin.x + size.x; col++) { - [[text mutableString] appendFormat:@"%c", data[col - origin.x].byte]; + [[text mutableString] appendFormat:@"%C", unsigned(data[col - origin.x].byte)]; if ((start < length) && (attr != data[col - origin.x].attrib)) { NSRange const run = NSMakeRange(start, length - start); -- cgit v1.2.3