[Lldb-commits] [PATCH] D107665: [lldb] [Commands] Fix reporting errors in 'platform file read/write'
mgorny updated this revision to Diff 365407.
mgorny added a comment.
Fix integer type mismatches.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107665/new/
https://reviews.llvm.org/D107665
Files:
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index:
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -60,11 +60,12 @@
self.match("platform file open /some/file.txt -v 0755",
[r"error: Invalid argument"],
error=True)
-# TODO: fix the commands to fail on unsuccessful result
self.match("platform file read 16 -o 11 -c 13",
- [r"Return = -1\nData = \"\""])
+ [r"error: Invalid argument"],
+ error=True)
self.match("platform file write 16 -o 11 -d teststring",
- [r"Return = -1"])
+ [r"error: Invalid argument"],
+ error=True)
self.match("platform file close 16",
[r"error: Invalid argument"],
error=True)
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -588,11 +588,15 @@
}
std::string buffer(m_options.m_count, 0);
Status error;
- uint32_t retcode = platform_sp->ReadFile(
+ uint64_t retcode = platform_sp->ReadFile(
fd, m_options.m_offset, &buffer[0], m_options.m_count, error);
- result.AppendMessageWithFormat("Return = %d\n", retcode);
- result.AppendMessageWithFormat("Data = \"%s\"\n", buffer.c_str());
- result.SetStatus(eReturnStatusSuccessFinishResult);
+ if (retcode != UINT64_MAX) {
+result.AppendMessageWithFormat("Return = %" PRIu64 "\n", retcode);
+result.AppendMessageWithFormat("Data = \"%s\"\n", buffer.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+ } else {
+result.AppendError(error.AsCString());
+ }
} else {
result.AppendError("no platform currently selected\n");
}
@@ -677,11 +681,15 @@
cmd_line);
return result.Succeeded();
}
- uint32_t retcode =
+ uint64_t retcode =
platform_sp->WriteFile(fd, m_options.m_offset, &m_options.m_data[0],
m_options.m_data.size(), error);
- result.AppendMessageWithFormat("Return = %d\n", retcode);
- result.SetStatus(eReturnStatusSuccessFinishResult);
+ if (retcode != UINT64_MAX) {
+result.AppendMessageWithFormat("Return = %" PRIu64 "\n", retcode);
+result.SetStatus(eReturnStatusSuccessFinishResult);
+ } else {
+result.AppendError(error.AsCString());
+ }
} else {
result.AppendError("no platform currently selected\n");
}
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -60,11 +60,12 @@
self.match("platform file open /some/file.txt -v 0755",
[r"error: Invalid argument"],
error=True)
-# TODO: fix the commands to fail on unsuccessful result
self.match("platform file read 16 -o 11 -c 13",
- [r"Return = -1\nData = \"\""])
+ [r"error: Invalid argument"],
+ error=True)
self.match("platform file write 16 -o 11 -d teststring",
- [r"Return = -1"])
+ [r"error: Invalid argument"],
+ error=True)
self.match("platform file close 16",
[r"error: Invalid argument"],
error=True)
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -588,11 +588,15 @@
}
std::string buffer(m_options.m_count, 0);
Status error;
- uint32_t retcode = platform_sp->ReadFile(
+ uint64_t retcode = platform_sp->ReadFile(
fd, m_options.m_offset, &buffer[0], m_options.m_count, error);
- result.AppendMessageWithFormat("Return = %d\n", retcode);
-
[Lldb-commits] [PATCH] D107780: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()
mgorny updated this revision to Diff 365408.
mgorny added a comment.
Fix integer-type related warnings.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107780/new/
https://reviews.llvm.org/D107780
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -77,3 +77,58 @@
])
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size(self):
+"""Test 'platform get-size'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1000"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+ [r"File size of /some/file\.txt \(remote\): 4096"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size_fallback(self):
+"""Test 'platform get-size fallback to vFile:fstat'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 28 * "\0" + "\0\0\0\0\0\1\2\3" + 28 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+ [r"File size of /some/file\.txt \(remote\): 66051"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -391,6 +391,8 @@
bool CloseFile(lldb::user_id_t fd, Status &error);
+ llvm::Optional FStat(lldb::user_id_t fd);
+
lldb::user_id_t GetFileSize(const FileSpec &file_spec);
void AutoCompleteDiskFileOrDirectory(CompletionRequest &request,
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
@@ -3035,6 +3035,46 @@
return false;
}
+llvm::Optional
+GDBRemoteCommunicationClient::FStat(lldb::user_id_t fd) {
+ lldb_private::StreamString stream;
+ stream.Printf("vFile:fstat:%" PRIx64, fd);
+ StringExtractorGDBRemote response;
+ if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
+ PacketResult::Success) {
+if (response.GetChar() != 'F')
+ return llvm::None;
+int64_t size = response.GetS64(-1, 16);
+if (size > 0 && response.GetChar() == ';') {
+ std::string buffer;
+ if (response.GetEscapedBinaryData(buffer)) {
+DataExtractor extractor{buffer.data(), buffer.size(),
+lldb::eByteOrderBig, sizeof(void *)};
+lldb::offset_t offset = 0;
+GDBRemoteFStatData out;
+
+out.gdb_st_dev = extractor.GetU32(&offset);
+out.gdb_st_ino = extractor.GetU32(&offset);
+out.gdb_st_mode = extractor.GetU32(&offset);
+out.gdb_st_nlink = extractor.GetU32(&offset);
+out.gdb
[Lldb-commits] [PATCH] D107809: [lldb] Add new commands and tests for getting file perms & exists
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jasonmolenda, JDevlieghere.
mgorny requested review of this revision.
Add two new commands 'platform get-file-permissions' and 'platform
file-exists' for the respective bits of LLDB protocol. Add tests for
them. Fix error handling in GetFilePermissions().
https://reviews.llvm.org/D107809
Files:
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -132,3 +132,72 @@
])
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_permissions(self):
+"""Test 'platform get-permissions'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1a4"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-permissions /some/file.txt",
+ [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists(self):
+"""Test 'platform file-exists'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,1"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists(self):
+"""Test 'platform file-exists' with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,0"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -658,9 +658,10 @@
std::error_code ec;
const uint32_t mode = FileSystem::Instance().GetPermissions(file_spec, ec);
StreamString response;
-response.Printf("F%x", mode);
-if (mode == 0 || ec)
- response.Printf(",%x", (int)Status(ec).GetError());
+if (mode != llvm::sys::fs::perms_not_known)
+ response.Printf("F%x", mode);
+else
+ response.Printf("F-1,%x", (int)Status(ec).GetError());
return SendPacketNoLock(response.GetString());
}
return SendErrorResponse(23);
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -926,6 +926,141 @@
}
};
+// "platform get-permissions remote-file-path"
+class CommandObjectPlatformGetPermissions : public CommandObjectParsed {
+public:
+ CommandObjectPlatformGetPermissions(CommandInterpreter &interpreter)
+ : CommandObjectParsed(interpreter,
[Lldb-commits] [PATCH] D107780: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()
mgorny updated this revision to Diff 365414.
mgorny added a comment.
Add a helper `Stat` method to take care of opening and closing the file. Add a
cache variable for whether `vFile:size` is supported.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107780/new/
https://reviews.llvm.org/D107780
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -77,3 +77,58 @@
])
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size(self):
+"""Test 'platform get-size'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1000"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+ [r"File size of /some/file\.txt \(remote\): 4096"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size_fallback(self):
+"""Test 'platform get-size fallback to vFile:fstat'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 28 * "\0" + "\0\0\0\0\0\1\2\3" + 28 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+ [r"File size of /some/file\.txt \(remote\): 66051"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -391,6 +391,10 @@
bool CloseFile(lldb::user_id_t fd, Status &error);
+ llvm::Optional FStat(lldb::user_id_t fd);
+
+ llvm::Optional Stat(const FileSpec &file_spec);
+
lldb::user_id_t GetFileSize(const FileSpec &file_spec);
void AutoCompleteDiskFileOrDirectory(CompletionRequest &request,
@@ -593,7 +597,7 @@
m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
- m_supports_jModulesInfo : 1;
+ m_supports_jModulesInfo : 1, m_supports_vFileSize : 1;
/// Current gdb remote protocol process identifier for all other operations
lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
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
@@ -65,6 +65,7 @@
m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
+ m_supports_vFileSize(true),
m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
@@ -
[Lldb-commits] [PATCH] D107809: [lldb] Add new commands and tests for getting file perms & exists
mgorny updated this revision to Diff 365416.
mgorny added a comment.
Fix colliding test name.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107809/new/
https://reviews.llvm.org/D107809
Files:
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -132,3 +132,72 @@
])
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_permissions(self):
+"""Test 'platform get-permissions'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1a4"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-permissions /some/file.txt",
+ [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists(self):
+"""Test 'platform file-exists'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,1"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_not(self):
+"""Test 'platform file-exists' with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,0"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -658,9 +658,10 @@
std::error_code ec;
const uint32_t mode = FileSystem::Instance().GetPermissions(file_spec, ec);
StreamString response;
-response.Printf("F%x", mode);
-if (mode == 0 || ec)
- response.Printf(",%x", (int)Status(ec).GetError());
+if (mode != llvm::sys::fs::perms_not_known)
+ response.Printf("F%x", mode);
+else
+ response.Printf("F-1,%x", (int)Status(ec).GetError());
return SendPacketNoLock(response.GetString());
}
return SendErrorResponse(23);
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -926,6 +926,141 @@
}
};
+// "platform get-permissions remote-file-path"
+class CommandObjectPlatformGetPermissions : public CommandObjectParsed {
+public:
+ CommandObjectPlatformGetPermissions(CommandInterpreter &interpreter)
+ : CommandObjectParsed(interpreter, "platform get-permissions",
+"Get the file permission bits from the remote end.",
+"platform get-permissions ", 0) {
+
[Lldb-commits] [PATCH] D107811: [lldb] [gdb-remote] Add fallbacks for vFile:mode and vFile:exists
mgorny created this revision.
mgorny added reviewers: krytarowski, emaste, labath, jasonmolenda, JDevlieghere.
mgorny requested review of this revision.
Add a GDB-compatible fallback to vFile:fstat for vFile:mode, and to
vFile:open for vFile:exists.
https://reviews.llvm.org/D107811
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -156,6 +156,38 @@
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+def test_file_permissions_fallback(self):
+"""Test 'platform get-permissions' fallback to fstat"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 8 * "\0\0\1\xA4" + 4 * "\0" + 52 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-permissions /some/file.txt",
+ [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
def test_file_exists(self):
"""Test 'platform file-exists'"""
@@ -201,3 +233,58 @@
])
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_fallback(self):
+"""Test 'platform file-exists' fallback to open"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_not_fallback(self):
+"""Test 'platform file-exists' fallback to open with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F-1,2"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/
[Lldb-commits] [PATCH] D101361: [LLDB] Support AArch64/Linux watchpoint on tagged addresses
andrew added a comment. FreeBSD doesn't currently support TBI. I'm trying to decide if it will be enabled everywhere, or just when needed (e.g. for HWASAN) due to how it interacts with PAC. I have a patch in review in the FreeBSD phabricator to add PAC support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101361/new/ https://reviews.llvm.org/D101361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D102757: [lldb] Remove non address bits when looking up memory regions
DavidSpickett updated this revision to Diff 365435.
DavidSpickett added a comment.
Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102757/new/
https://reviews.llvm.org/D102757
Files:
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.h
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.h
lldb/source/Target/Process.cpp
lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
lldb/test/API/linux/aarch64/tagged_memory_region/main.c
Index: lldb/test/API/linux/aarch64/tagged_memory_region/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_region/main.c
@@ -0,0 +1,19 @@
+#include
+#include
+#include
+#include
+
+int main(int argc, char const *argv[]) {
+ void *the_page = mmap(0, sysconf(_SC_PAGESIZE), PROT_READ | PROT_EXEC,
+MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (the_page == MAP_FAILED)
+return 1;
+
+ // Put something in the top byte
+ the_page = (void *)((size_t)the_page | ((size_t)0x34 << 56));
+
+ // Then sign it
+ __asm__ __volatile__("pacdzb %0" : "=r"(the_page) : "r"(the_page));
+
+ return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -0,0 +1,44 @@
+"""
+Test that "memory region" lookup removes the top byte and pointer
+authentication signatures of the address given.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTaggedMemoryRegionTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_mte_regions(self):
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Set break point at this line.'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# Despite the non address bits we should find a region
+self.expect("memory region the_page", patterns=[
+"\[0x[0-9A-Fa-f]+-0x[0-9A-Fa-f]+\) r-x"])
Index: lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_region/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -march=armv8.3-a
+
+include Makefile.rules
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -5864,6 +5864,13 @@
return retval;
}
+Status Process::GetMemoryRegionInfo(lldb::addr_t load_addr,
+MemoryRegionInfo &range_info) {
+ if (auto abi = GetABI())
+load_addr = abi->FixDataAddress(load_addr);
+ return DoGetMemoryRegionInfo(load_addr, range_info);
+}
+
Status
Process::GetMemoryRegions(lldb_private::MemoryRegionInfos ®ion_list) {
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -86,9 +86,6 @@
ArchSpec GetArchitecture();
- Status GetMemoryRegionInfo(lldb::addr_t load_addr,
- MemoryRegionInfo &range_info) override;
-
Status
GetMemoryRegions(lldb_pri
[Lldb-commits] [PATCH] D103626: [lldb][AArch64] Remove non address bits from memory read arguments
DavidSpickett updated this revision to Diff 365436.
DavidSpickett added a comment.
Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103626/new/
https://reviews.llvm.org/D103626
Files:
lldb/source/Commands/CommandObjectMemory.cpp
lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
lldb/test/API/linux/aarch64/tagged_memory_read/main.c
Index: lldb/test/API/linux/aarch64/tagged_memory_read/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_read/main.c
@@ -0,0 +1,18 @@
+#include
+
+static char *set_non_address_bits(char *ptr, size_t tag) {
+ // Set top byte tag
+ ptr = (char *)((size_t)ptr | (tag << 56));
+ // Sign it
+ __asm__ __volatile__("pacdzb %0" : "=r"(ptr) : "r"(ptr));
+ return ptr;
+}
+
+int main(int argc, char const *argv[]) {
+ char buf[32];
+
+ char *ptr1 = set_non_address_bits(buf, 0x34);
+ char *ptr2 = set_non_address_bits(buf, 0x56);
+
+ return 0; // Set break point at this line.
+}
Index: lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_read/TestAArch64LinuxTaggedMemoryRead.py
@@ -0,0 +1,55 @@
+"""
+Test that "memory read" removes the top byte and pointer
+authentication signature of addresses before using them.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTaggedMemoryReadTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_mte_tagged_memory_read(self):
+if not self.isAArch64PAuth():
+self.skipTest('Target must support pointer authentication.')
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(self, "main.c",
+line_number('main.c', '// Set break point at this line.'),
+num_expected_locations=1)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# If we do not remove non address bits, this can fail in two ways.
+# 1. We attempt to read much more than 16 bytes, probably more than
+#the default 1024 byte read size. (which is an error)
+# 2. We error because end address is < start address. Likely
+#because end's tag is < the beginning's tag.
+#
+# Each time we check that the printed line addresses do not include
+# either of the tags we set.
+tagged_addr_pattern = "0x(34|46)[0-9A-Fa-f]{14}:.*"
+self.expect("memory read ptr1 ptr2+16", patterns=[tagged_addr_pattern], matching=False)
+# Check that the stored previous end address is stripped
+self.expect("memory read", patterns=[tagged_addr_pattern], matching=False)
+self.expect("memory read ptr2 ptr1+16", patterns=[tagged_addr_pattern], matching=False)
+self.expect("memory read", patterns=[tagged_addr_pattern], matching=False)
Index: lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tagged_memory_read/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -march=armv8.3-a
+
+include Makefile.rules
Index: lldb/source/Commands/CommandObjectMemory.cpp
===
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -23,6 +23,7 @@
#include "lldb/Interpreter/Options.h"
#include "lldb/Symbol/SymbolFile.h"
#include "lldb/Symbol/TypeList.h"
+#include "lldb/Target/ABI.h"
#include "lldb/Target/Language.h"
#include "lldb/Target/MemoryHistory.h"
#include "lldb/Target/MemoryRegionInfo.h"
@@ -590,9 +591,16 @@
return false;
}
+ABISP abi = m_exe_ctx.GetProcessPtr()->GetABI();
+if (abi)
+ addr = abi->FixDataAddress(addr);
+
if (argc == 2) {
lldb::addr_t end_addr = OptionArgParser::ToAddress(
&m_exe_ctx, command[1].ref(), LLDB_INVALID_ADDRESS, nullptr);
+ if (end_addr != LLDB_INVALID_ADDRESS && abi)
+end_addr = abi->FixDataAddress(end_addr);
+
if (end_addr == LLDB_INVALID_ADDRESS) {
result.AppendError("invalid end address expression.");
result.AppendError(error.AsCString
[Lldb-commits] [PATCH] D107776: [lldb] Add a test for potentially conflicting names for the Objective-C class update utility expression
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG499489064b7a: [lldb] Add a test for potentially conflicting
names for the Objective-C class… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107776/new/
https://reviews.llvm.org/D107776
Files:
lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
Index: lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
===
--- /dev/null
+++ lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
@@ -0,0 +1,59 @@
+#import
+
+// Observable side effect that is changed when one of our trap functions is
+// called. This should always retain its initial value in a successful test run.
+const char *called_function = "none";
+
+// Below several trap functions are declared in different scopes that should
+// never be called even though they share the name of some of the utility
+// functions that LLDB has to call when updating the Objective-C class list
+// (i.e. 'free' and 'objc_copyRealizedClassList_nolock').
+// All functions just indicate that they got called by setting 'called_function'
+// to their own name.
+
+namespace N {
+void free(void *) { called_function = "N::free"; }
+void objc_copyRealizedClassList_nolock(unsigned int *) {
+ called_function = "N::objc_copyRealizedClassList_nolock";
+}
+}
+
+struct Context {
+ void free(void *) { called_function = "Context::free"; }
+ void objc_copyRealizedClassList_nolock(unsigned int *) {
+called_function = "Context::objc_copyRealizedClassList_nolock";
+ }
+};
+
+@interface ObjCContext : NSObject {
+}
+- (void)free:(void *)p;
+- (void)objc_copyRealizedClassList_nolock:(unsigned int *)outCount;
+@end
+
+@implementation ObjCContext
+- (void)free:(void *)p {
+ called_function = "ObjCContext::free";
+}
+
+- (void)objc_copyRealizedClassList_nolock:(unsigned int *)outCount {
+ called_function = "ObjCContext::objc_copyRealizedClassList_nolock";
+}
+@end
+
+int main(int argc, char **argv) {
+ id str = @"str";
+ // Make sure all our conflicting functions/methods are emitted. The condition
+ // is never executed in the test as the process is launched without args.
+ if (argc == 1234) {
+Context o;
+o.free(nullptr);
+o.objc_copyRealizedClassList_nolock(nullptr);
+N::free(nullptr);
+N::objc_copyRealizedClassList_nolock(nullptr);
+ObjCContext *obj = [[ObjCContext alloc] init];
+[obj free:nullptr];
+[obj objc_copyRealizedClassList_nolock:nullptr];
+ }
+ return 0; // break here
+}
Index: lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
===
--- /dev/null
+++ lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
@@ -0,0 +1,42 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+"""
+Tests that running the utility expression that retrieves the Objective-C
+class list works even when user-code contains functions with apparently
+conflicting identifiers (e.g. 'free') but that are not in the global
+scope.
+
+This is *not* supposed to test what happens when there are actual
+conflicts such as when a user somehow defined their own '::free'
+function.
+"""
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.mm"))
+
+# First check our side effect variable is in its initial state.
+self.expect_expr("called_function", result_summary='"none"')
+
+# Get the (dynamic) type of our 'id' variable so that our Objective-C
+# runtime information is updated.
+str_val = self.expect_expr("str")
+dyn_val = str_val.GetDynamicValue(lldb.eDynamicCanRunTarget)
+dyn_type = dyn_val.GetTypeName()
+
+# Check our side effect variable which should still be in its initial
+# state if none of our trap functions were called.
+# If this is failing, then LLDB called one of the trap functions.
+self.expect_expr("called_function", result_summary='"none"')
+
+# Double check that our dynamic type is correct. This is done last
+# as the assert message from above is the m
[Lldb-commits] [lldb] 4994890 - [lldb] Add a test for potentially conflicting names for the Objective-C class update utility expression
Author: Raphael Isemann
Date: 2021-08-10T14:42:06+02:00
New Revision: 499489064b7a4519f7c566e76077821ae1f3446d
URL:
https://github.com/llvm/llvm-project/commit/499489064b7a4519f7c566e76077821ae1f3446d
DIFF:
https://github.com/llvm/llvm-project/commit/499489064b7a4519f7c566e76077821ae1f3446d.diff
LOG: [lldb] Add a test for potentially conflicting names for the Objective-C
class update utility expression
We recently had an issue where a user declared a `Class::free` function which
then got picked up by accident by the expression evaluator when calling
`::free`. This was due to a too lax filter in the DWARFIndex (which was fixed by
https://reviews.llvm.org/D73191 ). This broke the Objective-C utility expression
that is trying to update the Objective-C class list (which is calling `:;free`).
This adds a regression test for situations where we have a bunch of functions
defined that share the name of the global functions that this utility function
calls. None of them are actually conflicting with the global functions we are
trying to call (they are all in namespaces, objects or classes).
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D107776
Added:
lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
Modified:
Removed:
diff --git
a/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
new file mode 100644
index 0..3af75c304c35d
--- /dev/null
+++
b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/Makefile
@@ -0,0 +1,4 @@
+OBJCXX_SOURCES := main.mm
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules
diff --git
a/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
new file mode 100644
index 0..cac65089a28f1
--- /dev/null
+++
b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/TestObjCConflictingNamesForClassUpdateExpr.py
@@ -0,0 +1,42 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test(self):
+"""
+Tests that running the utility expression that retrieves the
Objective-C
+class list works even when user-code contains functions with apparently
+conflicting identifiers (e.g. 'free') but that are not in the global
+scope.
+
+This is *not* supposed to test what happens when there are actual
+conflicts such as when a user somehow defined their own '::free'
+function.
+"""
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
lldb.SBFileSpec("main.mm"))
+
+# First check our side effect variable is in its initial state.
+self.expect_expr("called_function", result_summary='"none"')
+
+# Get the (dynamic) type of our 'id' variable so that our Objective-C
+# runtime information is updated.
+str_val = self.expect_expr("str")
+dyn_val = str_val.GetDynamicValue(lldb.eDynamicCanRunTarget)
+dyn_type = dyn_val.GetTypeName()
+
+# Check our side effect variable which should still be in its initial
+# state if none of our trap functions were called.
+# If this is failing, then LLDB called one of the trap functions.
+self.expect_expr("called_function", result_summary='"none"')
+
+# Double check that our dynamic type is correct. This is done last
+# as the assert message from above is the more descriptive one (it
+# contains the unintentionally called function).
+self.assertEqual(dyn_type, "__NSCFConstantString *")
diff --git
a/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
new file mode 100644
index 0..0ec825038c228
--- /dev/null
+++
b/lldb/test/API/lang/objcxx/conflicting-names-class-update-utility-expr/main.mm
@@ -0,0 +1,59 @@
+#import
+
+// Observable side effect that is changed when one of our trap functions is
+// called. This should always retain its initial value in a successful test
run.
+const char *called_function = "none";
+
+// Below several trap functions are declared in
diff erent scopes that should
+// never be called even though they share the name of some o
[Lldb-commits] [PATCH] D107821: [lldb] [gdb-server] Add tests for more vFile packets
mgorny created this revision.
mgorny added reviewers: krytarowski, labath, emaste, jasonmolenda, JDevlieghere.
mgorny requested review of this revision.
https://reviews.llvm.org/D107821
Files:
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -100,6 +100,80 @@
True)
self.expect_gdbremote_sequence()
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_size(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+test_data = b"test data of some length"
+temp_file.write(test_data)
+temp_file.flush()
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:size:%s#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)+#[0-9a-fA-F]{2}$",
+ "capture": {1: "size"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), len(test_data))
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_mode(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+test_mode = 0o751
+os.chmod(temp_file.fileno(), test_mode)
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:mode:%s#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)+#[0-9a-fA-F]{2}$",
+ "capture": {1: "mode"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["mode"], 16), test_mode)
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_exists(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:exists:%s#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ "send packet: $F,1#00"],
+True)
+self.expect_gdbremote_sequence()
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_exists_not(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.TemporaryDirectory() as temp_dir:
+test_path = os.path.join(temp_dir, "nonexist")
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:exists:%s#00" % (
+binascii.b2a_hex(test_path.encode()).decode(),),
+ "send packet: $F,0#00"],
+True)
+self.expect_gdbremote_sequence()
+
def expect_error(self):
self.test_sequence.add_log_lines(
[{"direction": "send",
Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -100,6 +100,80 @@
True)
self.expect_gdbremote_sequence()
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_size(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+test_data = b"test data of some length"
+temp_file.write(test_data)
+temp_file.flush()
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:size:%s#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)+#[0-9a-fA-F]{2}$",
+ "capture": {1: "size"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), len(test_data))
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_mode(self):
+
[Lldb-commits] [lldb] 57bf5c8 - [lldb] Add a test for user-defined objc_copyRealizedClassList_nolock
Author: Raphael Isemann
Date: 2021-08-10T14:54:42+02:00
New Revision: 57bf5c86591a1ff664f99d47e9f6a03a280f7a02
URL:
https://github.com/llvm/llvm-project/commit/57bf5c86591a1ff664f99d47e9f6a03a280f7a02
DIFF:
https://github.com/llvm/llvm-project/commit/57bf5c86591a1ff664f99d47e9f6a03a280f7a02.diff
LOG: [lldb] Add a test for user-defined objc_copyRealizedClassList_nolock
LLDB evaluates some utility expression to update the Objective-C class list that
ends up calling function such as `free` or `objc_copyRealizedClassList_nolock`.
This adds a test that just tries to define our own bogus version of
`objc_copyRealizedClassList_nolock`. It just tests that LLDB doesn't crash as we
currently don't have a way to tell LLDB to look for the function in a specific
library.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D107778
Added:
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/Makefile
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
Modified:
Removed:
diff --git
a/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/Makefile
b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/Makefile
new file mode 100644
index 0..afecbf969483e
--- /dev/null
+++ b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules
diff --git
a/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
new file mode 100644
index 0..3b446d48a6995
--- /dev/null
+++
b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
@@ -0,0 +1,30 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+# LLDB ends up calling the user-defined function (but at least doesn't
+# crash).
+@expectedFailureDarwin
+def test(self):
+"""
+Tests LLDB's behaviour if the user defines their own conflicting
+objc_copyRealizedClassList_nolock function.
+"""
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
lldb.SBFileSpec("main.m"))
+
+# Get the (dynamic) type of our 'id' variable so that our Objective-C
+# runtime information is updated.
+str_val = self.expect_expr("custom_class")
+dyn_val = str_val.GetDynamicValue(lldb.eDynamicCanRunTarget)
+
+# We should have retrieved the proper class list even in presence of
+# the user-defined function.
+self.assertEqual(dyn_val.GetTypeName(), "CustomClass *")
diff --git
a/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
new file mode 100644
index 0..144aa3ebaaf32
--- /dev/null
+++ b/lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
@@ -0,0 +1,27 @@
+#import
+#include
+
+// A function with this signature will be called by LLDB to retrieve the
+// Objective-C class list. We shouldn't call this function that is defined
+// by the user if possible.
+Class *objc_copyRealizedClassList_nolock(unsigned int *outCount) {
+ // Don't try to implement this properly but just abort.
+ abort();
+}
+
+// Define some custom class that makes LLDB read the Objective-C class list.
+@interface CustomClass : NSObject {
+};
+@end
+@implementation CustomClass
+@end
+
+int main(int argc, char **argv) {
+ id custom_class = [[CustomClass alloc] init];
+ // Make sure our trap function is emitted but never called (the test doesn't
+ // launch the executable with any args).
+ if (argc == 123) {
+objc_copyRealizedClassList_nolock(0);
+ }
+ return 0; // break here
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107778: [lldb] Add a test for user-defined objc_copyRealizedClassList_nolock
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG57bf5c86591a: [lldb] Add a test for user-defined
objc_copyRealizedClassList_nolock (authored by teemperor).
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107778/new/
https://reviews.llvm.org/D107778
Files:
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/Makefile
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
Index: lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
===
--- /dev/null
+++ lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
@@ -0,0 +1,27 @@
+#import
+#include
+
+// A function with this signature will be called by LLDB to retrieve the
+// Objective-C class list. We shouldn't call this function that is defined
+// by the user if possible.
+Class *objc_copyRealizedClassList_nolock(unsigned int *outCount) {
+ // Don't try to implement this properly but just abort.
+ abort();
+}
+
+// Define some custom class that makes LLDB read the Objective-C class list.
+@interface CustomClass : NSObject {
+};
+@end
+@implementation CustomClass
+@end
+
+int main(int argc, char **argv) {
+ id custom_class = [[CustomClass alloc] init];
+ // Make sure our trap function is emitted but never called (the test doesn't
+ // launch the executable with any args).
+ if (argc == 123) {
+objc_copyRealizedClassList_nolock(0);
+ }
+ return 0; // break here
+}
Index:
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
===
--- /dev/null
+++
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
@@ -0,0 +1,30 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessDarwin
+# LLDB ends up calling the user-defined function (but at least doesn't
+# crash).
+@expectedFailureDarwin
+def test(self):
+"""
+Tests LLDB's behaviour if the user defines their own conflicting
+objc_copyRealizedClassList_nolock function.
+"""
+
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
lldb.SBFileSpec("main.m"))
+
+# Get the (dynamic) type of our 'id' variable so that our Objective-C
+# runtime information is updated.
+str_val = self.expect_expr("custom_class")
+dyn_val = str_val.GetDynamicValue(lldb.eDynamicCanRunTarget)
+
+# We should have retrieved the proper class list even in presence of
+# the user-defined function.
+self.assertEqual(dyn_val.GetTypeName(), "CustomClass *")
Index:
lldb/test/API/lang/objc/conflicting-class-list-function-from-user/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/objc/conflicting-class-list-function-from-user/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules
Index: lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
===
--- /dev/null
+++ lldb/test/API/lang/objc/conflicting-class-list-function-from-user/main.m
@@ -0,0 +1,27 @@
+#import
+#include
+
+// A function with this signature will be called by LLDB to retrieve the
+// Objective-C class list. We shouldn't call this function that is defined
+// by the user if possible.
+Class *objc_copyRealizedClassList_nolock(unsigned int *outCount) {
+ // Don't try to implement this properly but just abort.
+ abort();
+}
+
+// Define some custom class that makes LLDB read the Objective-C class list.
+@interface CustomClass : NSObject {
+};
+@end
+@implementation CustomClass
+@end
+
+int main(int argc, char **argv) {
+ id custom_class = [[CustomClass alloc] init];
+ // Make sure our trap function is emitted but never called (the test doesn't
+ // launch the executable with any args).
+ if (argc == 123) {
+objc_copyRealizedClassList_nolock(0);
+ }
+ return 0; // break here
+}
Index: lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
===
--- /dev/null
+++ lldb/test/API/lang/objc/conflicting-class-list-function-from-user/TestObjCClassListFunctionFromUser.py
@@ -0,0 +1,30 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest im
[Lldb-commits] [lldb] 9900af5 - [lldb][NFC] Add a FIXME for NameSearchContext::AddFunDecl's missing addDecl
Author: Raphael Isemann Date: 2021-08-10T16:14:27+02:00 New Revision: 9900af52f6b186a260d83321791177728fb369c5 URL: https://github.com/llvm/llvm-project/commit/9900af52f6b186a260d83321791177728fb369c5 DIFF: https://github.com/llvm/llvm-project/commit/9900af52f6b186a260d83321791177728fb369c5.diff LOG: [lldb][NFC] Add a FIXME for NameSearchContext::AddFunDecl's missing addDecl Added: Modified: lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp Removed: diff --git a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp index d95b79a9b1f82..8709c2b0dcea3 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/NameSearchContext.cpp @@ -66,6 +66,7 @@ clang::NamedDecl *NameSearchContext::AddFunDecl(const CompilerType &type, context = LinkageSpecDecl::Create( ast, context, SourceLocation(), SourceLocation(), clang::LinkageSpecDecl::LanguageIDs::lang_c, false); +// FIXME: The LinkageSpecDecl here should be added to m_decl_context. } // Pass the identifier info for functions the decl_name is needed for ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 2db8461 - [lldb][NFC] Fix inversed documentation of Process::GetID/SetID
Author: Raphael Isemann
Date: 2021-08-10T16:15:57+02:00
New Revision: 2db8461a9492cb64046a085f35048b9c4e45bfc2
URL:
https://github.com/llvm/llvm-project/commit/2db8461a9492cb64046a085f35048b9c4e45bfc2
DIFF:
https://github.com/llvm/llvm-project/commit/2db8461a9492cb64046a085f35048b9c4e45bfc2.diff
LOG: [lldb][NFC] Fix inversed documentation of Process::GetID/SetID
Added:
Modified:
lldb/include/lldb/Target/Process.h
Removed:
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index aaa2470d29319..8dcc15b1667b8 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -536,13 +536,13 @@ class Process : public
std::enable_shared_from_this,
uint32_t GetAddressByteSize() const;
+ /// Returns the pid of the process or LLDB_INVALID_PROCESS_ID if there is
+ /// no known pid.
+ lldb::pid_t GetID() const { return m_pid; }
+
/// Sets the stored pid.
///
/// This does not change the pid of underlying process.
- lldb::pid_t GetID() const { return m_pid; }
-
- /// Returns the pid of the process or LLDB_INVALID_PROCESS_ID if there is
- /// no known pid.
void SetID(lldb::pid_t new_pid) { m_pid = new_pid; }
uint32_t GetUniqueID() const { return m_process_unique_id; }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107674: [tests] [trace] Add a more comprehensive test for `thread trace export ctf` command
jj10306 updated this revision to Diff 365477.
jj10306 added a comment.
lint + comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107674/new/
https://reviews.llvm.org/D107674
Files:
lldb/docs/htr.rst
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Plugins/TraceExporter/common/TraceHTR.h
lldb/source/Plugins/TraceExporter/docs/htr.rst
lldb/test/API/commands/trace/TestTraceExport.py
Index: lldb/test/API/commands/trace/TestTraceExport.py
===
--- lldb/test/API/commands/trace/TestTraceExport.py
+++ lldb/test/API/commands/trace/TestTraceExport.py
@@ -34,22 +34,96 @@
substrs=["error: Process is not being traced"],
error=True)
-def testExportCreatesFile(self):
+
+def testHtrBasicSuperBlockPassFullCheck(self):
+'''
+Test the BasicSuperBlock pass of HTR.
+
+This test uses a very small trace so that the expected output is digestible and
+it's possible to manually verify the behavior of the algorithm.
+
+This test exhaustively checks that each entry
+in the output JSON is equal to the expected value.
+
+'''
+
self.expect("trace load -v " +
os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
substrs=["intel-pt"])
ctf_test_file = self.getBuildArtifact("ctf-test.json")
-if os.path.exists(ctf_test_file):
-remove_file(ctf_test_file)
self.expect(f"thread trace export ctf --file {ctf_test_file}")
self.assertTrue(os.path.exists(ctf_test_file))
+with open(ctf_test_file) as f:
+data = json.load(f)
-def testHtrBasicSuperBlockPass(self):
'''
-Test the BasicSuperBlock pass of HTR
+The expected JSON contained by "ctf-test.json"
+
+dur: number of instructions in the block
+
+name: load address of the first instruction of the block and the
+name of the most frequently called function from the block (if applicable)
+
+ph: 'X' for Complete events (see link to documentation below)
+
+pid: the ID of the HTR layer the blocks belong to
+
+ts: offset from the beginning of the trace for the first instruction in the block
+
+See https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.j75x71ritcoy
+for documentation on the Trace Event Format
+'''
+# Comments on the right indicate if a block is a "head" and/or "tail"
+# See BasicSuperBlockMerge in TraceHTR.h for a description of the algorithm
+expected = [
+{"dur":1,"name":"0x400511","ph":"X","pid":0,"ts":0},
+{"dur":1,"name":"0x400518","ph":"X","pid":0,"ts":1},
+{"dur":1,"name":"0x40051f","ph":"X","pid":0,"ts":2},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":3}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":4}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":5},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":6},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":7}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":8}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":9},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":10},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":11}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":12}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":13},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":14},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":15}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":16}, # tail
+{"dur":1,"name":"0x400521","ph":"X","pid":0,"ts":17},
+{"dur":1,"name":"0x400525","ph":"X","pid":0,"ts":18},
+{"dur":1,"name":"0x400529","ph":"X","pid":0,"ts":19}, # head
+{"dur":1,"name":"0x40052d","ph":"X","pid":0,"ts":20}, # tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":3}},"dur":3,"name":"0x400511","ph":"X","pid":1,"ts":0},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":3}, # head, tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400521","ph":"X","pid":1,"ts":5},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":7}, # head, tail
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400521","ph":"X","pid":1,"ts":9},
+{"args":{"Metadata":{"Functions":[],"Number of Instructions":2}},"dur":2,"name":"0x400529","ph":"X","pid":1,"ts":11}, # head, tail
+{
[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks
gAlfonso-bit updated this revision to Diff 365497.
gAlfonso-bit added a comment.
Remove goto
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107704/new/
https://reviews.llvm.org/D107704
Files:
lldb/include/lldb/Core/IOHandler.h
lldb/source/Core/IOHandler.cpp
Index: lldb/source/Core/IOHandler.cpp
===
--- lldb/source/Core/IOHandler.cpp
+++ lldb/source/Core/IOHandler.cpp
@@ -362,17 +362,22 @@
if (prompt == nullptr)
prompt = GetPrompt();
-if (prompt && prompt[0]) {
- if (m_output_sp) {
-m_output_sp->Printf("%s", prompt);
-m_output_sp->Flush();
- }
+if (prompt && prompt[0] && m_output_sp) {
+ m_output_sp->Printf("%s", prompt);
+ m_output_sp->Flush();
}
}
Optional got_line = SplitLine(m_line_buffer);
- if (!got_line && !m_input_sp) {
+ if (got_line) {
+line = got_line.getValue();
+if (m_data_recorder)
+ m_data_recorder->Record(line, true);
+return true;
+ }
+
+ if (!m_input_sp) {
// No more input file, we are done...
SetIsDone(true);
return false;
@@ -381,24 +386,8 @@
FILE *in = GetInputFILE();
char buffer[256];
- if (!got_line && !in && m_input_sp) {
-// there is no FILE*, fall back on just reading bytes from the stream.
-while (!got_line) {
- size_t bytes_read = sizeof(buffer);
- Status error = m_input_sp->Read((void *)buffer, bytes_read);
- if (error.Success() && !bytes_read) {
-got_line = SplitLineEOF(m_line_buffer);
-break;
- }
- if (error.Fail())
-break;
- m_line_buffer += StringRef(buffer, bytes_read);
- got_line = SplitLine(m_line_buffer);
-}
- }
-
- if (!got_line && in) {
-while (!got_line) {
+ if (in) {
+do {
char *r = fgets(buffer, sizeof(buffer), in);
#ifdef _WIN32
// ReadFile on Windows is supposed to set ERROR_OPERATION_ABORTED
@@ -422,16 +411,30 @@
}
m_line_buffer += buffer;
got_line = SplitLine(m_line_buffer);
-}
+} while (!got_line);
+ } else {
+// there is no FILE*, fall back on just reading bytes from the stream.
+do {
+ size_t bytes_read = sizeof(buffer);
+ Status error = m_input_sp->Read((void *)buffer, bytes_read);
+ if (error.Success() && !bytes_read) {
+got_line = SplitLineEOF(m_line_buffer);
+break;
+ }
+ if (error.Fail())
+return false;
+ m_line_buffer += StringRef(buffer, bytes_read);
+ got_line = SplitLine(m_line_buffer);
+} while (!got_line);
}
- if (got_line) {
-line = got_line.getValue();
-if (m_data_recorder)
- m_data_recorder->Record(line, true);
- }
+ if (!got_line)
+return false;
- return (bool)got_line;
+ line = got_line.getValue();
+ if (m_data_recorder)
+m_data_recorder->Record(line, true);
+ return true;
}
#if LLDB_ENABLE_LIBEDIT
Index: lldb/include/lldb/Core/IOHandler.h
===
--- lldb/include/lldb/Core/IOHandler.h
+++ lldb/include/lldb/Core/IOHandler.h
@@ -34,7 +34,7 @@
namespace repro {
class DataRecorder;
}
-}
+} // namespace lldb_private
namespace curses {
class Application;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet
mgorny created this revision.
mgorny added reviewers: labath, JDevlieghere, jasonmolenda, krytarowski, emaste.
mgorny requested review of this revision.
https://reviews.llvm.org/D107840
Files:
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -9,8 +9,39 @@
from gdbremote_testcase import GdbRemoteTestCaseBase
import binascii
+import os
import stat
+import struct
import tempfile
+import typing
+
+
+class GDBStat(typing.NamedTuple):
+st_dev: int
+st_ino: int
+st_mode: int
+st_nlink: int
+st_uid: int
+st_gid: int
+st_rdev: int
+st_size: int
+st_blksize: int
+st_blocks: int
+st_atime: int
+st_mtime: int
+st_ctime: int
+
+
+def uint32_or_zero(x):
+return x if x < 2**32 else 0
+
+
+def uint32_or_max(x):
+return x if x < 2**32 else 2**32 - 1
+
+
+def uint32_trunc(x):
+return x & (2**32 - 1)
class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
@@ -174,6 +205,71 @@
True)
self.expect_gdbremote_sequence()
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_fstat(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+temp_file.write(b"some test data for stat")
+temp_file.flush()
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,0,0#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)#[0-9a-fA-F]{2}$",
+ "capture": {1: "fd"}}],
+True)
+
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+fd = int(context["fd"], 16)
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:fstat:%x#00" % (fd,),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+);(.*)#[0-9a-fA-F]{2}$",
+ "capture": {1: "size", 2: "data"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), 64)
+# NB: we're using .encode() as a hack because the test suite
+# is wrongly using (unicode) str instead of bytes
+gdb_stat = GDBStat(
+*struct.unpack(">IIIQQQIII",
+ self.decode_gdbremote_binary(context["data"])
+.encode("iso-8859-1")))
+sys_stat = os.fstat(temp_file.fileno())
+
+self.assertEqual(gdb_stat.st_dev, uint32_or_zero(sys_stat.st_dev))
+self.assertEqual(gdb_stat.st_ino, uint32_or_zero(sys_stat.st_ino))
+self.assertEqual(gdb_stat.st_mode, uint32_trunc(sys_stat.st_mode))
+self.assertEqual(gdb_stat.st_nlink, uint32_or_max(sys_stat.st_nlink))
+self.assertEqual(gdb_stat.st_uid, uint32_or_zero(sys_stat.st_uid))
+self.assertEqual(gdb_stat.st_gid, uint32_or_zero(sys_stat.st_gid))
+self.assertEqual(gdb_stat.st_rdev, uint32_or_zero(sys_stat.st_rdev))
+self.assertEqual(gdb_stat.st_size, sys_stat.st_size)
+self.assertEqual(gdb_stat.st_blksize, sys_stat.st_blksize)
+self.assertEqual(gdb_stat.st_blocks, sys_stat.st_blocks)
+self.assertEqual(gdb_stat.st_atime,
+ uint32_or_zero(int(sys_stat.st_atime)))
+self.assertEqual(gdb_stat.st_mtime,
+ uint32_or_zero(int(sys_stat.st_mtime)))
+self.assertEqual(gdb_stat.st_ctime,
+ uint32_or_zero(int(sys_stat.st_ctime)))
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:close:%x#00" % (fd,),
+ "send packet: $F0#00"],
+True)
+self.expect_gdbremote_sequence()
+
+
def expect_error(self):
self.test_sequence.add_log_lines(
[{"direction": "send",
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/St
[Lldb-commits] [lldb] f3932b9 - [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names
Author: Jan Kratochvil
Date: 2021-08-10T20:43:24+02:00
New Revision: f3932b9a0b0b7787ccd3572bad134acc4146acaa
URL:
https://github.com/llvm/llvm-project/commit/f3932b9a0b0b7787ccd3572bad134acc4146acaa
DIFF:
https://github.com/llvm/llvm-project/commit/f3932b9a0b0b7787ccd3572bad134acc4146acaa.diff
LOG: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using
.debug_names
Skeleton vs. DWO units mismatch has been fixed in D106270. As they both
have type DWARFUnit it is a bit difficult to debug. So it is better to
make it safe against future changes.
Reviewed By: kimanh, clayborg
Differential Revision: https://reviews.llvm.org/D107659
Added:
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-3.cpp
Modified:
lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index c786451a03df2..4e09b523b7781 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -79,6 +79,7 @@ void AppleDWARFIndex::GetGlobalVariables(
if (!m_apple_names_up)
return;
+ lldbassert(!cu.GetSymbolFileDWARF().GetDwoNum());
const DWARFUnit &non_skeleton_cu = cu.GetNonSkeletonUnit();
DWARFMappedHash::DIEInfoArray hash_data;
m_apple_names_up->AppendAllDIEsInRange(non_skeleton_cu.GetOffset(),
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index 6f2698cc6e6fb..fac6c46b4ccff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -34,6 +34,7 @@ class DWARFIndex {
virtual void
GetGlobalVariables(const RegularExpression ®ex,
llvm::function_ref callback) = 0;
+ /// \a cu must be the skeleton unit if possible, not GetNonSkeletonUnit().
virtual void
GetGlobalVariables(DWARFUnit &cu,
llvm::function_ref callback) = 0;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 2b2c13abb250b..4a148e7744bb1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -124,6 +124,7 @@ void DebugNamesDWARFIndex::GetGlobalVariables(
void DebugNamesDWARFIndex::GetGlobalVariables(
DWARFUnit &cu, llvm::function_ref callback) {
+ lldbassert(!cu.GetSymbolFileDWARF().GetDwoNum());
uint64_t cu_offset = cu.GetOffset();
bool found_entry_for_cu = false;
for (const DebugNames::NameIndex &ni: *m_debug_names_up) {
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 242daa9293914..4d36ef9b34bc4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -359,9 +359,9 @@ void ManualDWARFIndex::GetGlobalVariables(
void ManualDWARFIndex::GetGlobalVariables(
DWARFUnit &unit, llvm::function_ref callback) {
+ lldbassert(!unit.GetSymbolFileDWARF().GetDwoNum());
Index();
- m_set.globals.FindAllEntriesForUnit(unit.GetNonSkeletonUnit(),
- DIERefCallback(callback));
+ m_set.globals.FindAllEntriesForUnit(unit, DIERefCallback(callback));
}
void ManualDWARFIndex::GetObjCMethods(
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
index 42e96af84a969..493d1b4a27023 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
@@ -45,15 +45,16 @@ bool NameToDIE::Find(const RegularExpression ®ex,
}
void NameToDIE::FindAllEntriesForUnit(
-const DWARFUnit &unit,
-llvm::function_ref callback) const {
+DWARFUnit &s_unit, llvm::function_ref callback) const {
+ lldbassert(!s_unit.GetSymbolFileDWARF().GetDwoNum());
+ const DWARFUnit &ns_unit = s_unit.GetNonSkeletonUnit();
const uint32_t size = m_map.GetSize();
for (uint32_t i = 0; i < size; ++i) {
const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-if (unit.GetSymbolFileDWARF().GetDwoNum() == die_ref.dwo_num() &&
-unit.GetDebugSection() == die_ref.section() &&
-unit.GetOffset() <= die_ref.die_offset() &&
-die_ref.die_offset() < unit.GetNextUnitOff
[Lldb-commits] [PATCH] D107521: [lldb/Plugins] Introduce Scripted Interface Factory
JDevlieghere added inline comments. Comment at: lldb/bindings/python/python-wrapper.swig:291 - PyErr_Cleaner py_err_cleaner(true); Unrelated whitespace change? Comment at: lldb/include/lldb/Interpreter/ScriptedInterface.h:25 + virtual StructuredData::GenericSP + CreatePluginObject(const llvm::StringRef class_name, + ExecutionContext &exe_ctx, We generally don't use const when passing by value. Comment at: lldb/include/lldb/Interpreter/ScriptedInterface.h:30 +protected: + StructuredData::GenericSP m_object_instance_sp = nullptr; +}; Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:28 +#include "llvm/ADT/Twine.h" + Why is this needed? Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:38-39 - std::string error_string; + Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN, + Locker::FreeLock); + Why was this moved up? The lock should be as tight as possible. Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:88 + // TODO: Implement + return nullptr; } Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:99 + "ScriptedProcess::%s ERROR = %s", __FUNCTION__, + message.str().c_str()); +return StructuredData::DictionarySP(); Seems like you're always calling `error_with_message` with a constant string, so we can avoid allocating a std::string. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107521/new/ https://reviews.llvm.org/D107521 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107659: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3932b9a0b0b: [nfc] [lldb] Assertions for D106270 - [DWARF5]
Fix offset check when using . (authored by jankratochvil).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107659/new/
https://reviews.llvm.org/D107659
Files:
lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-3.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
Index: lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
@@ -31,12 +31,15 @@
// the compile unit using the name index if it is split.
// RUN: %clang -c -o %t-1.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %s
// RUN: %clang -c -o %t-2.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %S/Inputs/find-variable-file-2.cpp
-// RUN: ld.lld %t-1.o %t-2.o -o %t
+// RUN: %clang -c -o %t-3.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %S/Inputs/find-variable-file-3.cpp
+// RUN: ld.lld %t-1.o %t-2.o %t-3.o -o %t
// RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix NAMES
// RUN: lldb-test symbols --file=find-variable-file.cpp --find=variable %t | \
// RUN: FileCheck --check-prefix=ONE %s
// RUN: lldb-test symbols --file=find-variable-file-2.cpp --find=variable %t | \
// RUN: FileCheck --check-prefix=TWO %s
+// RUN: lldb-test symbols --file=find-variable-file-3.cpp --find=variable \
+// RUN: --name=notexists %t
// NAMES: Name: .debug_names
Index: lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-3.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-3.cpp
@@ -0,0 +1,2 @@
+// No variable should exist in this file.
+void f() {}
Index: lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
@@ -38,8 +38,9 @@
bool Find(const lldb_private::RegularExpression ®ex,
llvm::function_ref callback) const;
+ /// \a unit must be the skeleton unit if possible, not GetNonSkeletonUnit().
void
- FindAllEntriesForUnit(const DWARFUnit &unit,
+ FindAllEntriesForUnit(DWARFUnit &unit,
llvm::function_ref callback) const;
void
Index: lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
@@ -45,15 +45,16 @@
}
void NameToDIE::FindAllEntriesForUnit(
-const DWARFUnit &unit,
-llvm::function_ref callback) const {
+DWARFUnit &s_unit, llvm::function_ref callback) const {
+ lldbassert(!s_unit.GetSymbolFileDWARF().GetDwoNum());
+ const DWARFUnit &ns_unit = s_unit.GetNonSkeletonUnit();
const uint32_t size = m_map.GetSize();
for (uint32_t i = 0; i < size; ++i) {
const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-if (unit.GetSymbolFileDWARF().GetDwoNum() == die_ref.dwo_num() &&
-unit.GetDebugSection() == die_ref.section() &&
-unit.GetOffset() <= die_ref.die_offset() &&
-die_ref.die_offset() < unit.GetNextUnitOffset()) {
+if (ns_unit.GetSymbolFileDWARF().GetDwoNum() == die_ref.dwo_num() &&
+ns_unit.GetDebugSection() == die_ref.section() &&
+ns_unit.GetOffset() <= die_ref.die_offset() &&
+die_ref.die_offset() < ns_unit.GetNextUnitOffset()) {
if (!callback(die_ref))
return;
}
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -359,9 +359,9 @@
void ManualDWARFIndex::GetGlobalVariables(
DWARFUnit &unit, llvm::function_ref callback) {
+ lldbassert(!unit.GetSymbolFileDWARF().GetDwoNum());
Index();
- m_set.globals.FindAllEntriesForUnit(unit.GetNonSkeletonUnit(),
- DIERefCallback(callback));
+ m_set.globals.FindAllEntriesForUnit(unit, DIERefCallback(callback));
}
void ManualDWARFIndex::GetObjCMethods(
Index: lldb/source/Plugins/SymbolFile/DWARF/D
[Lldb-commits] [PATCH] D107521: [lldb/Plugins] Introduce Scripted Interface Factory
jingham added a comment.
This is a good start, but it seems like there's still a lot of boilerplate once
you get to the python side that could be trimmed down.
For instance, ScriptedProcessPythonInterface::GetThreadWithID is almost
entirely generic. The only things that make it specific to this use are the
method name that's called, and the fact that it passes an lldb::tid_t to and
returns an SBStructuredData.
You've started to take care of the latter by building up a set of generic "call
a method, return X" routines, and surely an SBStructuredData one will be pretty
common.
Then you can add a way to pass arguments to the generic routines. Maybe an
array of some little struct that has either the duple "python format
string/value" or a PythonObject so the dispatcher can call the code
appropriately?
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp:32-44
+llvm::Optional
+ScriptedPythonInterface::GetGenericInteger(llvm::StringRef method_name) {
+ Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
+ Locker::FreeLock);
+
+ if (!m_object_instance_sp)
+return llvm::None;
It seems like there is a whole lot of duplicated code here. The only
difference is in what we do with the returned PythonObject.
Maybe we should have a centralized "dispatch" method and then these routines
differ in the conversion.
Also, it seems a shame to throw away error info, but that's what the
GetGenericInteger & GetGenericString do when the object instance isn't valid,
etc. llvm::Optional is good for forcing validity checks, but has no
information on why the operation didn't succeed. It would be better to have
all these methods take a status object for errors. You can still return the
result wrapped in an optional.
When you are developing one of these classes, it would be nice to be told "no
such method" for instance so you can go check if you misspelled it, or
something, rather than just getting llvm::None that eventually results in some
more remote error.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107521/new/
https://reviews.llvm.org/D107521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107704: [LLDB][NFC] Simplify IOHandler's getLine to avoid strange casts and redundant checks
gAlfonso-bit marked an inline comment as done. gAlfonso-bit added a comment. Addressed all issues! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107704/new/ https://reviews.llvm.org/D107704 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form
OmarEmaraDev created this revision.
OmarEmaraDev added a reviewer: clayborg.
Herald added a reviewer: teemperor.
OmarEmaraDev requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
This patch adds a process launch form. Additionally, a LazyBoolean field
was implemented and numerous utility methods were added to various
fields to get the launch form working.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D107869
Files:
lldb/source/Core/IOHandlerCursesGUI.cpp
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -28,6 +28,7 @@
#include "lldb/Core/StreamFile.h"
#include "lldb/Core/ValueObjectUpdater.h"
#include "lldb/Host/File.h"
+#include "lldb/Host/HostInfo.h"
#include "lldb/Utility/Predicate.h"
#include "lldb/Utility/Status.h"
#include "lldb/Utility/StreamString.h"
@@ -1619,6 +1620,33 @@
}
};
+class LazyBooleanFieldDelegate : public ChoicesFieldDelegate {
+public:
+ LazyBooleanFieldDelegate(const char *label, const char *calculate_label)
+ : ChoicesFieldDelegate(label, 3, GetPossibleOptions(calculate_label)) {}
+
+ static constexpr const char *kNo = "No";
+ static constexpr const char *kYes = "Yes";
+
+ std::vector GetPossibleOptions(const char *calculate_label) {
+std::vector options;
+options.push_back(calculate_label);
+options.push_back(kYes);
+options.push_back(kNo);
+return options;
+ }
+
+ LazyBool GetLazyBoolean() {
+std::string choice = GetChoiceContent();
+if (choice == kNo)
+ return eLazyBoolNo;
+else if (choice == kYes)
+ return eLazyBoolYes;
+else
+ return eLazyBoolCalculate;
+ }
+};
+
template class ListFieldDelegate : public FieldDelegate {
public:
ListFieldDelegate(const char *label, T default_field)
@@ -1909,6 +1937,21 @@
SelectionType m_selection_type;
};
+class ArgumentsFieldDelegate : public ListFieldDelegate {
+public:
+ ArgumentsFieldDelegate()
+ : ListFieldDelegate("Arguments",
+ TextFieldDelegate("Argument", "", false)) {}
+
+ Args GetArguments() {
+Args arguments;
+for (int i = 0; i < GetNumberOfFields(); i++) {
+ arguments.AppendArgument(GetField(i).GetText());
+}
+return arguments;
+ }
+};
+
template
class MappingFieldDelegate : public FieldDelegate {
public:
@@ -2077,6 +2120,15 @@
EnvironmentVariableListFieldDelegate()
: ListFieldDelegate("Environment Variables",
EnvironmentVariableFieldDelegate()) {}
+
+ Environment GetEnvironment() {
+Environment environment;
+for (int i = 0; i < GetNumberOfFields(); i++) {
+ environment.insert(
+ std::make_pair(GetField(i).GetName(), GetField(i).GetValue()));
+}
+return environment;
+ }
};
class FormAction {
@@ -2201,6 +2253,14 @@
return delegate;
}
+ LazyBooleanFieldDelegate *AddLazyBooleanField(const char *label,
+const char *calculate_label) {
+LazyBooleanFieldDelegate *delegate =
+new LazyBooleanFieldDelegate(label, calculate_label);
+m_fields.push_back(FieldDelegateUP(delegate));
+return delegate;
+ }
+
ChoicesFieldDelegate *AddChoicesField(const char *label, int height,
std::vector choices) {
ChoicesFieldDelegate *delegate =
@@ -2230,6 +2290,12 @@
return delegate;
}
+ ArgumentsFieldDelegate *AddArgumentsField() {
+ArgumentsFieldDelegate *delegate = new ArgumentsFieldDelegate();
+m_fields.push_back(FieldDelegateUP(delegate));
+return delegate;
+ }
+
template
MappingFieldDelegate *AddMappingField(K key_field, V value_field) {
MappingFieldDelegate *delegate =
@@ -3031,6 +3097,309 @@
ChoicesFieldDelegate *m_load_dependent_files_field;
};
+class ProcessLaunchFormDelegate : public FormDelegate {
+public:
+ ProcessLaunchFormDelegate(Debugger &debugger, WindowSP main_window_sp)
+ : m_debugger(debugger), m_main_window_sp(main_window_sp) {
+
+m_arguments_field = AddArgumentsField();
+m_enviroment_field = AddEnvironmentVariableListField();
+
+m_show_advanced_field = AddBooleanField("Show advanced settings.", false);
+
+m_stop_at_entry_field = AddBooleanField("Stop at entry point.", false);
+m_working_directory_field =
+AddDirectoryField("Working Directory", "", /*need_to_exist=*/true,
+ /*required=*/false);
+m_disable_aslr_field =
+AddLazyBooleanField("Disable ASLR", "Use target setting");
+m_plugin_field = AddProcessPluginField();
+m_arch_field = AddArchField("Architecture", "", /*required=*/false);
+m_shell_field = AddFileField("Shell", "", /*need_to_exist=*/true,
+ /*required=*/false);
+m_expand_shell_arguments_field =
+
[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form
OmarEmaraDev added a comment. Herald added a subscriber: JDevlieghere. This is not fully working yet, and I am still debugging it. But I thought I would push it for early feedback regardless. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107869/new/ https://reviews.llvm.org/D107869 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D107869: [LLDB][GUI] Add Process Launch form
clayborg added inline comments.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3105
+
+m_arguments_field = AddArgumentsField();
+m_enviroment_field = AddEnvironmentVariableListField();
We should fill in any arguments automatically into this field from the target
and let the user modify them as needed.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3106
+m_arguments_field = AddArgumentsField();
+m_enviroment_field = AddEnvironmentVariableListField();
+
We should fill in any current environment variables from the target if we
aren't already doing this.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3110
+
+m_stop_at_entry_field = AddBooleanField("Stop at entry point.", false);
+m_working_directory_field =
Set default values for all fields in here from the target if/when possible.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3123
+m_standard_output_field =
+AddFileField("Standard Output", "", /*need_to_exist=*/false,
+ /*required=*/false);
Maybe label as "Redirect Standard Output" or "Standard Output File"?
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3126
+m_standard_error_field =
+AddFileField("Standard Error", "", /*need_to_exist=*/false,
+ /*required=*/false);
If you change above field, then modify this one as well.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3129
+m_standard_input_field =
+AddFileField("Standard Input", "", /*need_to_exist=*/false,
+ /*required=*/false);
If you change above field, then modify this one as well.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3140
+ m_stop_at_entry_field->FieldDelegateShow();
+ m_working_directory_field->FieldDelegateShow();
+ m_disable_aslr_field->FieldDelegateShow();
I'd pull working directory out of advanced settings.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3163
+
+ void SetArguments(ProcessLaunchInfo &launch_info) {
+TargetSP target = m_debugger.GetSelectedTarget();
Should this be labeled "GetArguments"? The name seems like it would set the
arguments in this class from launch_info, but it is getting them from this
class and filling them into "launch_info". Ditto for all accessors that fill in
"launch_info" below.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3237-3240
+} else {
+ action.Open(STDIN_FILENO, dev_null, true, false);
+ launch_info.AppendFileAction(action);
+}
We don't need to do anything if this isn't specified as by default the input
will be hooked up by the debugging to something valid. If the users wants to
redirect to /dev/null, they can just specify "/dev/null" (or the right path for
this on their system.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3246-3249
+} else {
+ action.Open(STDOUT_FILENO, dev_null, false, true);
+ launch_info.AppendFileAction(action);
+}
Ditto above, don't do anything if this isn't specified.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3255-3258
+} else {
+ action.Open(STDERR_FILENO, dev_null, false, true);
+ launch_info.AppendFileAction(action);
+}
Ditto above, don't do anything if this isn't specified.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3264-3265
+
+if (target->GetInheritTCC())
+ launch_info.GetFlags().Set(eLaunchFlagInheritTCCFromParent);
+
This is an obscure MacOS specific setting, so no need to make a field of it
like suggested below.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3267-3268
+
+if (target->GetDetachOnError())
+ launch_info.GetFlags().Set(eLaunchFlagDetachOnError);
+
We should make a Boolean input field in the advanced settings, and the default
should be set to "target->GetDetachOnError()". Then the user can modify this if
needed.
Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:3270-3271
+
+if (target->GetDisableSTDIO())
+ launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
+ }
This should be moved above all of the STDIO redirection stuff above since it
will cause everything to be redirected to /dev/null or equivalent. We should
make a boolean setting for this in the advanced stuff, and if this is set to
true, then don't show the output redirection fields (m_standard_input_field,
m_standard_output_field and m_standard_error_field) in UpdateFieldsVisibility()
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107869/new/
[Lldb-commits] [lldb] c874dd5 - [llvm][clang][NFC] updates inline licence info
Author: Christopher Di Bella Date: 2021-08-11T02:48:53Z New Revision: c874dd53628db8170d4c5ba3878817abc385a695 URL: https://github.com/llvm/llvm-project/commit/c874dd53628db8170d4c5ba3878817abc385a695 DIFF: https://github.com/llvm/llvm-project/commit/c874dd53628db8170d4c5ba3878817abc385a695.diff LOG: [llvm][clang][NFC] updates inline licence info Some files still contained the old University of Illinois Open Source Licence header. This patch replaces that with the Apache 2 with LLVM Exception licence. Differential Revision: https://reviews.llvm.org/D107528 Added: Modified: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.h clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h clang/include/clang/AST/ASTConcept.h clang/include/clang/AST/ASTImporterSharedState.h clang/include/clang/AST/CurrentSourceLocExprScope.h clang/include/clang/AST/JSONNodeDumper.h clang/include/clang/Sema/SemaConcept.h clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h clang/lib/AST/ASTConcept.cpp clang/lib/Format/MacroExpander.cpp clang/lib/Format/Macros.h clang/lib/Index/FileIndexRecord.cpp clang/lib/Sema/SemaConcept.cpp clang/lib/StaticAnalyzer/Core/SMTConstraintManager.cpp clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp clang/unittests/Format/TestLexer.h clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaTemplateParams.cpp lldb/docs/use/python.rst lldb/unittests/Symbol/TestLineEntry.cpp llvm/include/llvm/Analysis/HeatUtils.h llvm/include/llvm/CodeGen/MIRFormatter.h llvm/include/llvm/CodeGen/RegAllocCommon.h llvm/include/llvm/Support/ExtensibleRTTI.h llvm/include/llvm/Support/LICENSE.TXT llvm/include/llvm/Support/Signposts.h llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerOptions.h llvm/include/llvm/Transforms/Instrumentation/HWAddressSanitizer.h llvm/include/llvm/Transforms/Instrumentation/InstrOrderFile.h llvm/include/llvm/Transforms/Instrumentation/MemProfiler.h llvm/include/llvm/Transforms/Utils/MemoryOpRemark.h llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp llvm/lib/Analysis/HeatUtils.cpp llvm/lib/Analysis/InlineSizeEstimatorAnalysis.cpp llvm/lib/Analysis/TFUtils.cpp llvm/lib/BinaryFormat/MsgPackDocumentYAML.cpp llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp llvm/lib/DebugInfo/GSYM/FileWriter.cpp llvm/lib/DebugInfo/GSYM/Range.cpp llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp llvm/lib/ExecutionEngine/JITLink/ELF.cpp llvm/lib/ExecutionEngine/JITLink/JITLink.cpp llvm/lib/ExecutionEngine/JITLink/JITLinkMemoryManager.cpp llvm/lib/ExecutionEngine/JITLink/MachO.cpp llvm/lib/ExecutionEngine/Orc/TargetProcess/RegisterEHFrames.cpp llvm/lib/Support/ExtensibleRTTI.cpp llvm/lib/Support/Signposts.cpp llvm/lib/Target/AArch64/AArch64StackTagging.cpp llvm/lib/Target/AArch64/AArch64StackTaggingPreRA.cpp llvm/lib/Target/AArch64/SVEIntrinsicOpts.cpp llvm/lib/Target/CSKY/AsmParser/CSKYAsmParser.cpp llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.h llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFStreamer.cpp llvm/lib/Target/PowerPC/MCTargetDesc/PPCXCOFFStreamer.h llvm/lib/Target/X86/X86InstrKL.td llvm/lib/Transforms/Instrumentation/InstrOrderFile.cpp llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp llvm/unittests/BinaryFormat/MsgPackDocumentTest.cpp llvm/unittests/DebugInfo/GSYM/GSYMTest.cpp llvm/unittests/Support/ExtensibleRTTITest.cpp Removed: diff --git a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp index 9b449515f27da..86e3f3b7da6a2 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp @@ -1,9 +1,8 @@ //===--- BranchCloneCheck.cpp - clang-tidy ===// // -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illi
