[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module
siger-young updated this revision to Diff 375081.
siger-young added a comment.
This update mainly fixed problematic typemaps and adding necessary comments.
Together, it forced Lua installation path as "PREFIX/lib/lua/5.3" and removed
"lit.util" in tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
Files:
lldb/CMakeLists.txt
lldb/bindings/lua/CMakeLists.txt
lldb/bindings/lua/lua-typemaps.swig
lldb/test/API/lua_api/TestComprehensive.lua
lldb/test/API/lua_api/TestLuaAPI.py
Index: lldb/test/API/lua_api/TestLuaAPI.py
===
--- lldb/test/API/lua_api/TestLuaAPI.py
+++ lldb/test/API/lua_api/TestLuaAPI.py
@@ -5,8 +5,129 @@
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
-import lit.util
+import subprocess
+def to_string(b):
+"""Return the parameter as type 'str', possibly encoding it.
+
+In Python2, the 'str' type is the same as 'bytes'. In Python3, the
+'str' type is (essentially) Python2's 'unicode' type, and 'bytes' is
+distinct.
+
+"""
+if isinstance(b, str):
+# In Python2, this branch is taken for types 'str' and 'bytes'.
+# In Python3, this branch is taken only for 'str'.
+return b
+if isinstance(b, bytes):
+# In Python2, this branch is never taken ('bytes' is handled as 'str').
+# In Python3, this is true only for 'bytes'.
+try:
+return b.decode('utf-8')
+except UnicodeDecodeError:
+# If the value is not valid Unicode, return the default
+# repr-line encoding.
+return str(b)
+
+# By this point, here's what we *don't* have:
+#
+# - In Python2:
+#- 'str' or 'bytes' (1st branch above)
+# - In Python3:
+#- 'str' (1st branch above)
+#- 'bytes' (2nd branch above)
+#
+# The last type we might expect is the Python2 'unicode' type. There is no
+# 'unicode' type in Python3 (all the Python3 cases were already handled). In
+# order to get a 'str' object, we need to encode the 'unicode' object.
+try:
+return b.encode('utf-8')
+except AttributeError:
+raise TypeError('not sure how to convert %s to %s' % (type(b), str))
+
+class ExecuteCommandTimeoutException(Exception):
+def __init__(self, msg, out, err, exitCode):
+assert isinstance(msg, str)
+assert isinstance(out, str)
+assert isinstance(err, str)
+assert isinstance(exitCode, int)
+self.msg = msg
+self.out = out
+self.err = err
+self.exitCode = exitCode
+
+
+# Close extra file handles on UNIX (on Windows this cannot be done while
+# also redirecting input).
+kUseCloseFDs = not (platform.system() == 'Windows')
+
+
+def executeCommand(command, cwd=None, env=None, input=None, timeout=0):
+"""Execute command ``command`` (list of arguments or string) with.
+
+* working directory ``cwd`` (str), use None to use the current
+working directory
+* environment ``env`` (dict), use None for none
+* Input to the command ``input`` (str), use string to pass
+no input.
+* Max execution time ``timeout`` (int) seconds. Use 0 for no timeout.
+
+Returns a tuple (out, err, exitCode) where
+* ``out`` (str) is the standard output of running the command
+* ``err`` (str) is the standard error of running the command
+* ``exitCode`` (int) is the exitCode of running the command
+
+If the timeout is hit an ``ExecuteCommandTimeoutException``
+is raised.
+
+"""
+if input is not None:
+input = to_bytes(input)
+p = subprocess.Popen(command, cwd=cwd,
+stdin=subprocess.PIPE,
+stdout=subprocess.PIPE,
+stderr=subprocess.PIPE,
+env=env, close_fds=kUseCloseFDs)
+timerObject = None
+# FIXME: Because of the way nested function scopes work in Python 2.x we
+# need to use a reference to a mutable object rather than a plain
+# bool. In Python 3 we could use the "nonlocal" keyword but we need
+# to support Python 2 as well.
+hitTimeOut = [False]
+try:
+if timeout > 0:
+def killProcess():
+# We may be invoking a shell so we need to kill the
+# process and all its children.
+hitTimeOut[0] = True
+killProcessAndChildren(p.pid)
+
+timerObject = threading.Timer(timeout, killProcess)
+timerObject.start()
+
+out, err = p.communicate(input=input)
+exitCode = p.wait()
+finally:
+if timerObject != None:
+timerObject.cancel()
+
+# Ensure the resulting output is always of string type.
+out = to_string(out)
+err = to_string(err)
+
+if hitTimeOut[0]:
+
[Lldb-commits] [PATCH] D108515: [lldb/lua] Force Lua version to be 5.3
siger-young updated this revision to Diff 375086. siger-young added a comment. Remove "REQUIRED" flags when finding Lua at "FindLuaAndSwig.cmake". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108515/new/ https://reviews.llvm.org/D108515 Files: lldb/cmake/modules/FindLuaAndSwig.cmake lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt Index: lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt === --- lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt +++ lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt @@ -1,5 +1,3 @@ -find_package(Lua REQUIRED) - add_lldb_library(lldbPluginScriptInterpreterLua PLUGIN Lua.cpp ScriptInterpreterLua.cpp Index: lldb/cmake/modules/FindLuaAndSwig.cmake === --- lldb/cmake/modules/FindLuaAndSwig.cmake +++ lldb/cmake/modules/FindLuaAndSwig.cmake @@ -9,7 +9,7 @@ else() find_package(SWIG 3.0) if (SWIG_FOUND) -find_package(Lua 5.3) +find_package(Lua 5.3 EXACT) if(LUA_FOUND AND SWIG_FOUND) mark_as_advanced( LUA_LIBRARIES Index: lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt === --- lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt +++ lldb/source/Plugins/ScriptInterpreter/Lua/CMakeLists.txt @@ -1,5 +1,3 @@ -find_package(Lua REQUIRED) - add_lldb_library(lldbPluginScriptInterpreterLua PLUGIN Lua.cpp ScriptInterpreterLua.cpp Index: lldb/cmake/modules/FindLuaAndSwig.cmake === --- lldb/cmake/modules/FindLuaAndSwig.cmake +++ lldb/cmake/modules/FindLuaAndSwig.cmake @@ -9,7 +9,7 @@ else() find_package(SWIG 3.0) if (SWIG_FOUND) -find_package(Lua 5.3) +find_package(Lua 5.3 EXACT) if(LUA_FOUND AND SWIG_FOUND) mark_as_advanced( LUA_LIBRARIES ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D110472: [lldb] [gdb-remote] Use llvm::StringRef.split() and llvm::to_integer()
mgorny marked 3 inline comments as done. mgorny added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1667 +addr_t page; +if (llvm::to_integer(x, page)) dirty_page_list.push_back(page); teemperor wrote: > Base 16 arg is lost here. Good catch, thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110472/new/ https://reviews.llvm.org/D110472 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D110472: [lldb] [gdb-remote] Use llvm::StringRef.split() and llvm::to_integer()
mgorny marked an inline comment as not done. mgorny added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1667 +addr_t page; +if (llvm::to_integer(x, page)) dirty_page_list.push_back(page); mgorny wrote: > teemperor wrote: > > Base 16 arg is lost here. > Good catch, thanks. Actually, scratch that. The values here include `0x` prefix. and apparently `llvm::to_integer()` handles that correctly only if base isn't specified. Though it might be reasonable to fix `llvm::to_integer()` to allow matching prefix, I guess. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110472/new/ https://reviews.llvm.org/D110472 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based split()
mgorny created this revision.
mgorny added reviewers: labath, teemperor, krytarowski, JDevlieghere, emaste,
lattner, rriddle.
Herald added subscribers: dexonsmith, hiraditya.
mgorny requested review of this revision.
Herald added a project: LLVM.
Add a llvm::split() implementation that can be used via range-for loop,
e.g.:
for (StringRef x : llvm::split("foo,bar,baz", ','))
...
The implementation uses an additional SplittingIterator class that
uses StringRef::split() internally.
https://reviews.llvm.org/D110496
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
llvm/include/llvm/ADT/StringExtras.h
llvm/lib/IR/DataLayout.cpp
llvm/unittests/ADT/StringExtrasTest.cpp
Index: llvm/unittests/ADT/StringExtrasTest.cpp
===
--- llvm/unittests/ADT/StringExtrasTest.cpp
+++ llvm/unittests/ADT/StringExtrasTest.cpp
@@ -274,3 +274,35 @@
EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 10), "-1");
EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 16), "-1");
}
+
+TEST(StringExtrasTest, splitStringRef) {
+ auto spl = split("foo<=>bar<=><=>baz", "<=>");
+ auto it = spl.begin();
+ auto end = spl.end();
+
+ ASSERT_NE(it, end);
+ EXPECT_EQ(*it, StringRef("foo"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("bar"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef(""));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("baz"));
+ ASSERT_EQ(++it, end);
+}
+
+TEST(StringExtrasTest, splitChar) {
+ auto spl = split("foo,bar,,baz", ',');
+ auto it = spl.begin();
+ auto end = spl.end();
+
+ ASSERT_NE(it, end);
+ EXPECT_EQ(*it, StringRef("foo"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("bar"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef(""));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("baz"));
+ ASSERT_EQ(++it, end);
+}
Index: llvm/lib/IR/DataLayout.cpp
===
--- llvm/lib/IR/DataLayout.cpp
+++ llvm/lib/IR/DataLayout.cpp
@@ -216,8 +216,8 @@
}
/// Checked version of split, to ensure mandatory subparts.
-static Error split(StringRef Str, char Separator,
- std::pair &Split) {
+static Error dataLayoutSplit(StringRef Str, char Separator,
+ std::pair &Split) {
assert(!Str.empty() && "parse error, string can't be empty here");
Split = Str.split(Separator);
if (Split.second.empty() && Split.first != Str)
@@ -260,12 +260,12 @@
while (!Desc.empty()) {
// Split at '-'.
std::pair Split;
-if (Error Err = split(Desc, '-', Split))
+if (Error Err = dataLayoutSplit(Desc, '-', Split))
return Err;
Desc = Split.second;
// Split at ':'.
-if (Error Err = split(Split.first, ':', Split))
+if (Error Err = dataLayoutSplit(Split.first, ':', Split))
return Err;
// Aliases used below.
@@ -274,7 +274,7 @@
if (Tok == "ni") {
do {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
Rest = Split.second;
unsigned AS;
@@ -315,7 +315,7 @@
if (Rest.empty())
return reportError(
"Missing size specification for pointer in datalayout string");
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
unsigned PointerMemSize;
if (Error Err = getIntInBytes(Tok, PointerMemSize))
@@ -327,7 +327,7 @@
if (Rest.empty())
return reportError(
"Missing alignment specification for pointer in datalayout string");
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
unsigned PointerABIAlign;
if (Error Err = getIntInBytes(Tok, PointerABIAlign))
@@ -342,7 +342,7 @@
// Preferred alignment.
unsigned PointerPrefAlign = PointerABIAlign;
if (!Rest.empty()) {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
if (Error Err = getIntInBytes(Tok, PointerPrefAlign))
return Err;
@@ -352,7 +352,7 @@
// Now read the index. It is the second optional parameter here.
if (!Rest.empty()) {
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
if (Error Err = getIntInBytes(Tok, IndexSize))
return Err;
@@ -393,7 +393,7 @@
if (Rest.empty())
return reportError(
"Missing alignment specification in datalayout string");
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSpl
[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based split()
mgorny added inline comments.
Comment at: llvm/lib/IR/DataLayout.cpp:219
/// Checked version of split, to ensure mandatory subparts.
-static Error split(StringRef Str, char Separator,
- std::pair &Split) {
I needed to rename this due to collision with `llvm::split` class.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110496/new/
https://reviews.llvm.org/D110496
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module
siger-young updated this revision to Diff 375100.
siger-young added a comment.
Fix typo in SBData test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
Files:
lldb/test/API/lua_api/TestComprehensive.lua
Index: lldb/test/API/lua_api/TestComprehensive.lua
===
--- lldb/test/API/lua_api/TestComprehensive.lua
+++ lldb/test/API/lua_api/TestComprehensive.lua
@@ -71,7 +71,7 @@
assertTrue(error:Success())
local data_le =
lldb.SBData.CreateDataFromUInt32Array(lldb.eByteOrderLittle, 1, {0xDEADBEEF})
local data_be = lldb.SBData.CreateDataFromUInt32Array(lldb.eByteOrderBig,
1, {0xDEADBEEF})
-assertTrue(data_le:GetUnsignedInt32(error, 0) == 0xDCADBEEF or
data_be:GetUnsignedInt32(error, 0) == 0xDCADBEEF)
+assertTrue(data_le:GetUnsignedInt32(error, 0) == 0xDEADBEEF or
data_be:GetUnsignedInt32(error, 0) == 0xDEADBEEF)
assertTrue(raw_data == "\xEF\xBE\xAD\xDE" or raw_data ==
"\xDE\xAD\xBE\xEF")
end
Index: lldb/test/API/lua_api/TestComprehensive.lua
===
--- lldb/test/API/lua_api/TestComprehensive.lua
+++ lldb/test/API/lua_api/TestComprehensive.lua
@@ -71,7 +71,7 @@
assertTrue(error:Success())
local data_le = lldb.SBData.CreateDataFromUInt32Array(lldb.eByteOrderLittle, 1, {0xDEADBEEF})
local data_be = lldb.SBData.CreateDataFromUInt32Array(lldb.eByteOrderBig, 1, {0xDEADBEEF})
-assertTrue(data_le:GetUnsignedInt32(error, 0) == 0xDCADBEEF or data_be:GetUnsignedInt32(error, 0) == 0xDCADBEEF)
+assertTrue(data_le:GetUnsignedInt32(error, 0) == 0xDEADBEEF or data_be:GetUnsignedInt32(error, 0) == 0xDEADBEEF)
assertTrue(raw_data == "\xEF\xBE\xAD\xDE" or raw_data == "\xDE\xAD\xBE\xEF")
end
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module
siger-young updated this revision to Diff 375103.
siger-young added a comment.
Rebase commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
Files:
lldb/CMakeLists.txt
lldb/bindings/lua/CMakeLists.txt
lldb/bindings/lua/lua-typemaps.swig
lldb/bindings/lua/lua-wrapper.swig
lldb/bindings/lua/lua.swig
lldb/source/API/liblldb-private.exports
lldb/source/API/liblldb.exports
lldb/test/API/lit.site.cfg.py.in
lldb/test/API/lldbtest.py
lldb/test/API/lua_api/Makefile
lldb/test/API/lua_api/TestBreakpointAPI.lua
lldb/test/API/lua_api/TestComprehensive.lua
lldb/test/API/lua_api/TestFileHandle.lua
lldb/test/API/lua_api/TestLuaAPI.py
lldb/test/API/lua_api/TestProcessAPI.lua
lldb/test/API/lua_api/lua_lldb_test.lua
lldb/test/API/lua_api/main.c
Index: lldb/test/API/lua_api/main.c
===
--- /dev/null
+++ lldb/test/API/lua_api/main.c
@@ -0,0 +1,35 @@
+#include
+
+void BFunction()
+{
+}
+
+void AFunction()
+{
+printf("I am a function.\n");
+}
+
+int main(int argc, const char *argv[])
+{
+int inited = 0xDEADBEEF;
+int sum = 0;
+if(argc > 1)
+{
+for(int i = 0; i < argc; i++)
+{
+puts(argv[i]);
+}
+if(argc > 2)
+{
+return argc;
+}
+}
+AFunction();
+for(int i = 1; i <= 100; i++)
+{
+BFunction();
+sum += i;
+}
+printf("sum = %d\n", sum);
+return 0;
+}
Index: lldb/test/API/lua_api/lua_lldb_test.lua
===
--- /dev/null
+++ lldb/test/API/lua_api/lua_lldb_test.lua
@@ -0,0 +1,107 @@
+-- Import all functions of luaunit
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+
+-- Make lldb available in global
+lldb = require('lldb')
+
+-- Global helper functions
+function read_file_non_empty_lines(f)
+local lines = {}
+while true do
+local line = f:read('*l')
+if not line then break end
+if line ~= '\n' then table.insert(lines, line) end
+end
+return lines
+end
+
+function split_lines(str)
+local lines = {}
+for line in str:gmatch("[^\r\n]+") do
+table.insert(lines, line)
+end
+return lines
+end
+
+function get_stopped_threads(process, reason)
+local threads = {}
+for i = 0, process:GetNumThreads() - 1 do
+local t = process:GetThreadAtIndex(i)
+if t:IsValid() and t:GetStopReason() == reason then
+table.insert(threads, t)
+end
+end
+return threads
+end
+
+function get_stopped_thread(process, reason)
+local threads = get_stopped_threads(process, reason)
+if #threads ~= 0 then return threads[1]
+else return nil end
+end
+
+-- Test helper
+
+local _M = {}
+local _m = {}
+
+local _mt = { __index = _m }
+
+function _M.create_test(name, exe, output, input)
+print('[lldb/lua] Doing test ' .. name)
+exe = exe or os.getenv('TEST_EXE')
+output = output or os.getenv('TEST_OUTPUT')
+input = input or os.getenv('TEST_INPUT')
+lldb.SBDebugger.Initialize()
+local debugger = lldb.SBDebugger.Create()
+-- Ensure that debugger is created
+assertNotNil(debugger)
+assertTrue(debugger:IsValid())
+
+debugger:SetAsync(false)
+
+local lua_language = debugger:GetScriptingLanguage('lua')
+assertNotNil(lua_language)
+debugger:SetScriptLanguage(lua_language)
+
+local test = setmetatable({
+output = output,
+input = input,
+name = name,
+exe = exe,
+debugger = debugger
+}, _mt)
+_G[name] = test
+return test
+end
+
+function _m:create_target(exe)
+local target
+if not exe then exe = self.exe end
+target = self.debugger:CreateTarget(exe)
+-- Ensure that target is created
+assertNotNil(target)
+assertTrue(target:IsValid())
+return target
+end
+
+function _m:handle_command(command, collect)
+if collect == nil then collect = true end
+if collect then
+local ret = lldb.SBCommandReturnObject()
+local interpreter = self.debugger:GetCommandInterpreter()
+assertTrue(interpreter:IsValid())
+interpreter:HandleCommand(command, ret)
+self.debugger:GetOutputFile():Flush()
+self.debugger:GetErrorFile():Flush()
+assertTrue(ret:Succeeded())
+return ret:GetOutput()
+else
+self.debugger:HandleCommand(command)
+self.debugger:GetOutputFile():Flush()
+self.debugger:GetErrorFile():Flush()
+end
+end
+
+return _M
Index: lldb/test/API/lua_api/TestProcessAPI.lua
===
--- /dev/null
+++ lldb/test/API/lua_api/TestProcessAPI.lua
@@ -0,0 +1,59 @@
+_T = require('lua_lldb_test').create_test('TestProcessAPI')
+
+function _T:TestProcessLaunchSimple()
+local target = self:create_target()
+local
[Lldb-commits] [PATCH] D108090: [lldb/lua] Supplement Lua bindings for lldb module
tammela added a comment.
Just one last thing and I think we are done!
Thanks for your work.
Comment at: lldb/test/API/lua_api/lua_lldb_test.lua:3
+EXPORT_ASSERT_TO_GLOBALS = true
+require('luaunit')
+
Could we not use an external dependency?
For instance in my setup it fails because it couldn't find this library.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108090/new/
https://reviews.llvm.org/D108090
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D110472: [lldb] [gdb-remote] Use llvm::StringRef.split() and llvm::to_integer()
mgorny marked an inline comment as done. mgorny added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1667 +addr_t page; +if (llvm::to_integer(x, page)) dirty_page_list.push_back(page); mgorny wrote: > mgorny wrote: > > teemperor wrote: > > > Base 16 arg is lost here. > > Good catch, thanks. > Actually, scratch that. The values here include `0x` prefix. and apparently > `llvm::to_integer()` handles that correctly only if base isn't specified. > > Though it might be reasonable to fix `llvm::to_integer()` to allow matching > prefix, I guess. I've pinged @labath about this and he suggested just stripping the `0x` prefix (and fixing the sender not to send it). I'll do the first part in this patch, and then look into updating the code and docs to rid of `0x` prefix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110472/new/ https://reviews.llvm.org/D110472 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e2f780f - [lldb] [gdb-remote] Use llvm::StringRef.split() and llvm::to_integer()
Author: Michał Górny
Date: 2021-09-26T21:23:26+02:00
New Revision: e2f780fba96c55b0dcb7aa3c4719110875b36dfb
URL:
https://github.com/llvm/llvm-project/commit/e2f780fba96c55b0dcb7aa3c4719110875b36dfb
DIFF:
https://github.com/llvm/llvm-project/commit/e2f780fba96c55b0dcb7aa3c4719110875b36dfb.diff
LOG: [lldb] [gdb-remote] Use llvm::StringRef.split() and llvm::to_integer()
Replace the uses of StringConvert combined with hand-rolled array
splitting with llvm::StringRef.split() and llvm::to_integer().
Differential Revision: https://reviews.llvm.org/D110472
Added:
Modified:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
Removed:
diff --git
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index bf4baf7b7a266..6d2a267f294c1 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -16,7 +16,6 @@
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Host/HostInfo.h"
-#include "lldb/Host/StringConvert.h"
#include "lldb/Host/XML.h"
#include "lldb/Symbol/Symbol.h"
#include "lldb/Target/MemoryRegionInfo.h"
@@ -1660,22 +1659,15 @@ Status
GDBRemoteCommunicationClient::GetMemoryRegionInfo(
error_extractor.GetHexByteString(error_string);
error.SetErrorString(error_string.c_str());
} else if (name.equals("dirty-pages")) {
+ llvm::SmallVector split_value;
std::vector dirty_page_list;
- std::string comma_sep_str = value.str();
- size_t comma_pos;
- addr_t page;
- while ((comma_pos = comma_sep_str.find(',')) != std::string::npos) {
-comma_sep_str[comma_pos] = '\0';
-page = StringConvert::ToUInt64(comma_sep_str.c_str(),
- LLDB_INVALID_ADDRESS, 16);
-if (page != LLDB_INVALID_ADDRESS)
+ value.split(split_value, ',');
+ for (llvm::StringRef x : split_value) {
+addr_t page;
+x.consume_front("0x");
+if (llvm::to_integer(x, page, 16))
dirty_page_list.push_back(page);
-comma_sep_str.erase(0, comma_pos + 1);
}
- page = StringConvert::ToUInt64(comma_sep_str.c_str(),
- LLDB_INVALID_ADDRESS, 16);
- if (page != LLDB_INVALID_ADDRESS)
-dirty_page_list.push_back(page);
region_info.SetDirtyPageList(dirty_page_list);
}
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index ad49f9ddad2cc..2c1b4fa319ffa 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -43,7 +43,6 @@
#include "lldb/Host/HostThread.h"
#include "lldb/Host/PosixApi.h"
#include "lldb/Host/PseudoTerminal.h"
-#include "lldb/Host/StringConvert.h"
#include "lldb/Host/ThreadLauncher.h"
#include "lldb/Host/XML.h"
#include "lldb/Interpreter/CommandInterpreter.h"
@@ -381,20 +380,16 @@ bool ProcessGDBRemote::ParsePythonTargetDefinition(
}
static size_t SplitCommaSeparatedRegisterNumberString(
-const llvm::StringRef &comma_separated_regiter_numbers,
+const llvm::StringRef &comma_separated_register_numbers,
std::vector ®nums, int base) {
regnums.clear();
- std::pair value_pair;
- value_pair.second = comma_separated_regiter_numbers;
- do {
-value_pair = value_pair.second.split(',');
-if (!value_pair.first.empty()) {
- uint32_t reg = StringConvert::ToUInt32(value_pair.first.str().c_str(),
- LLDB_INVALID_REGNUM, base);
- if (reg != LLDB_INVALID_REGNUM)
-regnums.push_back(reg);
-}
- } while (!value_pair.second.empty());
+ llvm::SmallVector split_string;
+ comma_separated_register_numbers.split(split_string, ',');
+ for (llvm::StringRef x : split_string) {
+uint32_t reg;
+if (llvm::to_integer(x, reg, base))
+ regnums.push_back(reg);
+ }
return regnums.size();
}
@@ -1459,21 +1454,16 @@ size_t
ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue(
return m_thread_ids.size();
}
-size_t
-ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(std::string &value)
{
+size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(
+llvm::StringRef value) {
m_thread_pcs.clear();
- size_t comma_pos;
- lldb::addr_t pc;
- while ((comma_pos = value.find(',')) != std::string::npos) {
-value[comma_pos] = '\0';
-pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVA
[Lldb-commits] [PATCH] D110472: [lldb] [gdb-remote] Use llvm::StringRef.split() and llvm::to_integer()
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe2f780fba96c: [lldb] [gdb-remote] Use
llvm::StringRef.split() and llvm::to_integer() (authored by mgorny).
Herald added a project: LLDB.
Changed prior to commit:
https://reviews.llvm.org/D110472?vs=375022&id=375125#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110472/new/
https://reviews.llvm.org/D110472
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -359,7 +359,7 @@
bool CalculateThreadStopInfo(ThreadGDBRemote *thread);
- size_t UpdateThreadPCsFromStopReplyThreadsValue(std::string &value);
+ size_t UpdateThreadPCsFromStopReplyThreadsValue(llvm::StringRef value);
size_t UpdateThreadIDsFromStopReplyThreadsValue(llvm::StringRef value);
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -43,7 +43,6 @@
#include "lldb/Host/HostThread.h"
#include "lldb/Host/PosixApi.h"
#include "lldb/Host/PseudoTerminal.h"
-#include "lldb/Host/StringConvert.h"
#include "lldb/Host/ThreadLauncher.h"
#include "lldb/Host/XML.h"
#include "lldb/Interpreter/CommandInterpreter.h"
@@ -381,20 +380,16 @@
}
static size_t SplitCommaSeparatedRegisterNumberString(
-const llvm::StringRef &comma_separated_regiter_numbers,
+const llvm::StringRef &comma_separated_register_numbers,
std::vector ®nums, int base) {
regnums.clear();
- std::pair value_pair;
- value_pair.second = comma_separated_regiter_numbers;
- do {
-value_pair = value_pair.second.split(',');
-if (!value_pair.first.empty()) {
- uint32_t reg = StringConvert::ToUInt32(value_pair.first.str().c_str(),
- LLDB_INVALID_REGNUM, base);
- if (reg != LLDB_INVALID_REGNUM)
-regnums.push_back(reg);
-}
- } while (!value_pair.second.empty());
+ llvm::SmallVector split_string;
+ comma_separated_register_numbers.split(split_string, ',');
+ for (llvm::StringRef x : split_string) {
+uint32_t reg;
+if (llvm::to_integer(x, reg, base))
+ regnums.push_back(reg);
+ }
return regnums.size();
}
@@ -1459,21 +1454,16 @@
return m_thread_ids.size();
}
-size_t
-ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(std::string &value) {
+size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue(
+llvm::StringRef value) {
m_thread_pcs.clear();
- size_t comma_pos;
- lldb::addr_t pc;
- while ((comma_pos = value.find(',')) != std::string::npos) {
-value[comma_pos] = '\0';
-pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
-if (pc != LLDB_INVALID_ADDRESS)
+ llvm::SmallVector split_value;
+ value.split(split_value, ',');
+ for (llvm::StringRef x : split_value) {
+lldb::addr_t pc;
+if (llvm::to_integer(x, pc, 16))
m_thread_pcs.push_back(pc);
-value.erase(0, comma_pos + 1);
}
- pc = StringConvert::ToUInt64(value.c_str(), LLDB_INVALID_ADDRESS, 16);
- if (pc != LLDB_INVALID_ADDRESS)
-m_thread_pcs.push_back(pc);
return m_thread_pcs.size();
}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -16,7 +16,6 @@
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Host/HostInfo.h"
-#include "lldb/Host/StringConvert.h"
#include "lldb/Host/XML.h"
#include "lldb/Symbol/Symbol.h"
#include "lldb/Target/MemoryRegionInfo.h"
@@ -1660,22 +1659,15 @@
error_extractor.GetHexByteString(error_string);
error.SetErrorString(error_string.c_str());
} else if (name.equals("dirty-pages")) {
+ llvm::SmallVector split_value;
std::vector dirty_page_list;
- std::string comma_sep_str = value.str();
- size_t comma_pos;
- addr_t page;
- while ((comma_pos = comma_sep_str.find(',')) != std::string::npos) {
-comma_sep_str[comma_pos] = '\0';
-page = StringConvert::ToUInt64(comma_sep_str.c_str(),
- LLDB_INVALID_ADDRESS, 16);
-if (page != LLDB_INVALID_ADDRESS)
+ value.split(split_value, ',');
+ for (llvm::StringRef x : split_value) {
[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based split()
mgorny updated this revision to Diff 375126.
mgorny added a comment.
Convert more LLDB gdb-remote calls.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110496/new/
https://reviews.llvm.org/D110496
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
llvm/include/llvm/ADT/StringExtras.h
llvm/lib/IR/DataLayout.cpp
llvm/unittests/ADT/StringExtrasTest.cpp
Index: llvm/unittests/ADT/StringExtrasTest.cpp
===
--- llvm/unittests/ADT/StringExtrasTest.cpp
+++ llvm/unittests/ADT/StringExtrasTest.cpp
@@ -274,3 +274,35 @@
EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 10), "-1");
EXPECT_EQ(toString(APSInt(APInt(8, 255), isUnsigned), 16), "-1");
}
+
+TEST(StringExtrasTest, splitStringRef) {
+ auto spl = split("foo<=>bar<=><=>baz", "<=>");
+ auto it = spl.begin();
+ auto end = spl.end();
+
+ ASSERT_NE(it, end);
+ EXPECT_EQ(*it, StringRef("foo"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("bar"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef(""));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("baz"));
+ ASSERT_EQ(++it, end);
+}
+
+TEST(StringExtrasTest, splitChar) {
+ auto spl = split("foo,bar,,baz", ',');
+ auto it = spl.begin();
+ auto end = spl.end();
+
+ ASSERT_NE(it, end);
+ EXPECT_EQ(*it, StringRef("foo"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("bar"));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef(""));
+ ASSERT_NE(++it, end);
+ EXPECT_EQ(*it, StringRef("baz"));
+ ASSERT_EQ(++it, end);
+}
Index: llvm/lib/IR/DataLayout.cpp
===
--- llvm/lib/IR/DataLayout.cpp
+++ llvm/lib/IR/DataLayout.cpp
@@ -216,8 +216,8 @@
}
/// Checked version of split, to ensure mandatory subparts.
-static Error split(StringRef Str, char Separator,
- std::pair &Split) {
+static Error dataLayoutSplit(StringRef Str, char Separator,
+ std::pair &Split) {
assert(!Str.empty() && "parse error, string can't be empty here");
Split = Str.split(Separator);
if (Split.second.empty() && Split.first != Str)
@@ -260,12 +260,12 @@
while (!Desc.empty()) {
// Split at '-'.
std::pair Split;
-if (Error Err = split(Desc, '-', Split))
+if (Error Err = dataLayoutSplit(Desc, '-', Split))
return Err;
Desc = Split.second;
// Split at ':'.
-if (Error Err = split(Split.first, ':', Split))
+if (Error Err = dataLayoutSplit(Split.first, ':', Split))
return Err;
// Aliases used below.
@@ -274,7 +274,7 @@
if (Tok == "ni") {
do {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
Rest = Split.second;
unsigned AS;
@@ -315,7 +315,7 @@
if (Rest.empty())
return reportError(
"Missing size specification for pointer in datalayout string");
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
unsigned PointerMemSize;
if (Error Err = getIntInBytes(Tok, PointerMemSize))
@@ -327,7 +327,7 @@
if (Rest.empty())
return reportError(
"Missing alignment specification for pointer in datalayout string");
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
unsigned PointerABIAlign;
if (Error Err = getIntInBytes(Tok, PointerABIAlign))
@@ -342,7 +342,7 @@
// Preferred alignment.
unsigned PointerPrefAlign = PointerABIAlign;
if (!Rest.empty()) {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
if (Error Err = getIntInBytes(Tok, PointerPrefAlign))
return Err;
@@ -352,7 +352,7 @@
// Now read the index. It is the second optional parameter here.
if (!Rest.empty()) {
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
if (Error Err = getIntInBytes(Tok, IndexSize))
return Err;
@@ -393,7 +393,7 @@
if (Rest.empty())
return reportError(
"Missing alignment specification in datalayout string");
- if (Error Err = split(Rest, ':', Split))
+ if (Error Err = dataLayoutSplit(Rest, ':', Split))
return Err;
unsigned ABIAlign;
if (Error Err = getIntInBytes(Tok, ABIAlign))
@@ -410,7 +410,7 @@
// Preferred alignment.
unsigned PrefAlign = ABIAlign;
if (!Rest.empty()) {
-if (Error Err = split(Rest, ':', Split))
+if (Error Err = dataLay
[Lldb-commits] [PATCH] D110510: [lldb] Remove "0x" prefix from hex values in dirty-pages
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, JDevlieghere, teemperor.
mgorny requested review of this revision.
Remove the redudant "0x" prefix in the "dirty-pages" key of
qMemoryRegionInfo packet. The client accepts hex values both with
and without the prefix.
https://reviews.llvm.org/D110510
Files:
lldb/test/API/functionalities/gdb_remote_client/TestMemoryRegionDirtyPages.py
lldb/tools/debugserver/source/RNBRemote.cpp
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -4306,7 +4306,7 @@
if (!first)
ostrm << ",";
first = false;
-ostrm << "0x" << std::hex << addr;
+ostrm << std::hex << addr;
}
}
ostrm << ";";
Index:
lldb/test/API/functionalities/gdb_remote_client/TestMemoryRegionDirtyPages.py
===
---
lldb/test/API/functionalities/gdb_remote_client/TestMemoryRegionDirtyPages.py
+++
lldb/test/API/functionalities/gdb_remote_client/TestMemoryRegionDirtyPages.py
@@ -19,9 +19,9 @@
if addr == 0x1:
return
"start:1;size:4000;permissions:rx;dirty-pages:;"
if addr == 0x14000:
-return
"start:14000;size:4000;permissions:r;dirty-pages:0x14000;"
+return
"start:14000;size:4000;permissions:r;dirty-pages:14000;"
if addr == 0x1000a2000:
-return
"start:1000a2000;size:5000;permissions:r;dirty-pages:0x1000a2000,0x1000a3000,0x1000a4000,0x1000a5000,0x1000a6000;"
+return
"start:1000a2000;size:5000;permissions:r;dirty-pages:1000a2000,1000a3000,1000a4000,1000a5000,1000a6000;"
self.server.responder = MyResponder()
target = self.dbg.CreateTarget('')
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -4306,7 +4306,7 @@
if (!first)
ostrm << ",";
first = false;
-ostrm << "0x" << std::hex << addr;
+ostrm << std::hex << addr;
}
}
ostrm << ";";
Index: lldb/test/API/functionalities/gdb_remote_client/TestMemoryRegionDirtyPages.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestMemoryRegionDirtyPages.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestMemoryRegionDirtyPages.py
@@ -19,9 +19,9 @@
if addr == 0x1:
return "start:1;size:4000;permissions:rx;dirty-pages:;"
if addr == 0x14000:
-return "start:14000;size:4000;permissions:r;dirty-pages:0x14000;"
+return "start:14000;size:4000;permissions:r;dirty-pages:14000;"
if addr == 0x1000a2000:
-return "start:1000a2000;size:5000;permissions:r;dirty-pages:0x1000a2000,0x1000a3000,0x1000a4000,0x1000a5000,0x1000a6000;"
+return "start:1000a2000;size:5000;permissions:r;dirty-pages:1000a2000,1000a3000,1000a4000,1000a5000,1000a6000;"
self.server.responder = MyResponder()
target = self.dbg.CreateTarget('')
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D110496: [llvm] [ADT] Add a range/iterator-based split()
lattner accepted this revision. lattner added a comment. This revision is now accepted and ready to land. This is really nice! Please fix the clang-tidy casing issues, but otherwise LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110496/new/ https://reviews.llvm.org/D110496 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
