[Lldb-commits] [PATCH] D92729: [LLDB] fix error message for one-line breakpoint scripts

[Pedro Tammela via Phabricator via lldb-commits]

tammela created this revision.
tammela requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

LLDB is ignoring compilation errors for one-line breakpoint scripts.
This patch fixes the issues and now the error message of the
ScriptInterpreter is shown to the user.

I had to remove a new-line character for the Lua interpreter since it
was duplicated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92729

Files:
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
  lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test


Index: lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
@@ -0,0 +1,7 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language python 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o "1234_foo"
+# CHECK: error: SyntaxError({{.*}})
Index: lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
@@ -0,0 +1,5 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o '1234_foo'
+# CHECK: error: {{.*}} unexpected symbol near '1234'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -86,7 +86,7 @@
   std::string func_str = llvm::formatv(fmt_str, body).str();
   if (luaL_dostring(m_lua_state, func_str.c_str()) != LUA_OK) {
 llvm::Error e = llvm::make_error(
-llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
+llvm::formatv("{0}", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());
 // Pop error message from the stack.
 lua_pop(m_lua_state, 2);
Index: lldb/source/Commands/CommandObjectBreakpointCommand.cpp
===
--- lldb/source/Commands/CommandObjectBreakpointCommand.cpp
+++ lldb/source/Commands/CommandObjectBreakpointCommand.cpp
@@ -414,22 +414,23 @@
   // to set or collect command callback.  Otherwise, call the methods
   // associated with this object.
   if (m_options.m_use_script_language) {
+Status error;
 ScriptInterpreter *script_interp = GetDebugger().GetScriptInterpreter(
 /*can_create=*/true, m_options.m_script_language);
 // Special handling for one-liner specified inline.
 if (m_options.m_use_one_liner) {
-  script_interp->SetBreakpointCommandCallback(
+  error = script_interp->SetBreakpointCommandCallback(
   m_bp_options_vec, m_options.m_one_liner.c_str());
 } else if (!m_func_options.GetName().empty()) {
-  Status error = script_interp->SetBreakpointCommandCallbackFunction(
+  error = script_interp->SetBreakpointCommandCallbackFunction(
   m_bp_options_vec, m_func_options.GetName().c_str(),
   m_func_options.GetStructuredData());
-  if (!error.Success())
-result.SetError(error);
 } else {
   script_interp->CollectDataForBreakpointCommandCallback(
   m_bp_options_vec, result);
 }
+if (!error.Success())
+  result.SetError(error);
   } else {
 // Special handling for one-liner specified inline.
 if (m_options.m_use_one_liner)


Index: lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/fail_breakpoint_oneline.test
@@ -0,0 +1,7 @@
+# REQUIRES: python
+# UNSUPPORTED: lldb-repro
+#
+# RUN: cat %s | %lldb --script-language python 2>&1 | FileCheck %s
+b main
+breakpoint command add -s python -o "1234_foo"
+# CHECK: error: SyntaxError({{.*}})
Index: lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/fail_breakpoint_oneline.test
@@ -0,0 +1,5 @@
+# REQUIRES: lua
+# RUN: %lldb -s %s --script-language lua 2>&1 | FileCheck %s
+b main
+breakpoint command add -s lua -o '1234_foo'
+# CHECK: error: {{.*}} unexpected symbol near '1234'
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterprete

[Lldb-commits] [PATCH] D92264: [lldb] [POSIX-DYLD] Update the cached exe path after attach

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

Seems reasonable to me


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92264/new/

https://reviews.llvm.org/D92264

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92314: [lldb] [Process/FreeBSDRemote] Implement GetLoadedModuleFileSpec() and GetFileLoadAddress()

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D92314#2422995 , @emaste wrote:

> No objection, but maybe add a comment explaining the status of this 
> implementation? Does/will NetBSD do the same?

I will add the comment, and I'm planning to sync NetBSD in my free time, later 
on.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92314/new/

https://reviews.llvm.org/D92314

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D92314: [lldb] [Process/FreeBSDRemote] Implement GetLoadedModuleFileSpec() and GetFileLoadAddress()

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 309793.
mgorny added a comment.

Added an explanatory comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92314/new/

https://reviews.llvm.org/D92314

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
@@ -67,9 +67,13 @@
   Status SetBreakpoint(lldb::addr_t addr, uint32_t size,
bool hardware) override;
 
+  // The two following methods are probably not necessary and probably
+  // will never be called.  Nevertheless, we implement them right now
+  // to reduce the differences between different platforms and reduce
+  // the risk of the lack of implementation actually breaking something,
+  // at least for the time being.
   Status GetLoadedModuleFileSpec(const char *module_path,
  FileSpec &file_spec) override;
-
   Status GetFileLoadAddress(const llvm::StringRef &file_name,
 lldb::addr_t &load_addr) override;
 
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -630,14 +630,40 @@
 
 Status NativeProcessFreeBSD::GetLoadedModuleFileSpec(const char *module_path,
  FileSpec &file_spec) {
-  return Status("Unimplemented");
+  Status error = PopulateMemoryRegionCache();
+  if (error.Fail())
+return error;
+
+  FileSpec module_file_spec(module_path);
+  FileSystem::Instance().Resolve(module_file_spec);
+
+  file_spec.Clear();
+  for (const auto &it : m_mem_region_cache) {
+if (it.second.GetFilename() == module_file_spec.GetFilename()) {
+  file_spec = it.second;
+  return Status();
+}
+  }
+  return Status("Module file (%s) not found in process' memory map!",
+module_file_spec.GetFilename().AsCString());
 }
 
 Status
 NativeProcessFreeBSD::GetFileLoadAddress(const llvm::StringRef &file_name,
  lldb::addr_t &load_addr) {
   load_addr = LLDB_INVALID_ADDRESS;
-  return Status();
+  Status error = PopulateMemoryRegionCache();
+  if (error.Fail())
+return error;
+
+  FileSpec file(file_name);
+  for (const auto &it : m_mem_region_cache) {
+if (it.second == file) {
+  load_addr = it.first.GetRange().GetRangeBase();
+  return Status();
+}
+  }
+  return Status("No load address found for file %s.", file_name.str().c_str());
 }
 
 void NativeProcessFreeBSD::SigchldHandler() {


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
@@ -67,9 +67,13 @@
   Status SetBreakpoint(lldb::addr_t addr, uint32_t size,
bool hardware) override;
 
+  // The two following methods are probably not necessary and probably
+  // will never be called.  Nevertheless, we implement them right now
+  // to reduce the differences between different platforms and reduce
+  // the risk of the lack of implementation actually breaking something,
+  // at least for the time being.
   Status GetLoadedModuleFileSpec(const char *module_path,
  FileSpec &file_spec) override;
-
   Status GetFileLoadAddress(const llvm::StringRef &file_name,
 lldb::addr_t &load_addr) override;
 
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -630,14 +630,40 @@
 
 Status NativeProcessFreeBSD::GetLoadedModuleFileSpec(const char *module_path,
  FileSpec &file_spec) {
-  return Status("Unimplemented");
+  Status error = PopulateMemoryRegionCache();
+  if (error.Fail())
+return error;
+
+  FileSpec module_file_spec(module_path);
+  FileSystem::Instance().Resolve(module_file_spec);
+
+  file_spec.Clear();
+  for (const auto &it : m_mem_region_cache) {
+if (it.second.GetFilename() == module_file_spec.GetFilename()) {
+  file_spec = it.second;
+  return Status();
+}
+  }
+  return Status("Module file (%s) not found in process' memory map!",
+module_file_spec.GetFilename().AsCString());
 

[Lldb-commits] [PATCH] D92740: [lldb] [test] Link FreeBSD test failures to bugs

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: emaste, labath, krytarowski.
mgorny requested review of this revision.

https://reviews.llvm.org/D92740

Files:
  lldb/test/API/api/multithreaded/TestMultithreaded.py
  
lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
  
lldb/test/API/commands/process/launch-with-shellexpand/TestLaunchWithShellExpand.py
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/API/commands/target/create-deps/TestTargetCreateDeps.py
  
lldb/test/API/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
  
lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py
  lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
  
lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
  lldb/test/API/functionalities/return-value/TestReturnValue.py
  
lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py
  
lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
  
lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
  lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
  lldb/test/API/python_api/event/TestEvents.py
  lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/test/API/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
  lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
  lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py

Index: lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -181,8 +181,8 @@
 
 @skipIfWindows
 @skipIfLinux # shell argument expansion doesn't seem to work on Linux
-@expectedFailureNetBSD
-@expectedFailureAll(oslist=["freebsd"])
+@expectedFailureAll(oslist=["freebsd", "netbsd"],
+bugnumber="llvm.org/pr48349")
 @skipIfRemote
 def test_shellExpandArguments_enabled(self):
 '''
Index: lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
===
--- lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
+++ lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
@@ -25,7 +25,7 @@
 
 @skipIfWindows
 @skipIfRemote
-@expectedFailureAll(oslist=["freebsd"])
+@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
 def test_source_map(self):
 self.build_and_create_debug_adaptor()
 
@@ -90,7 +90,7 @@
 
 @skipIfWindows
 @skipIfRemote
-@expectedFailureAll(oslist=["freebsd"])
+@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
 def test_set_and_clear(self):
 '''Tests setting and clearing source file and line breakpoints.
This packet is a bit tricky on the debug adaptor side since there
@@ -223,7 +223,7 @@
 
 @skipIfWindows
 @skipIfRemote
-@expectedFailureAll(oslist=["freebsd"])
+@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
 def test_clear_breakpoints_unset_breakpoints(self):
 '''Test clearing breakpoints like test_set_and_clear, but clear
breakpoints by omitting the breakpoints array instead of sending an
@@ -266,7 +266,7 @@
 
 @skipIfWindows
 @skipIfRemote
-@expectedFailureAll(oslist=["freebsd"])
+@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48421")
 def test_functionality(self):
 '''Tests hitting breakpoints and the functionality of a single
breakpoint, like 'conditions' and 'hitCondition' settings.'''
Index: lldb/test/API/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
===
--- lldb/test/API/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
+++ lldb/test/API/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
@@ -136,7 +136,8 @@
 self.assertEqual(
 ['0x727476787a7c7e71', '0x737577797b7d7f70'], get_reg_value('xmm15'))
 
-@expectedFailureAll(oslist=["freebsd", "netbsd"])
+@expectedFailureAll(oslist=["freebsd"], bugnumber="llvm.org/pr48420")
+@expectedFailureNetBSD
 @llgs_test
 def test_g_returns_correct_data_with_suffix_llgs(self):
 self.init_llgs_test()
@@ -144,7 +145,8 @@
 self.set_inferior_startup_launch()
 self.g_returns_correct_data(True)
 
-@expectedFailureAll(oslist=["freebsd", "netbsd"])
+@expe