[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

2021-10-10 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

> Is it possible to get your IDE to record the module & the address?

I don't control the definition of the protocol (FWIW it's Microsoft's Debug 
Adapter Protocol), but I'll look into that, maybe there's a way to sneak in 
extra information.   In which case, what is the correct way to save/restore 
instruction breakpoints across lldb runs?   Even if I save module path or UUID, 
there seems to be no SB API to create a breakpoint using this information.

What if the module gets recompiled?  I still could end up with my breakpoint 
slapped in the middle of an instruction.   It seems to me that perfect safety 
is just impossible here...

> More generally, using raw address breakpoints is fallible, and whenever you 
> find yourself doing that you should probably see if you can do anything else.
> ...
> This patch as is worries me because it moves us further down the path of 
> passing around raw addresses from run to run, and when that doesn't work it 
> can cause hard to diagnose problems in the running of the process.

I dunno, I guess I am in the camp of "Give a stern warning, but don't second 
guess the developer".It's pretty annoying when your tool refuses to do 
something, which you know is reasonable under the circumstances, not because of 
a technical limitation, but "for your own good".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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


[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

2021-10-08 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

@JDevlieghere: ping.Or are you saying you wouldn't consider landing such a 
change at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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


[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

2021-09-15 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

Hi Jim,
I think there's a bit of confusion going on here:
The original bug was opened by Ted Woodward, and I think his scenario was
motivated by embedded debugging.   He also provided that example with a
breakpoint being set in main.

I've rediscovered this bug independently, while working on an IDE
integration project.  In my case, the IDE saves just the address of the
instruction where a breakpoint was set in the previous debugging session.
In the next session, the debug adapter is expected to restore instruction
breakpoints using just the absolute addresses; the protocol does not
provide for any user interaction.  Also, this happens via the SB API, not
via LLDB commands.   Fortunately, when ASLR is disabled, modules are
usually loaded at the same address, so things tend to work out 99% of the
time.  The other 1%... well, if you are debugging on the assembly
instruction level, you are kinda expected to know what you are doing and be
prepared to deal with problems like those you've described above.

When the target is created, the modules for the target are loaded and

> mapped at whatever address is their default load address.

Clearly, this isn't happening.  From what I can see, section addresses are
not known until the process starts running.   Maybe this is another bug
that needs fixing?
Even so, I think that address breakpoints should still function when
section addresses aren't known at breakpoint creation time (for whatever
reason), if only on a best-effort basis.

3. If the address doesn't fall into any sections, then it would be fine to

> assign it to the first section that shows up at that address, but I would
> convert it to section relative at that point.

This is what I am attempting to do with my patch.  Is there something else
I should do to make sure I am not regressing other scenarios?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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


[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

2021-09-15 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn updated this revision to Diff 372636.
vadimcn added a comment.

Added patch context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

Files:
  lldb/source/Breakpoint/BreakpointResolverAddress.cpp
  
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py


Index: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
===
--- 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
+++ 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
@@ -79,10 +79,33 @@
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)
 
-thread = get_threads_stopped_at_breakpoint(process, breakpoint)
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
 self.assertEqual(
 len(threads), 1,
 "There should be a thread stopped at our breakpoint")
 
 # The hit count for the breakpoint should now be 2.
 self.assertEquals(breakpoint.GetHitCount(), 2)
+
+process.Kill()
+
+# Create a breakpoint again, this time using the load address
+load_address = address.GetLoadAddress(target)
+
+# Re-create the target to make sure that nothing is cached
+target = self.createTestTarget()
+breakpoint = target.BreakpointCreateByAddress(load_address)
+
+launch_info.Clear()
+launch_info.SetLaunchFlags(flags)
+
+process = target.Launch(launch_info, error)
+self.assertTrue(process, PROCESS_IS_VALID)
+
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
+self.assertEqual(
+len(threads), 1,
+"There should be a thread stopped at our breakpoint")
+
+# The hit count for the breakpoint should be 1.
+self.assertEquals(breakpoint.GetHitCount(), 1)
Index: lldb/source/Breakpoint/BreakpointResolverAddress.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -97,8 +97,12 @@
   bool re_resolve = false;
   if (m_addr.GetSection() || m_module_filespec)
 re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
-re_resolve = true;
+  else {
+BreakpointSP breakpoint = GetBreakpoint();
+if (breakpoint->GetNumLocations() == 0 ||
+breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
+  re_resolve = true;
+  }
 
   if (re_resolve)
 BreakpointResolver::ResolveBreakpoint(filter);
@@ -110,8 +114,12 @@
   bool re_resolve = false;
   if (m_addr.GetSection())
 re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
-re_resolve = true;
+  else {
+BreakpointSP breakpoint = GetBreakpoint();
+if (breakpoint->GetNumLocations() == 0 ||
+breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
+  re_resolve = true;
+  }
 
   if (re_resolve)
 BreakpointResolver::ResolveBreakpointInModules(filter, modules);
@@ -151,7 +159,7 @@
   BreakpointLocationSP loc_sp = breakpoint.GetLocationAtIndex(0);
   lldb::addr_t cur_load_location =
   m_addr.GetLoadAddress(());
-  if (cur_load_location != m_resolved_addr) {
+  if (cur_load_location != m_resolved_addr || !loc_sp->IsResolved()) {
 m_resolved_addr = cur_load_location;
 loc_sp->ClearBreakpointSite();
 loc_sp->ResolveBreakpointSite();


Index: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
===
--- lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
+++ lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
@@ -79,10 +79,33 @@
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)
 
-thread = get_threads_stopped_at_breakpoint(process, breakpoint)
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
 self.assertEqual(
 len(threads), 1,
 "There should be a thread stopped at our breakpoint")
 
 # The hit count for the breakpoint should now be 2.
 self.assertEquals(breakpoint.GetHitCount(), 2)
+
+process.Kill()
+
+# Create a breakpoint again, this time using the load address
+load_address = address.GetLoadAddress(target)
+
+# Re-create the target to make sure that nothing is cached
+target = self.createTestTarget()
+breakpoint = target.BreakpointCreateByAddress(load_address)
+
+launch_info.Clear()
+launch_info.SetLaunchFlags(flags)
+
+   

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

2021-09-15 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

> In the future, can you generate patches with context (i.e. pass -U to
> "git diff" or "git show"), it's a lot easier to read patches with context.

Sure thing, will do.

This doesn't seem like the right solution to the problem to me.  The way

> address breakpoints SHOULD work  is that when you set the breakpoint, we
> resolve the address you pass in against the images in the target.  If the
> address is in an image in the list, then we convert the load address to a
> section relative address before making the breakpoint.  Address breakpoints
> made that way will always return true for m_addr.GetSection(), and so set
> re_resolve to true before getting to the test you modified.

No, actually.   In my scenario, the breakpoint is being created by load
address (via SBTarget::BreakpointCreateByAddress(addr_t address)), right
after the target has been created.   Since the process hasn't been launched
at this point yet, there are no code sections mapped, so the breakpoint
address stays non-section-relative.   My understanding is that in such a
case, LLDB should attempt to re-resolve it upon loading each new module.

> OTOH, lldb is a bit hesitant to reset raw load address breakpoints that
> aren't in any module.  After all, you have no idea, what with loading &
> unloading & the effects of ASLR, a raw address doesn't have much meaning
> run to run.

LLDB disables ASLR by default, so load addresses are usually stable.
Also, please see Ted's comment about embedded.

> In your case, you have an address that's clearly in a section, so it
> should have been handled by the if(m_addr.GetSection() ||
> m_module_filespec) test.  So the correct fix is to figure out how you ended
> up with a non-section relative address for the breakpoint.
>
> I don't want lldb to silently reset non-section relative addresses.  The
> only way those should show up is if you set the breakpoint on some code
> that was NOT in any section, maybe JIT code or something, and lldb has no
> way of knowing what to do with those addresses on re-run.  Refusing to
> re-resolve them may be too harsh, but we should at least warn about them.
> Whereas if the address is section relative, we can always do the right
> thing.

I don't see what's the problem with resolving them.  If I set a breakpoint
on a raw memory address, I expect it to be hit as soon as there's some code
mapped and executed at that location.   Anything else would be surprising
to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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


[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

2021-09-13 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn created this revision.
vadimcn added a reviewer: jingham.
vadimcn added a project: LLDB.
Herald added a subscriber: JDevlieghere.
vadimcn requested review of this revision.
Herald added a subscriber: lldb-commits.

Setting an address breakpoint unconditionally creates a location, however that 
location may not necessarily have a resolved site (e.g. if the process hasn't 
been launched yet).  Before this patch, LLDB wouldn't try to re-resolve such 
breakpoints.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109738

Files:
  lldb/source/Breakpoint/BreakpointResolverAddress.cpp
  
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py


Index: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
===
--- 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
+++ 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
@@ -79,10 +79,33 @@
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)
 
-thread = get_threads_stopped_at_breakpoint(process, breakpoint)
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
 self.assertEqual(
 len(threads), 1,
 "There should be a thread stopped at our breakpoint")
 
 # The hit count for the breakpoint should now be 2.
 self.assertEquals(breakpoint.GetHitCount(), 2)
+
+process.Kill()
+
+# Create a breakpoint again, this time using the load address
+load_address = address.GetLoadAddress(target)
+
+# Re-create the target to make sure that nothing is cached
+target = self.createTestTarget()
+breakpoint = target.BreakpointCreateByAddress(load_address)
+
+launch_info.Clear()
+launch_info.SetLaunchFlags(flags)
+
+process = target.Launch(launch_info, error)
+self.assertTrue(process, PROCESS_IS_VALID)
+
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
+self.assertEqual(
+len(threads), 1,
+"There should be a thread stopped at our breakpoint")
+
+# The hit count for the breakpoint should be 1.
+self.assertEquals(breakpoint.GetHitCount(), 1)
Index: lldb/source/Breakpoint/BreakpointResolverAddress.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -97,8 +97,12 @@
   bool re_resolve = false;
   if (m_addr.GetSection() || m_module_filespec)
 re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
-re_resolve = true;
+  else {
+BreakpointSP breakpoint = GetBreakpoint();
+if (breakpoint->GetNumLocations() == 0 ||
+breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
+  re_resolve = true;
+  }
 
   if (re_resolve)
 BreakpointResolver::ResolveBreakpoint(filter);
@@ -110,8 +114,12 @@
   bool re_resolve = false;
   if (m_addr.GetSection())
 re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
-re_resolve = true;
+  else {
+BreakpointSP breakpoint = GetBreakpoint();
+if (breakpoint->GetNumLocations() == 0 ||
+breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
+  re_resolve = true;
+  }
 
   if (re_resolve)
 BreakpointResolver::ResolveBreakpointInModules(filter, modules);
@@ -151,7 +159,7 @@
   BreakpointLocationSP loc_sp = breakpoint.GetLocationAtIndex(0);
   lldb::addr_t cur_load_location =
   m_addr.GetLoadAddress(());
-  if (cur_load_location != m_resolved_addr) {
+  if (cur_load_location != m_resolved_addr || !loc_sp->IsResolved()) {
 m_resolved_addr = cur_load_location;
 loc_sp->ClearBreakpointSite();
 loc_sp->ResolveBreakpointSite();


Index: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
===
--- lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
+++ lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
@@ -79,10 +79,33 @@
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)
 
-thread = get_threads_stopped_at_breakpoint(process, breakpoint)
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
 self.assertEqual(
 len(threads), 1,
 "There should be a thread stopped at our breakpoint")
 
 # The hit count for the breakpoint should now be 2.
 self.assertEquals(breakpoint.GetHitCount(), 2)
+
+process.Kill()
+
+# Create a breakpoint again, this time using the load address
+  

[Lldb-commits] [PATCH] D109339: Fix compilation error with older libstdc++

2021-09-13 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn updated this revision to Diff 372391.
vadimcn added a comment.

Re-formatted


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

https://reviews.llvm.org/D109339

Files:
  lldb/include/lldb/Target/ThreadPlanStack.h


Index: lldb/include/lldb/Target/ThreadPlanStack.h
===
--- lldb/include/lldb/Target/ThreadPlanStack.h
+++ lldb/include/lldb/Target/ThreadPlanStack.h
@@ -124,7 +124,8 @@
 
   void AddThread(Thread ) {
 lldb::tid_t tid = thread.GetID();
-m_plans_list.emplace(tid, thread);
+m_plans_list.emplace(std::piecewise_construct, std::forward_as_tuple(tid),
+ std::forward_as_tuple(thread));
   }
 
   bool RemoveTID(lldb::tid_t tid) {


Index: lldb/include/lldb/Target/ThreadPlanStack.h
===
--- lldb/include/lldb/Target/ThreadPlanStack.h
+++ lldb/include/lldb/Target/ThreadPlanStack.h
@@ -124,7 +124,8 @@
 
   void AddThread(Thread ) {
 lldb::tid_t tid = thread.GetID();
-m_plans_list.emplace(tid, thread);
+m_plans_list.emplace(std::piecewise_construct, std::forward_as_tuple(tid),
+ std::forward_as_tuple(thread));
   }
 
   bool RemoveTID(lldb::tid_t tid) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D109339: Fix compilation error with older libstdc++

2021-09-06 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn created this revision.
vadimcn added a reviewer: jingham.
vadimcn added a project: LLDB.
Herald added a subscriber: JDevlieghere.
vadimcn requested review of this revision.
Herald added a subscriber: lldb-commits.

Until GCC 6, std::unordered_map could not emplace non-copyable values, which 
results in this error: "error: call to implicitly-deleted copy constructor of 
'lldb_private::ThreadPlanStack'" (due to ThreadPlanStack::m_stack_mutex not 
being copyable).   Using std::piecewise_construct works around this problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109339

Files:
  lldb/include/lldb/Target/ThreadPlanStack.h


Index: lldb/include/lldb/Target/ThreadPlanStack.h
===
--- lldb/include/lldb/Target/ThreadPlanStack.h
+++ lldb/include/lldb/Target/ThreadPlanStack.h
@@ -124,7 +124,9 @@
 
   void AddThread(Thread ) {
 lldb::tid_t tid = thread.GetID();
-m_plans_list.emplace(tid, thread);
+m_plans_list.emplace(std::piecewise_construct,
+ std::forward_as_tuple(tid),
+ std::forward_as_tuple(thread));
   }
 
   bool RemoveTID(lldb::tid_t tid) {


Index: lldb/include/lldb/Target/ThreadPlanStack.h
===
--- lldb/include/lldb/Target/ThreadPlanStack.h
+++ lldb/include/lldb/Target/ThreadPlanStack.h
@@ -124,7 +124,9 @@
 
   void AddThread(Thread ) {
 lldb::tid_t tid = thread.GetID();
-m_plans_list.emplace(tid, thread);
+m_plans_list.emplace(std::piecewise_construct,
+ std::forward_as_tuple(tid),
+ std::forward_as_tuple(thread));
   }
 
   bool RemoveTID(lldb::tid_t tid) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D100503: [lldb] [client] Implement follow-fork-mode

2021-07-21 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

@mgorny  Do your changes allow debugging both the parent and child processes 
after a fork?


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

https://reviews.llvm.org/D100503

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


[Lldb-commits] [PATCH] D67793: new api class: SBFile

2020-01-10 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added inline comments.



Comment at: lldb/trunk/scripts/Python/python-typemaps.swig:481
+  PyBuffer_Release();
+  $1 = ($1_ltype) buf;
+  $2 = ($2_ltype) (size/sizeof($*1_type));

Sorry for being late to the party, but I just stumbled upon this code...  

It seems to return a pointer from a view that had just been released.  Isn't 
this kind of risky?   While most of the time buffer views point into object's 
internal memory, buffer protocol does not prohibit allocating memory just to 
fulfill the buffer request.  In which case PyBuffer_Release would be expected 
to release that memory, leaving the caller with a dangling pointer.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67793



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


[Lldb-commits] [PATCH] D52404: Prevent double import of _lldb module

2018-10-14 Thread Vadim Chugunov via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344474: Fix double import of _lldb module. (authored by 
vadimcn, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52404?vs=166627=169595#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52404

Files:
  lldb/trunk/scripts/lldb.swig
  lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp


Index: 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ 
lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -132,6 +132,9 @@
 
 InitializePythonHome();
 
+// Register _lldb as a built-in module.
+PyImport_AppendInittab("_lldb", g_swig_init_callback);
+
 // Python < 3.2 and Python >= 3.2 reversed the ordering requirements for
 // calling `Py_Initialize` and `PyEval_InitThreads`.  < 3.2 requires that you
 // call `PyEval_InitThreads` first, and >= 3.2 requires that you call it last.
Index: lldb/trunk/scripts/lldb.swig
===
--- lldb/trunk/scripts/lldb.swig
+++ lldb/trunk/scripts/lldb.swig
@@ -41,12 +41,12 @@
 */
 %define MODULEIMPORT
 "try:
-# Try a relative import first
-from . import $module
+# Try an absolute import first.  If we're being loaded from lldb,
+# _lldb should be a built-in module.
+import $module
 except ImportError:
-# Maybe absolute import will work (if we're being loaded from lldb, it
-# should).
-import $module"
+# Relative import should work if we are being loaded by Python.
+from . import $module"
 %enddef
 // These versions will not generate working python modules, so error out early.
 #if SWIG_VERSION >= 0x030009 && SWIG_VERSION < 0x030011


Index: lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -132,6 +132,9 @@
 
 InitializePythonHome();
 
+// Register _lldb as a built-in module.
+PyImport_AppendInittab("_lldb", g_swig_init_callback);
+
 // Python < 3.2 and Python >= 3.2 reversed the ordering requirements for
 // calling `Py_Initialize` and `PyEval_InitThreads`.  < 3.2 requires that you
 // call `PyEval_InitThreads` first, and >= 3.2 requires that you call it last.
Index: lldb/trunk/scripts/lldb.swig
===
--- lldb/trunk/scripts/lldb.swig
+++ lldb/trunk/scripts/lldb.swig
@@ -41,12 +41,12 @@
 */
 %define MODULEIMPORT
 "try:
-# Try a relative import first
-from . import $module
+# Try an absolute import first.  If we're being loaded from lldb,
+# _lldb should be a built-in module.
+import $module
 except ImportError:
-# Maybe absolute import will work (if we're being loaded from lldb, it
-# should).
-import $module"
+# Relative import should work if we are being loaded by Python.
+from . import $module"
 %enddef
 // These versions will not generate working python modules, so error out early.
 #if SWIG_VERSION >= 0x030009 && SWIG_VERSION < 0x030011
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D52404: Prevent double import of _lldb module

2018-10-07 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

Thanks,
What changed in SWIG 3.0.11?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52404



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


[Lldb-commits] [PATCH] D52404: Prevent double import of _lldb module

2018-10-06 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

Changing title as it looks like this is not a Windows-only problem: 
https://github.com/rust-lang/rust/issues/54126.

The above illustrates why I think this needs to be fixed: Right now LLDB cannot 
be installed correctly unless the installer can preserve symlinks.  Merely 
making a copy of LLDB files will break it, unless one is extra careful to use 
symlink-preserving copy, and I am not even sure what tool could be used on 
Windows to copy it correctly.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D52404



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


[Lldb-commits] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

2017-09-18 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

Maybe LLDB should use `qAttached` to determine if there's an active process?  
OpenOCD seems to implement 

 that one correctly.


Repository:
  rL LLVM

https://reviews.llvm.org/D37934



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


[Lldb-commits] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

2017-09-18 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

Sorry, for not catching this regression!   I've checked that attaching to a 
standard gdbserver still worked, but was not aware of the other scenarios.

Does lldb-server return 'OK' string in response to 'qfThreadInfo'?   According 
to this 
,
 'OK' is not a valid response to qfThreadInfo. (Is there a better reference for 
gdb-remote protocol?)

Also, Tamas Berghammer has already submitted a fix 
, does is 
still not resolve this issue?


Repository:
  rL LLVM

https://reviews.llvm.org/D37934



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


[Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

2017-09-17 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

FYI - this broke at least Windows, because the default behavior of 
Process::WillResume() changed from success to an error and WindowsProcess does 
not override WillResume().  Not sure what other platforms are in the same boat.


Repository:
  rL LLVM

https://reviews.llvm.org/D37651



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


[Lldb-commits] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

2017-09-15 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

This is what I see in the log:

  <  16> send packet: $qfThreadInfo#bb
  <   5> read packet: $l#6c

And here's code that generates it:  
https://github.com/gnu-mcu-eclipse/openocd/blob/b21ab1d683aaee501d45fe8a509a2043123f16fd/src/rtos/rtos.c#L370

I agree, they should have responded with "not supported", but I've never heard 
of a system that allows processes without threads, and openocd is pretty 
popular in embedded, so I think it would make sense to just support this quirk. 
  That's what gdb does anyways...


Repository:
  rL LLVM

https://reviews.llvm.org/D37934



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


[Lldb-commits] [PATCH] D37934: Fix compatibility with OpenOCD debug stub.

2017-09-15 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn created this revision.
vadimcn added a project: LLDB.

OpenOCD sends register classes as two separate  nodes, fixed parser to 
process both of them.

OpenOCD returns "l" in response to "qfThreadInfo", so IsUnsupportedResponse() 
was false and we were ending up without any threads in the process.   I think 
it's reasonable to assume that there's always at least one thread.


Repository:
  rL LLVM

https://reviews.llvm.org/D37934

Files:
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4168,7 +4168,6 @@
   std::string osabi;
   stringVec includes;
   RegisterSetMap reg_set_map;
-  XMLNode feature_node;
 };
 
 bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo _info,
@@ -4374,8 +4373,8 @@
 
 XMLNode target_node = xml_document.GetRootElement("target");
 if (target_node) {
-  XMLNode feature_node;
-  target_node.ForEachChildElement([_info, _node](
+  std::vector feature_nodes;
+  target_node.ForEachChildElement([_info, _nodes](
   const XMLNode ) -> bool {
 llvm::StringRef name = node.GetName();
 if (name == "architecture") {
@@ -4387,7 +4386,7 @@
   if (!href.empty())
 target_info.includes.push_back(href.str());
 } else if (name == "feature") {
-  feature_node = node;
+  feature_nodes.push_back(node);
 } else if (name == "groups") {
   node.ForEachChildElementWithName(
   "group", [_info](const XMLNode ) -> bool {
@@ -4423,7 +4422,7 @@
   // set the Target's architecture yet, so the ABI is also potentially
   // incorrect.
   ABISP abi_to_use_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
-  if (feature_node) {
+  for (auto feature_node : feature_nodes) {
 ParseRegisters(feature_node, target_info, this->m_register_info,
abi_to_use_sp, cur_reg_num, reg_offset);
   }
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2624,8 +2624,7 @@
  * tid.
  * Assume pid=tid=1 in such cases.
 */
-if (response.IsUnsupportedResponse() && thread_ids.size() == 0 &&
-IsConnected()) {
+if (thread_ids.size() == 0 && IsConnected()) {
   thread_ids.push_back(1);
 }
   } else {


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -4168,7 +4168,6 @@
   std::string osabi;
   stringVec includes;
   RegisterSetMap reg_set_map;
-  XMLNode feature_node;
 };
 
 bool ParseRegisters(XMLNode feature_node, GdbServerTargetInfo _info,
@@ -4374,8 +4373,8 @@
 
 XMLNode target_node = xml_document.GetRootElement("target");
 if (target_node) {
-  XMLNode feature_node;
-  target_node.ForEachChildElement([_info, _node](
+  std::vector feature_nodes;
+  target_node.ForEachChildElement([_info, _nodes](
   const XMLNode ) -> bool {
 llvm::StringRef name = node.GetName();
 if (name == "architecture") {
@@ -4387,7 +4386,7 @@
   if (!href.empty())
 target_info.includes.push_back(href.str());
 } else if (name == "feature") {
-  feature_node = node;
+  feature_nodes.push_back(node);
 } else if (name == "groups") {
   node.ForEachChildElementWithName(
   "group", [_info](const XMLNode ) -> bool {
@@ -4423,7 +4422,7 @@
   // set the Target's architecture yet, so the ABI is also potentially
   // incorrect.
   ABISP abi_to_use_sp = ABI::FindPlugin(shared_from_this(), arch_to_use);
-  if (feature_node) {
+  for (auto feature_node : feature_nodes) {
 ParseRegisters(feature_node, target_info, this->m_register_info,
abi_to_use_sp, cur_reg_num, reg_offset);
   }
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2624,8 +2624,7 @@
  * tid.
  * Assume pid=tid=1 in such cases.
 */
-if (response.IsUnsupportedResponse() && thread_ids.size() == 0 &&
-IsConnected()) {
+if (thread_ids.size() == 

[Lldb-commits] [PATCH] D27476: Install lldb Python module on Windows.

2016-12-21 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn added a comment.

Ping!   Should I ask somebody else to review?


Repository:
  rL LLVM

https://reviews.llvm.org/D27476



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


[Lldb-commits] [PATCH] D27476: Install lldb Python module on Windows.

2016-12-06 Thread Vadim Chugunov via Phabricator via lldb-commits
vadimcn created this revision.
vadimcn added reviewers: zturner, hans.
vadimcn added a subscriber: lldb-commits.
vadimcn set the repository for this revision to rL LLVM.
vadimcn added a project: LLDB.
Herald added a subscriber: mgorny.

With this change, "script import lldb" succeeds when executed in installed lldb.


Repository:
  rL LLVM

https://reviews.llvm.org/D27476

Files:
  scripts/CMakeLists.txt


Index: scripts/CMakeLists.txt
===
--- scripts/CMakeLists.txt
+++ scripts/CMakeLists.txt
@@ -11,8 +11,13 @@
 
 include(FindPythonInterp)
 
-set(SWIG_PYTHON_DIR
-  
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR})
+if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
+  set(SWIG_PYTHON_DIR
+
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR})
+else()
+  set(SWIG_PYTHON_DIR 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/site-packages)
+endif()
+
 set(SWIG_INSTALL_DIR lib${LLVM_LIBDIR_SUFFIX})
 
 # Generating the LLDB framework correctly is a bit complicated because the
@@ -47,10 +52,8 @@
 
 add_custom_target(swig_wrapper ALL DEPENDS ${LLDB_WRAP_PYTHON})
 
-# Install the LLDB python module on all operating systems (except Windows)
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
-  install(DIRECTORY ${SWIG_PYTHON_DIR} DESTINATION ${SWIG_INSTALL_DIR})
-endif()
+# Install the LLDB python module
+install(DIRECTORY ${SWIG_PYTHON_DIR} DESTINATION ${SWIG_INSTALL_DIR})
 
 # build Python modules
 add_subdirectory(Python/modules)


Index: scripts/CMakeLists.txt
===
--- scripts/CMakeLists.txt
+++ scripts/CMakeLists.txt
@@ -11,8 +11,13 @@
 
 include(FindPythonInterp)
 
-set(SWIG_PYTHON_DIR
-  ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR})
+if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
+  set(SWIG_PYTHON_DIR
+${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR})
+else()
+  set(SWIG_PYTHON_DIR ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/site-packages)
+endif()
+
 set(SWIG_INSTALL_DIR lib${LLVM_LIBDIR_SUFFIX})
 
 # Generating the LLDB framework correctly is a bit complicated because the
@@ -47,10 +52,8 @@
 
 add_custom_target(swig_wrapper ALL DEPENDS ${LLDB_WRAP_PYTHON})
 
-# Install the LLDB python module on all operating systems (except Windows)
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
-  install(DIRECTORY ${SWIG_PYTHON_DIR} DESTINATION ${SWIG_INSTALL_DIR})
-endif()
+# Install the LLDB python module
+install(DIRECTORY ${SWIG_PYTHON_DIR} DESTINATION ${SWIG_INSTALL_DIR})
 
 # build Python modules
 add_subdirectory(Python/modules)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits