Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Jim Ingham via lldb-commits]

> On Sep 20, 2017, at 4:16 PM, Leonard Mosescu  wrote:
> 
> I don't quite understand the comment about signals adding indeterminacy. No 
> signal delivery is required to test this part.  The lldb driver has a sigint 
> handler that calls SBDebugger::DispatchInputInterrupt.  But since you aren't 
> testing whether SIGINT actually calls DispatchInputInterrupt, you can just 
> call it directly.  
> 
> Agreed.
> 
> I was suggesting executing a command (with 
> SBCommandInterpreter::ExecuteCommand) for a Python command that cycles 
> calling WasInterrupted.  Then you would make another thread in Python and 
> have that thread wait a bit then call DispatchInputInterrupt,  If "wait a 
> bit" bothers you then you could have the python based command you are 
> interrupting signal the interrupting thread before going into its loop.  I 
> don't see how this would result in indeterminacy,  And it would be testing 
> exactly what you want to  test: does calling DispatchInputInterrupt cause a 
> command in flight to be interrupted.
> 
> Once you have a second thread you end up with the non-determinism I hinted to 
> (this is true regardless if you hardcode a wait, use an event or keep 
> interrupting at a fixed rate). Now this is not a deal breaker in itself, 
> after all if you go after testing async behavior it's part of the deal.
> 
> But in this case it gets a bit more complicated as far as I can tell: first, 
> DispatchInputInterrupt() is only passed on the top IO Handler, if any. So 
> DispatchInputInterrupt() through SBDebugger is not exactly the same as a real 
> input interrupt.
> 
> Second, if we want to allow the interruption of commands that are executed 
> through SBDebugger::HandleCommand() the command state machine is not a simple 
> idle -> executing (->interrupted) -> idle since you get reentrancy (the 
> command can invoke other commands, etc...). Note that in the current version, 
> the states are only tracking in CommandInterpreter::IOHandlerInputComplete() 
> which should not lead to reentrancy (I did manual testing for this - if 
> anything I'd love a way to automate testing _this_ part btw)
> 
> I got pretty far in dancing around all this, but it become clear that I was 
> not really testing the real path and I was just introducing more artificial 
> complexity. If I'm missing anything I'd be happy to be revisit this and to be 
> proven wrong.

It been a couple of years since I dug into this code - Greg’s mostly been 
maintaining it.  So I’m entirely willing to believe it’s changed in a way that 
makes my memory of how it works unhelpful, but I’ll have to do some reading up 
to see.  I’ll do that when I get a chance.

Jim

> 
> On Wed, Sep 20, 2017 at 3:51 PM, Jim Ingham via Phabricator 
> > wrote:
> jingham accepted this revision.
> jingham added a comment.
> 
> I don't quite understand the comment about signals adding indeterminacy. No 
> signal delivery is required to test this part.  The lldb driver has a sigint 
> handler that calls SBDebugger::DispatchInputInterrupt.  But since you aren't 
> testing whether SIGINT actually calls DispatchInputInterrupt, you can just 
> call it directly.  I was suggesting executing a command (with 
> SBCommandInterpreter::ExecuteCommand) for a Python command that cycles 
> calling WasInterrupted.  Then you would make another thread in Python and 
> have that thread wait a bit then call DispatchInputInterrupt,  If "wait a 
> bit" bothers you then you could have the python based command you are 
> interrupting signal the interrupting thread before going into its loop.  I 
> don't see how this would result in indeterminacy,  And it would be testing 
> exactly what you want to  test: does calling DispatchInputInterrupt cause a 
> command in flight to be interrupted.
> 
> But this change is fine.  Check it in and I'll add a test when I get a chance.
> 
> 
> https://reviews.llvm.org/D37923 
> 
> 
> 
> 

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

Re: [Lldb-commits] FreeBSD kernel debugging fixes

[Koropoff, Brian via lldb-commits]

Jason,

I'm performing address to symbol resolution after setting load addresses for 
all sections.  It correctly identifies the module
and section where the address resides, and in many cases gives correct results. 
 However, because it translates the
load address to a file address before indexing into the symtab, overlapping 
file addresses for sections in the same module
can cause the wrong name to be returned, such as returning a symbol in the bss 
section even though the address is
in the text section.

An alternative way to fix this would be to split m_file_addr_to_index into 
per-section maps, but that doesn't solve the
problem of ResolveFileAddress being unusable, or the general expectation within 
lldb that file addresses uniquely
identify something within a module.

-- Brian

From: Jason Molenda [jmole...@apple.com]
Sent: Wednesday, September 20, 2017 4:18 PM
To: Koropoff, Brian
Cc: lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] FreeBSD kernel debugging fixes

Right, we always record symbol addresses as the offset to the section that 
contains them.  The Address class in lldb is used everywhere for this.  The 
Target has a SectionLoadList which tells us where each Section is loaded in 
memory -- this is how you translate an Address object to a load address.

When sections have not been given their load addresses yet, lldb will treat 
file addresses == load addresses.  Which sounds like what you're seeing.  So we 
are often in the situation where an address->symbol lookup results in multiple 
symbols being matched; they are all overlapping at this point.

As soon as the sections are given load addresses in the Target, then this 
overlapping problem is resolved.

gdb didn't have the difference between load address and file address and we had 
to play games with shuffling things around to arbitrary addresses so they don't 
overlap.  (and from what I can recall, changing the load address of a binary in 
lldb meant going through the symbol table to update all the addresses -- we 
wanted to separate the symbol table addresses from the load addresses in a 
given target, so we came up with this system.)


Are you trying to do address->symbol resolution before you know where the 
binaries are actually loaded in the address space?  Or are you missing the part 
that sets the load addresses for the sections in the Target?  I suspect it's 
the latter.



> On Sep 20, 2017, at 4:12 PM, Koropoff, Brian  wrote:
>
> Jason,
>
> I'm setting the load addresses appropriately for all sections in my script.  
> The problem is that the symbol map
> is internally indexed by the "file address", which is the virtual address 
> that the ELF section asks to
> be loaded at, regardless of what the actual load address turns out to be:
>
> https://github.com/llvm-mirror/lldb/blob/master/source/Symbol/Symtab.cpp#L878
>
> Symbol lookup proceeds via the file address:
>
> https://github.com/llvm-mirror/lldb/blob/master/source/Core/Module.cpp#L510
>
> From what I can gather, the use of file addresses is to avoid needing to 
> recompute the symtab
> when a load address is changed.  This implementation detail means that file 
> addresses
> must be non-overlapping even if the load addresses are correctly set.  The 
> generation of synthetic
> file addresses has the added benefit of permitting offline symbolication by 
> (module, file address) pair
> without needing to know the load map, which appears to be an intended use 
> case, e.g.
> SBModule::ResolveFileAddress():
>
> https://github.com/llvm-mirror/lldb/blob/master/include/lldb/API/SBModule.h#L120
>
> Regards,
> Brian Koropoff
> Dell EMC
>
> 
> From: Jason Molenda [jmole...@apple.com]
> Sent: Wednesday, September 20, 2017 3:47 PM
> To: Koropoff, Brian
> Cc: lldb-commits@lists.llvm.org
> Subject: Re: [Lldb-commits] FreeBSD kernel debugging fixes
>
> Regarding the overlapping files -- when lldb first loads multiple binaries 
> (but does not have a running process), it doesn't know where to set the load 
> addresses of these binaries so they are all 0-based (or if they have a 
> specified load address in the object file, at that address).
>
> We rely on the dynamic linker on the system to tell us where libc.so is, and 
> then we update the target's section load list with that address.
>
> For macos kernel debugging, we have a DynamicLoaderDarwinKernel that knows 
> how to load all the modules ("kexts") at the correct addresses for the 
> program.  The user can also do this manually in command line lldb, like
>
> target modules add 
> target modules load -f  -s  address>
>
> but it is correct behavior that in the absence of being told where the 
> binaries are loaded in memory, lldb will load them all at their base address, 
> often 0 in the modern days of pic code.
>
>
> I haven't looked at the patch, but a long time ago I did hack that sounds 
> similar to yours for gdb, 

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Leonard Mosescu via lldb-commits]

Yes, using line endings as split points is somewhat arbitrary, anything
that's a reasonable compromise between interruption responsiveness and low
interrupt polling overhead would do. I feel that the lines are somewhat
nicer since arbitrary splitting may lead to confusion and/or formatting
ugliness.

On Wed, Sep 20, 2017 at 4:24 PM, Adrian McCarthy via Phabricator <
revi...@reviews.llvm.org> wrote:

> amccarth accepted this revision.
> amccarth added a comment.
>
> LGTM.
>
> But just a thought:  Is it worth doing all the work to scan for line
> endings for the interruption points?  What if, instead, it just printed the
> next _n_ characters on each iteration until the entire buffer has been
> printed.  Sure, sometimes an interruption will split a line, and sometimes
> it won't.  I'm not sure that's important for interactive usage.  It would
> mean less fiddly code, faster output (because you don't have to scan every
> character), and a zillion short lines will print as fast as a smaller
> number of longer lines that represents the same volume of text.
>
>
> https://reviews.llvm.org/D37923
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth accepted this revision.
amccarth added a comment.

LGTM.

But just a thought:  Is it worth doing all the work to scan for line endings 
for the interruption points?  What if, instead, it just printed the next _n_ 
characters on each iteration until the entire buffer has been printed.  Sure, 
sometimes an interruption will split a line, and sometimes it won't.  I'm not 
sure that's important for interactive usage.  It would mean less fiddly code, 
faster output (because you don't have to scan every character), and a zillion 
short lines will print as fast as a smaller number of longer lines that 
represents the same volume of text.


https://reviews.llvm.org/D37923



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

Re: [Lldb-commits] FreeBSD kernel debugging fixes

[Jason Molenda via lldb-commits]

Right, we always record symbol addresses as the offset to the section that 
contains them.  The Address class in lldb is used everywhere for this.  The 
Target has a SectionLoadList which tells us where each Section is loaded in 
memory -- this is how you translate an Address object to a load address.

When sections have not been given their load addresses yet, lldb will treat 
file addresses == load addresses.  Which sounds like what you're seeing.  So we 
are often in the situation where an address->symbol lookup results in multiple 
symbols being matched; they are all overlapping at this point.

As soon as the sections are given load addresses in the Target, then this 
overlapping problem is resolved.

gdb didn't have the difference between load address and file address and we had 
to play games with shuffling things around to arbitrary addresses so they don't 
overlap.  (and from what I can recall, changing the load address of a binary in 
lldb meant going through the symbol table to update all the addresses -- we 
wanted to separate the symbol table addresses from the load addresses in a 
given target, so we came up with this system.)


Are you trying to do address->symbol resolution before you know where the 
binaries are actually loaded in the address space?  Or are you missing the part 
that sets the load addresses for the sections in the Target?  I suspect it's 
the latter.



> On Sep 20, 2017, at 4:12 PM, Koropoff, Brian  wrote:
> 
> Jason,
> 
> I'm setting the load addresses appropriately for all sections in my script.  
> The problem is that the symbol map
> is internally indexed by the "file address", which is the virtual address 
> that the ELF section asks to
> be loaded at, regardless of what the actual load address turns out to be:
> 
> https://github.com/llvm-mirror/lldb/blob/master/source/Symbol/Symtab.cpp#L878
> 
> Symbol lookup proceeds via the file address:
> 
> https://github.com/llvm-mirror/lldb/blob/master/source/Core/Module.cpp#L510
> 
> From what I can gather, the use of file addresses is to avoid needing to 
> recompute the symtab
> when a load address is changed.  This implementation detail means that file 
> addresses
> must be non-overlapping even if the load addresses are correctly set.  The 
> generation of synthetic
> file addresses has the added benefit of permitting offline symbolication by 
> (module, file address) pair
> without needing to know the load map, which appears to be an intended use 
> case, e.g.
> SBModule::ResolveFileAddress():
> 
> https://github.com/llvm-mirror/lldb/blob/master/include/lldb/API/SBModule.h#L120
> 
> Regards,
> Brian Koropoff
> Dell EMC
> 
> 
> From: Jason Molenda [jmole...@apple.com]
> Sent: Wednesday, September 20, 2017 3:47 PM
> To: Koropoff, Brian
> Cc: lldb-commits@lists.llvm.org
> Subject: Re: [Lldb-commits] FreeBSD kernel debugging fixes
> 
> Regarding the overlapping files -- when lldb first loads multiple binaries 
> (but does not have a running process), it doesn't know where to set the load 
> addresses of these binaries so they are all 0-based (or if they have a 
> specified load address in the object file, at that address).
> 
> We rely on the dynamic linker on the system to tell us where libc.so is, and 
> then we update the target's section load list with that address.
> 
> For macos kernel debugging, we have a DynamicLoaderDarwinKernel that knows 
> how to load all the modules ("kexts") at the correct addresses for the 
> program.  The user can also do this manually in command line lldb, like
> 
> target modules add 
> target modules load -f  -s  address>
> 
> but it is correct behavior that in the absence of being told where the 
> binaries are loaded in memory, lldb will load them all at their base address, 
> often 0 in the modern days of pic code.
> 
> 
> I haven't looked at the patch, but a long time ago I did hack that sounds 
> similar to yours for gdb, where I would assign binaries random addresses 
> until we had connected to a live process & learned where they should be.  So 
> address -> symbol resolution would work.  It never worked great and we 
> decided to avoid doing that in lldb.
> 
> 
>> On Sep 20, 2017, at 3:41 PM, Koropoff, Brian via lldb-commits 
>>  wrote:
>> 
>> Greetings.  I'm submitting a few patches that resolve issues I
>> encountered when using lldb to symbolicate FreeBSD kernel backtraces.
>> The problems mostly centered around FreeBSD kernel modules actually
>> being relocatable (.o) ELF Files.
>> 
>> The major problems:
>> 
>> - Relocations were not being applied to the DWARF debug info despite
>>  there being code to do this.  Several issues prevented it from working:
>> 
>>  * Relocations are computed at the same time as the symbol table, but
>>in the case of split debug files, symbol table parsing always
>>redirects to the primary object file, meaning that relocations
>>would never be 

Re: [Lldb-commits] FreeBSD kernel debugging fixes

[Koropoff, Brian via lldb-commits]

Jason,

I'm setting the load addresses appropriately for all sections in my script.  
The problem is that the symbol map
is internally indexed by the "file address", which is the virtual address that 
the ELF section asks to
be loaded at, regardless of what the actual load address turns out to be:

https://github.com/llvm-mirror/lldb/blob/master/source/Symbol/Symtab.cpp#L878

Symbol lookup proceeds via the file address:

https://github.com/llvm-mirror/lldb/blob/master/source/Core/Module.cpp#L510

From what I can gather, the use of file addresses is to avoid needing to 
recompute the symtab
when a load address is changed.  This implementation detail means that file 
addresses
must be non-overlapping even if the load addresses are correctly set.  The 
generation of synthetic
file addresses has the added benefit of permitting offline symbolication by 
(module, file address) pair
without needing to know the load map, which appears to be an intended use case, 
e.g.
SBModule::ResolveFileAddress():

https://github.com/llvm-mirror/lldb/blob/master/include/lldb/API/SBModule.h#L120

Regards,
Brian Koropoff
Dell EMC


From: Jason Molenda [jmole...@apple.com]
Sent: Wednesday, September 20, 2017 3:47 PM
To: Koropoff, Brian
Cc: lldb-commits@lists.llvm.org
Subject: Re: [Lldb-commits] FreeBSD kernel debugging fixes

Regarding the overlapping files -- when lldb first loads multiple binaries (but 
does not have a running process), it doesn't know where to set the load 
addresses of these binaries so they are all 0-based (or if they have a 
specified load address in the object file, at that address).

We rely on the dynamic linker on the system to tell us where libc.so is, and 
then we update the target's section load list with that address.

For macos kernel debugging, we have a DynamicLoaderDarwinKernel that knows how 
to load all the modules ("kexts") at the correct addresses for the program.  
The user can also do this manually in command line lldb, like

target modules add 
target modules load -f  -s 

but it is correct behavior that in the absence of being told where the binaries 
are loaded in memory, lldb will load them all at their base address, often 0 in 
the modern days of pic code.


I haven't looked at the patch, but a long time ago I did hack that sounds 
similar to yours for gdb, where I would assign binaries random addresses until 
we had connected to a live process & learned where they should be.  So address 
-> symbol resolution would work.  It never worked great and we decided to avoid 
doing that in lldb.


> On Sep 20, 2017, at 3:41 PM, Koropoff, Brian via lldb-commits 
>  wrote:
>
> Greetings.  I'm submitting a few patches that resolve issues I
> encountered when using lldb to symbolicate FreeBSD kernel backtraces.
> The problems mostly centered around FreeBSD kernel modules actually
> being relocatable (.o) ELF Files.
>
> The major problems:
>
> - Relocations were not being applied to the DWARF debug info despite
>   there being code to do this.  Several issues prevented it from working:
>
>   * Relocations are computed at the same time as the symbol table, but
> in the case of split debug files, symbol table parsing always
> redirects to the primary object file, meaning that relocations
> would never be applied in the debug file.
>
>   * There's actually no guarantee that the symbol table has been
> parsed yet when trying to parse debug information.
>
>   * When actually applying relocations, it will segfault because the
> object files are not mapped with MAP_PRIVATE and PROT_WRITE.
>
> - LLDB returned invalid results when performing ordinary
>   address-to-symbol resolution. It turned out that the addresses
>   specified in the section headers were all 0, so LLDB believed all the
>   sections had overlapping "file addresses" and would sometimes
>   return a symbol from the wrong section.
>
> I rearranged some of the symbol table parsing code to ensure
> relocations would get applied consistently and added manual calls to
> make sure it happens before trying to use DWARF info, but it feels
> kind of hacky.  I'm open to suggestions for refactoring it.
>
> I solved the file address problem by computing synthetic addresses for
> the sections in object files so that they would not overlap in LLDB's
> lookup maps.
>
> With all these changes I'm able to successfully symbolicate backtraces
> that pass through FreeBSD kernel modules.  Let me know if there is a
> better/cleaner way to achieve any of these fixes.
>
> --
>
> Brian Koropoff
> Dell EMC
> <0001-ObjectFile-ELF-use-private-memory-mappings.patch><0002-ObjectFile-ELF-ensure-relocations-are-done-for-split.patch><0003-SymbolFile-DWARF-force-application-of-relocations.patch><0004-ObjectFile-ELF-create-synthetic-file-addresses-for-r.patch>___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> 

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.

I don't quite understand the comment about signals adding indeterminacy. No 
signal delivery is required to test this part.  The lldb driver has a sigint 
handler that calls SBDebugger::DispatchInputInterrupt.  But since you aren't 
testing whether SIGINT actually calls DispatchInputInterrupt, you can just call 
it directly.  I was suggesting executing a command (with 
SBCommandInterpreter::ExecuteCommand) for a Python command that cycles calling 
WasInterrupted.  Then you would make another thread in Python and have that 
thread wait a bit then call DispatchInputInterrupt,  If "wait a bit" bothers 
you then you could have the python based command you are interrupting signal 
the interrupting thread before going into its loop.  I don't see how this would 
result in indeterminacy,  And it would be testing exactly what you want to  
test: does calling DispatchInputInterrupt cause a command in flight to be 
interrupted.

But this change is fine.  Check it in and I'll add a test when I get a chance.


https://reviews.llvm.org/D37923



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

Re: [Lldb-commits] FreeBSD kernel debugging fixes

[Jason Molenda via lldb-commits]

Regarding the overlapping files -- when lldb first loads multiple binaries (but 
does not have a running process), it doesn't know where to set the load 
addresses of these binaries so they are all 0-based (or if they have a 
specified load address in the object file, at that address).

We rely on the dynamic linker on the system to tell us where libc.so is, and 
then we update the target's section load list with that address.

For macos kernel debugging, we have a DynamicLoaderDarwinKernel that knows how 
to load all the modules ("kexts") at the correct addresses for the program.  
The user can also do this manually in command line lldb, like

target modules add 
target modules load -f  -s 

but it is correct behavior that in the absence of being told where the binaries 
are loaded in memory, lldb will load them all at their base address, often 0 in 
the modern days of pic code.


I haven't looked at the patch, but a long time ago I did hack that sounds 
similar to yours for gdb, where I would assign binaries random addresses until 
we had connected to a live process & learned where they should be.  So address 
-> symbol resolution would work.  It never worked great and we decided to avoid 
doing that in lldb.


> On Sep 20, 2017, at 3:41 PM, Koropoff, Brian via lldb-commits 
>  wrote:
> 
> Greetings.  I'm submitting a few patches that resolve issues I
> encountered when using lldb to symbolicate FreeBSD kernel backtraces.
> The problems mostly centered around FreeBSD kernel modules actually
> being relocatable (.o) ELF Files.
> 
> The major problems:
> 
> - Relocations were not being applied to the DWARF debug info despite
>   there being code to do this.  Several issues prevented it from working:
> 
>   * Relocations are computed at the same time as the symbol table, but
> in the case of split debug files, symbol table parsing always
> redirects to the primary object file, meaning that relocations
> would never be applied in the debug file.
> 
>   * There's actually no guarantee that the symbol table has been
> parsed yet when trying to parse debug information.
> 
>   * When actually applying relocations, it will segfault because the
> object files are not mapped with MAP_PRIVATE and PROT_WRITE.
> 
> - LLDB returned invalid results when performing ordinary
>   address-to-symbol resolution. It turned out that the addresses
>   specified in the section headers were all 0, so LLDB believed all the
>   sections had overlapping "file addresses" and would sometimes
>   return a symbol from the wrong section.
> 
> I rearranged some of the symbol table parsing code to ensure
> relocations would get applied consistently and added manual calls to
> make sure it happens before trying to use DWARF info, but it feels
> kind of hacky.  I'm open to suggestions for refactoring it.
> 
> I solved the file address problem by computing synthetic addresses for
> the sections in object files so that they would not overlap in LLDB's
> lookup maps.
> 
> With all these changes I'm able to successfully symbolicate backtraces
> that pass through FreeBSD kernel modules.  Let me know if there is a
> better/cleaner way to achieve any of these fixes.
> 
> --
> 
> Brian Koropoff
> Dell EMC
> <0001-ObjectFile-ELF-use-private-memory-mappings.patch><0002-ObjectFile-ELF-ensure-relocations-are-done-for-split.patch><0003-SymbolFile-DWARF-force-application-of-relocations.patch><0004-ObjectFile-ELF-create-synthetic-file-addresses-for-r.patch>___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Lldb-commits] FreeBSD kernel debugging fixes

[Koropoff, Brian via lldb-commits]

Greetings.  I'm submitting a few patches that resolve issues I
encountered when using lldb to symbolicate FreeBSD kernel backtraces.
The problems mostly centered around FreeBSD kernel modules actually
being relocatable (.o) ELF Files.

The major problems:

- Relocations were not being applied to the DWARF debug info despite
  there being code to do this.  Several issues prevented it from working:

  * Relocations are computed at the same time as the symbol table, but
in the case of split debug files, symbol table parsing always
redirects to the primary object file, meaning that relocations
would never be applied in the debug file.

  * There's actually no guarantee that the symbol table has been
parsed yet when trying to parse debug information.

  * When actually applying relocations, it will segfault because the
object files are not mapped with MAP_PRIVATE and PROT_WRITE.

- LLDB returned invalid results when performing ordinary
  address-to-symbol resolution. It turned out that the addresses
  specified in the section headers were all 0, so LLDB believed all the
  sections had overlapping "file addresses" and would sometimes
  return a symbol from the wrong section.

I rearranged some of the symbol table parsing code to ensure
relocations would get applied consistently and added manual calls to
make sure it happens before trying to use DWARF info, but it feels
kind of hacky.  I'm open to suggestions for refactoring it.

I solved the file address problem by computing synthetic addresses for
the sections in object files so that they would not overlap in LLDB's
lookup maps.

With all these changes I'm able to successfully symbolicate backtraces
that pass through FreeBSD kernel modules.  Let me know if there is a
better/cleaner way to achieve any of these fixes.

--

Brian Koropoff
Dell EMC
From d923e9ff178c10f2b0c8747d09e8d13aa950df72 Mon Sep 17 00:00:00 2001
From: Brian Koropoff 
Date: Wed, 20 Sep 2017 10:44:08 -0700
Subject: [PATCH 1/4] ObjectFile:ELF: use private memory mappings

This is necessary to be able to apply relocations to DWARF info.
---
 source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 0f67ab5..e2f986b 100644
--- a/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -405,7 +405,7 @@ ObjectFile *ObjectFileELF::CreateInstance(const lldb::ModuleSP _sp,
   lldb::offset_t length) {
   if (!data_sp) {
 data_sp =
-DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset);
+DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset, true);
 if (!data_sp)
   return nullptr;
 data_offset = 0;
@@ -423,7 +423,7 @@ ObjectFile *ObjectFileELF::CreateInstance(const lldb::ModuleSP _sp,
   // Update the data to contain the entire file if it doesn't already
   if (data_sp->GetByteSize() < length) {
 data_sp =
-DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset);
+DataBufferLLVM::CreateSliceFromPath(file->GetPath(), length, file_offset, true);
 if (!data_sp)
   return nullptr;
 data_offset = 0;
-- 
2.7.5

From 698b13b8fc43036e9df034363fa8a61b4707718e Mon Sep 17 00:00:00 2001
From: Brian Koropoff 
Date: Wed, 30 Aug 2017 18:49:16 -0700
Subject: [PATCH 2/4] ObjectFile:ELF: ensure relocations are done for split
 debug symbols

---
 source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 27 ++---
 source/Plugins/ObjectFile/ELF/ObjectFileELF.h   |  7 ++-
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index e2f986b..f6ea67e 100644
--- a/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -815,7 +815,8 @@ ObjectFileELF::ObjectFileELF(const lldb::ModuleSP _sp,
 : ObjectFile(module_sp, file, file_offset, length, data_sp, data_offset),
   m_header(), m_uuid(), m_gnu_debuglink_file(), m_gnu_debuglink_crc(0),
   m_program_headers(), m_section_headers(), m_dynamic_symbols(),
-  m_filespec_ap(), m_entry_point_address(), m_arch_spec() {
+  m_filespec_ap(), m_entry_point_address(), m_arch_spec(),
+  m_did_relocations(false) {
   if (file)
 m_file = *file;
   ::memset(_header, 0, sizeof(m_header));
@@ -2797,7 +2798,8 @@ unsigned ObjectFileELF::RelocateSection(
 }
 
 unsigned ObjectFileELF::RelocateDebugSections(const ELFSectionHeader *rel_hdr,
-  user_id_t rel_id) {
+  user_id_t rel_id,
+  lldb_private::Symtab *thetab) {
   

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Zachary Turner via Phabricator via lldb-commits]

zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.

seems fine to me, I agree with the point about non-determinism, due to the 
nature of signals and the fact this is only intended to provide a best-effort 
interruption.


https://reviews.llvm.org/D37923



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

[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Leonard Mosescu via Phabricator via lldb-commits]

lemo updated this revision to Diff 116074.
lemo added a comment.

1. Added SB APIs (SBCommandInterpreter::WasInterrupted()). This allows python 
commands to query for interrupt requests.

2. I talked offline with Zach and decided that the line_iterator would require 
more tinkering to get it to work and it's not worth blocking the change for it.

3. I looked into adding a test for the SB APIs, but it would not really test 
much of the real path - which targets the _interactive_ interruption. Also, 
while I appreciate the ideal of testing everything I'm afraid that the net 
value of a test here might be negative (testing for async interruption would 
introduce some level of non determinism in the test suite and would require 
extra logic that's only used for testing). If anyone has any specific guidance 
on how to create a good test please let me know!




https://reviews.llvm.org/D37923

Files:
  include/lldb/API/SBCommandInterpreter.h
  include/lldb/Core/IOHandler.h
  include/lldb/Interpreter/CommandInterpreter.h
  scripts/interface/SBCommandInterpreter.i
  source/API/SBCommandInterpreter.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Core/Debugger.cpp
  source/Interpreter/CommandInterpreter.cpp

Index: source/Interpreter/CommandInterpreter.cpp
===
--- source/Interpreter/CommandInterpreter.cpp
+++ source/Interpreter/CommandInterpreter.cpp
@@ -546,7 +546,7 @@
   char buffer[1024];
   int num_printed =
   snprintf(buffer, 1024, "%s %s", break_regexes[i][1], "-o");
-  assert(num_printed < 1024);
+  lldbassert(num_printed < 1024);
   UNUSED_IF_ASSERT_DISABLED(num_printed);
   success =
   tbreak_regex_cmd_ap->AddRegexCommand(break_regexes[i][0], buffer);
@@ -891,7 +891,7 @@
 const lldb::CommandObjectSP _sp,
 bool can_replace) {
   if (cmd_sp.get())
-assert((this == _sp->GetCommandInterpreter()) &&
+lldbassert((this == _sp->GetCommandInterpreter()) &&
"tried to add a CommandObject from a different interpreter");
 
   if (name.empty())
@@ -913,7 +913,7 @@
 const lldb::CommandObjectSP _sp,
 bool can_replace) {
   if (cmd_sp.get())
-assert((this == _sp->GetCommandInterpreter()) &&
+lldbassert((this == _sp->GetCommandInterpreter()) &&
"tried to add a CommandObject from a different interpreter");
 
   if (!name.empty()) {
@@ -1062,7 +1062,7 @@
  lldb::CommandObjectSP _obj_sp,
  llvm::StringRef args_string) {
   if (command_obj_sp.get())
-assert((this == _obj_sp->GetCommandInterpreter()) &&
+lldbassert((this == _obj_sp->GetCommandInterpreter()) &&
"tried to add a CommandObject from a different interpreter");
 
   std::unique_ptr command_alias_up(
@@ -1839,7 +1839,7 @@
   matches.Clear();
 
   // Only max_return_elements == -1 is supported at present:
-  assert(max_return_elements == -1);
+  lldbassert(max_return_elements == -1);
   bool word_complete;
   num_command_matches = HandleCompletionMatches(
   parsed_line, cursor_index, cursor_char_position, match_start_point,
@@ -2677,6 +2677,57 @@
   return total_bytes;
 }
 
+void CommandInterpreter::StartHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eInProgress);
+  lldbassert(prev_state == CommandHandlingState::eIdle);
+}
+
+void CommandInterpreter::FinishHandlingCommand() {
+  auto prev_state = m_command_state.exchange(CommandHandlingState::eIdle);
+  lldbassert(prev_state != CommandHandlingState::eIdle);
+}
+
+bool CommandInterpreter::InterruptCommand() {
+  auto in_progress = CommandHandlingState::eInProgress;
+  return m_command_state.compare_exchange_strong(
+  in_progress, CommandHandlingState::eInterrupted);
+}
+
+bool CommandInterpreter::WasInterrupted() const {
+  return m_command_state == CommandHandlingState::eInterrupted;
+}
+
+void CommandInterpreter::PrintCommandOutput(Stream , llvm::StringRef str,
+bool interruptible) {
+  if (str.empty())
+return;
+
+  if (interruptible) {
+// Split the output into lines and poll for interrupt requests
+const char *data = str.data();
+size_t size = str.size();
+while (size > 0 && !WasInterrupted()) {
+  size_t chunk_size = 0;
+  for (; chunk_size < size; ++chunk_size) {
+lldbassert(data[chunk_size] != '\0');
+if (data[chunk_size] == '\n') {
+  ++chunk_size;
+  break;
+}
+  }
+  chunk_size = stream.Write(data, chunk_size);
+  lldbassert(size >= chunk_size);
+  data += chunk_size;
+  size -= chunk_size;
+}
+if (size > 0) {
+  stream.Printf("\n... Interrupted.\n");
+}
+  } else {
+stream.PutCString(str);
+  }
+}
+
 void 

[Lldb-commits] [lldb] r313799 - Fix warning caused by new clang::BuiltinType::Float16 added in r312794

[Ted Woodward via lldb-commits]

Author: ted
Date: Wed Sep 20 12:16:53 2017
New Revision: 313799

URL: http://llvm.org/viewvc/llvm-project?rev=313799=rev
Log:
Fix warning caused by new clang::BuiltinType::Float16 added in r312794

Modified:
lldb/trunk/source/Symbol/ClangASTContext.cpp

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=313799=313798=313799=diff
==
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Wed Sep 20 12:16:53 2017
@@ -5009,6 +5009,7 @@ lldb::Encoding ClangASTContext::GetEncod
 
 case clang::BuiltinType::Half:
 case clang::BuiltinType::Float:
+case clang::BuiltinType::Float16:
 case clang::BuiltinType::Float128:
 case clang::BuiltinType::Double:
 case clang::BuiltinType::LongDouble:


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

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Jim Ingham via lldb-commits]

> On Sep 20, 2017, at 11:25 AM, Zachary Turner  wrote:
> 
> 
> 
> On Wed, Sep 20, 2017 at 11:14 AM Jim Ingham  > wrote:
> 
> The amount of test coverage lldb has at present has much more to do with the 
> very aggressive schedules lldb has been driven by since its inception than 
> any difficulties with writing tests IMHBSEO.  
> 
> This probably applies to the folks at Apple, but I think there's a large 
> disconnect between what you guys experience and what the rest of the LLVM 
> community (and external contributors) experience. 
> 
> I think this is the opposite of true for everyone else.  I've heard it time 
> and time again from long-time LLVM developers as well as would-be 
> contributors.


W.r.t. external contributors writing tests, I think the barrier is one of 
education not difficulty.  Yeah, okay you gotta learn a little Python but I 
don’t think e is a significant barrier to serious developers, and the 
alternative would be to figure out how to do the same thing with the 
lldb_private API’s - which is always going to be harder than the SB API’s - or 
with some DSL we invent which is unlikely to be that much more straightforward, 
and which unlike learning the SB API’s will benefit you in the rest of your 
life as a developer not at all.

When I explain how to write tests to folks I get more of enthusiasm for an 
opportunity to get familiar with a practical programmatic interface to the 
debugger than complaint about the difficulty of writing the test. Admittedly, 
that’s in part because that’s how I pitch it.  But in the same vein, there 
might be some bias in your impressions if I’m right in imagining that someone 
coming to ask you how to write a Python test gets more of an earful about how 
this testing methodology sucks than helpful advice on how to write one!  

Anyway, up till about 6 months ago there was no good template for writing new 
tests.  That was because most of us working on lldb already knew how to write 
tests.  I fixed that by adding the sample test templates a little while ago.  
If you can think of a way to make these easier to use, please do so.  But in my 
experience folks who haven’t written tests have been able to grab one or the 
other of them and get a test done pretty easily.  The other barrier is learning 
how to read a test failure.  That isn’t hard but the test output is noisy and 
its easy to overlook the part of the output that tells you directly where to 
look for the failure.  I have it on my list of tasks to write up a description 
of this process but I have many things on my list of tasks.  If somebody else 
wants to add their experiences to the testing README, please do!

Jim

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

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Jim Ingham via lldb-commits]

I write lots of tests.  I don’t really think that the SB API’s are really a 
barrier to writing tests.  The lldbinline tests are trivial to write and can 
even be made just as a transcription of an lldb command line session, so the 
barrier to entry is trivial.  That is surely the easiest way to write tests for 
behavior that is available at the Command line or SB levels.  The SB tests are 
not much harder to write.  The fact that we didn’t have easy templates for 
writing tests for a while made figuring out how to get started hard.  But the 
sample_tests make it easy to get yourself to a debugger with a loaded target 
that you can poke at.  I just can’t see how a developer who can write good C++ 
code could have any difficulty write an SB API test. 

Moreover, for anything that requires complex setup, the python tests are much 
easier to write than lldb_private API tests because the SB API’s are 
specifically meant to be a streamlined way to drive the debugger.  So for 
somebody coming in to make a surgical fix who is not an LLDB expert, making 
them learn the lldb_private API’s for driving a debug session is even more work 
that the SB API’s.

The amount of test coverage lldb has at present has much more to do with the 
very aggressive schedules lldb has been driven by since its inception than any 
difficulties with writing tests IMHBSEO.  

Jim


> On Sep 20, 2017, at 10:50 AM, Zachary Turner  wrote:
> 
> On Wed, Sep 20, 2017 at 10:46 AM Jim Ingham  > wrote:
> Directly WRT testing.  I’m not against ALSO adding gtests when you add some 
> functionality.  But when your change is actually adding behaviors to lldb, 
> one of the things you need to ask yourself is if this is functionality that 
> extension developers for lldb will benefit from.  If it is, then adding 
> affordances in the SB API should be considered an essential part of a 
> complete implementation of the feature.  And I assert from experience that 
> writing tests for that feature at the SB API layer is a really good way to 
> ensure that the API’s you add make sense and actually get at the things you 
> intended to make available.
> 
> I also assert from experience that unfortunately it's also a good way to make 
> sure that the test doesn't get written in the first place :(
> 

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

[Lldb-commits] [PATCH] D37926: Fix the SIGINT handlers

[Adrian McCarthy via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL313785: Fix the SIGINT handlers (authored by amccarth).

Changed prior to commit:
  https://reviews.llvm.org/D37926?vs=115473=116035#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37926

Files:
  lldb/trunk/tools/driver/Driver.cpp
  lldb/trunk/tools/lldb-mi/MIDriverMain.cpp


Index: lldb/trunk/tools/driver/Driver.cpp
===
--- lldb/trunk/tools/driver/Driver.cpp
+++ lldb/trunk/tools/driver/Driver.cpp
@@ -9,6 +9,7 @@
 
 #include "Driver.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -1177,17 +1178,16 @@
 }
 
 void sigint_handler(int signo) {
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   if (g_driver) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   g_driver->GetDebugger().DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
   return;
 }
   }
 
-  exit(signo);
+  _exit(signo);
 }
 
 void sigtstp_handler(int signo) {
Index: lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
===
--- lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
+++ lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
@@ -33,6 +33,7 @@
 
 // Third party headers:
 #include "lldb/API/SBHostOS.h"
+#include 
 #include 
 #include 
 
@@ -72,14 +73,13 @@
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows
   signal(SIGINT, sigint_handler);
 #endif
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   CMIDriverMgr  = CMIDriverMgr::Instance();
   lldb::SBDebugger *pDebugger = rDriverMgr.DriverGetTheDebugger();
   if (pDebugger != nullptr) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   pDebugger->DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
 }
   }
 


Index: lldb/trunk/tools/driver/Driver.cpp
===
--- lldb/trunk/tools/driver/Driver.cpp
+++ lldb/trunk/tools/driver/Driver.cpp
@@ -9,6 +9,7 @@
 
 #include "Driver.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -1177,17 +1178,16 @@
 }
 
 void sigint_handler(int signo) {
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   if (g_driver) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   g_driver->GetDebugger().DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
   return;
 }
   }
 
-  exit(signo);
+  _exit(signo);
 }
 
 void sigtstp_handler(int signo) {
Index: lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
===
--- lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
+++ lldb/trunk/tools/lldb-mi/MIDriverMain.cpp
@@ -33,6 +33,7 @@
 
 // Third party headers:
 #include "lldb/API/SBHostOS.h"
+#include 
 #include 
 #include 
 
@@ -72,14 +73,13 @@
 #ifdef _WIN32 // Restore handler as it is not persistent on Windows
   signal(SIGINT, sigint_handler);
 #endif
-  static bool g_interrupt_sent = false;
+  static std::atomic_flag g_interrupt_sent = ATOMIC_FLAG_INIT;
   CMIDriverMgr  = CMIDriverMgr::Instance();
   lldb::SBDebugger *pDebugger = rDriverMgr.DriverGetTheDebugger();
   if (pDebugger != nullptr) {
-if (!g_interrupt_sent) {
-  g_interrupt_sent = true;
+if (!g_interrupt_sent.test_and_set()) {
   pDebugger->DispatchInputInterrupt();
-  g_interrupt_sent = false;
+  g_interrupt_sent.clear();
 }
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Zachary Turner via lldb-commits]

On Wed, Sep 20, 2017 at 10:46 AM Jim Ingham  wrote:

> Directly WRT testing.  I’m not against ALSO adding gtests when you add
> some functionality.  But when your change is actually adding behaviors to
> lldb, one of the things you need to ask yourself is if this is
> functionality that extension developers for lldb will benefit from.  If it
> is, then adding affordances in the SB API should be considered an essential
> part of a complete implementation of the feature.  And I assert from
> experience that writing tests for that feature at the SB API layer is a
> really good way to ensure that the API’s you add make sense and actually
> get at the things you intended to make available.
>

I also assert from experience that unfortunately it's also a good way to
make sure that the test doesn't get written in the first place :(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Jim Ingham via lldb-commits]

Directly WRT testing.  I’m not against ALSO adding gtests when you add some 
functionality.  But when your change is actually adding behaviors to lldb, one 
of the things you need to ask yourself is if this is functionality that 
extension developers for lldb will benefit from.  If it is, then adding 
affordances in the SB API should be considered an essential part of a complete 
implementation of the feature.  And I assert from experience that writing tests 
for that feature at the SB API layer is a really good way to ensure that the 
API’s you add make sense and actually get at the things you intended to make 
available.

So for anything like the change we are directly discussing, you should first 
think about how to make an SB affordance for it.  Then test that to make sure 
it makes sense.  Then if you feel like you also want to add gtests because 
there was something you couldn’t easily get through the SB API’s, please do so!

Jim

> On Sep 20, 2017, at 10:41 AM, Zachary Turner  wrote:
> 
> 
> 
> On Wed, Sep 20, 2017 at 10:33 AM Jim Ingham  > wrote:
> One of the fundamental goals of lldb is that it be an extensible debugger.  
> The extension mechanism for command line lldb all runs through the SB API 
> through either Python or C++ (though most folks choose to use Python).  We 
> know first hand that many people take advantage this mechanism to add custom 
> commands specific to their workflows.  Again, look at the xnu macros for an 
> example if you remain unconvinced of this.  Facebook also made a nice set of 
> extension commands for introspecting UI elements when debugging iOS apps.  
> And every WWDC folks come with questions about how to implement some command 
> or other.  If this doesn’t convince you also check out the effort the gdb 
> folks have made and continue to make to support the Python extensions to what 
> is purely a command-line debugger.
> 
> Not to mention that most of the powerful things you can do with breakpoint 
> callbacks are available through the SB API's, and most sophisticated data 
> formatters are written in Python.  And again based on questions and bug 
> reports we get there are many people using this facility.
> 
> I’ve said this many times before, but the notion that the SB API’s are not a 
> crucial part of the command-line lldb experience shows a fundamental 
> misperception of the design of lldb.
> 
> Jim
> 
> I don't disagree with this, only with the notion that every test is best 
> written by involving the extensibility layer in the test.
> 
> A fundamental and universal principle of good test design is writing tests 
> that test the minimal amount of code necessary to verify a piece of 
> functionality.  You don't *need* an extensibility layer to check if you can 
> stop at a breakpoint.
> 
> The fact that the extensibility mechanism provides a way of stopping at 
> breakpoints just means you should *also* have a test that says "did the 
> extensibility mechanism properly tell the debugger to stop at a breakpoint?".
> 
> And, of course you can (and should) also have full integration tests that 
> check the extensibility mechanism end-to-end.
> 
> But they just should not be the first and only line of defense.

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

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Zachary Turner via lldb-commits]

On Wed, Sep 20, 2017 at 10:33 AM Jim Ingham  wrote:

> One of the fundamental goals of lldb is that it be an extensible
> debugger.  The extension mechanism for command line lldb all runs through
> the SB API through either Python or C++ (though most folks choose to use
> Python).  We know first hand that many people take advantage this mechanism
> to add custom commands specific to their workflows.  Again, look at the xnu
> macros for an example if you remain unconvinced of this.  Facebook also
> made a nice set of extension commands for introspecting UI elements when
> debugging iOS apps.  And every WWDC folks come with questions about how to
> implement some command or other.  If this doesn’t convince you also check
> out the effort the gdb folks have made and continue to make to support the
> Python extensions to what is purely a command-line debugger.
>
> Not to mention that most of the powerful things you can do with breakpoint
> callbacks are available through the SB API's, and most sophisticated data
> formatters are written in Python.  And again based on questions and bug
> reports we get there are many people using this facility.
>
> I’ve said this many times before, but the notion that the SB API’s are not
> a crucial part of the command-line lldb experience shows a fundamental
> misperception of the design of lldb.
>
> Jim
>

I don't disagree with this, only with the notion that every test is best
written by involving the extensibility layer in the test.

A fundamental and universal principle of good test design is writing tests
that test the minimal amount of code necessary to verify a piece of
functionality.  You don't *need* an extensibility layer to check if you can
stop at a breakpoint.

The fact that the extensibility mechanism provides a way of stopping at
breakpoints just means you should *also* have a test that says "did the
extensibility mechanism properly tell the debugger to stop at a
breakpoint?".

And, of course you can (and should) also have full integration tests that
check the extensibility mechanism end-to-end.

But they just should not be the first and only line of defense.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Jim Ingham via lldb-commits]

One of the fundamental goals of lldb is that it be an extensible debugger.  The 
extension mechanism for command line lldb all runs through the SB API through 
either Python or C++ (though most folks choose to use Python).  We know first 
hand that many people take advantage this mechanism to add custom commands 
specific to their workflows.  Again, look at the xnu macros for an example if 
you remain unconvinced of this.  Facebook also made a nice set of extension 
commands for introspecting UI elements when debugging iOS apps.  And every WWDC 
folks come with questions about how to implement some command or other.  If 
this doesn’t convince you also check out the effort the gdb folks have made and 
continue to make to support the Python extensions to what is purely a 
command-line debugger.

Not to mention that most of the powerful things you can do with breakpoint 
callbacks are available through the SB API's, and most sophisticated data 
formatters are written in Python.  And again based on questions and bug reports 
we get there are many people using this facility.

I’ve said this many times before, but the notion that the SB API’s are not a 
crucial part of the command-line lldb experience shows a fundamental 
misperception of the design of lldb.

Jim

> On Sep 20, 2017, at 7:37 AM, Zachary Turner  wrote:
> 
> 
> 
> On Tue, Sep 19, 2017 at 7:12 PM Jason Molenda  > wrote:
> 
> 
> > On Sep 19, 2017, at 6:57 PM, Zachary Turner via lldb-commits 
> > > wrote:
> >
> >
> >
> > After some additional thought, I stilled think testing below the sb api 
> > layer is more appropriate.
> >   While Xcode might be going through the SB api, users of upstream lldb 
> > (which is what we supposed to be testing upstream) are not.
> 
> 
> One thing to keep in mind is that while Xcode is the IDE using lldb that 
> Apple directly supports, there are many other IDEs that are using lldb today. 
>  I don't know for certain that all these are using SB API, but a partial list 
> of additional IDEs in addition to Xcode includes CLion ("A Cross-Platform IDE 
> for C and C+ by JetBrains"), Nuclide (atom based IDE), Android Studio, Visual 
> Code Studio, and Eclipse (I know this one is done via lldb-mi).
> 
> Command line lldb is the interface I personally tend to prefer, but it is not 
> the interface that the vast, vast majority of our users use when debugging 
> with lldb.  SB API is.  The lldb driver is more akin to one front-end among 
> many, and an odd one, given that it doesn't go through the SB API itself.
> 
> Definitely, but that's also one reason I think it's so important to split the 
> tests up.  Right now there is almost no coverage of not going through the SB 
> API, and yet that is the interface that we, as the upstream open-source 
> project, provide to users.  And while it may not be the interface that the 
> vast majority of users *currently* use, I think if we compared the number of 
> people using GDB to the number of people using LLDB, it paints a different 
> picture.  Namely, a picture of how many users *could* be using LLDB.  
> 
> I don't want to break any existing users of LLDB obviously, including those 
> that go through the SB API, but at the same time the primary product that 
> comes out of the open-source project is, in fact, a command line debugger, 
> and we should strive to make that experience as good as possible.
> 
> I believe if the tests are split into private tests and public API tests you 
> get the best of both worlds.  I would even go farther to say that the whole 
> is greater than the sum of parts, because it makes it easy to write tests for 
> any new api that comes along, and vastly reduces the barrier to entry to 
> writing new tests, which I still believe is one of the most critical bugs 
> facing the project today.

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

Re: [Lldb-commits] [lldb] r313655 - Re-land r313210 - Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

[Ed Maste via lldb-commits]

On 19 September 2017 at 14:07, Adrian McCarthy via lldb-commits
 wrote:
> Author: amccarth
> Date: Tue Sep 19 11:07:33 2017
> New Revision: 313655
>
> URL: http://llvm.org/viewvc/llvm-project?rev=313655=rev
> Log:
> Re-land r313210 - Fix for bug 34532 - A few rough corners related to 
> post-mortem debugging (core/minidump)

Thanks, tests look good on FreeBSD now -- as good as they have
recently, at least (four failing tests, four timeouts, one error, and
a few unexpected successes).
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r313726 - Signal polling is supported with pselect (re-land r313704 without a Windows breakage)

[Eugene Zemtsov via lldb-commits]

Author: eugene
Date: Tue Sep 19 23:56:46 2017
New Revision: 313726

URL: http://llvm.org/viewvc/llvm-project?rev=313726=rev
Log:
Signal polling is supported with pselect (re-land r313704 without a Windows 
breakage)

Older Android API levels don't have ppoll, but LLDB works just fine,
since on Android it always uses pselect anyway.

Modified:
lldb/trunk/include/lldb/Host/MainLoop.h
lldb/trunk/source/Host/common/MainLoop.cpp

Modified: lldb/trunk/include/lldb/Host/MainLoop.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/MainLoop.h?rev=313726=313725=313726=diff
==
--- lldb/trunk/include/lldb/Host/MainLoop.h (original)
+++ lldb/trunk/include/lldb/Host/MainLoop.h Tue Sep 19 23:56:46 2017
@@ -15,7 +15,11 @@
 #include "llvm/ADT/DenseMap.h"
 #include 
 
-#if !HAVE_PPOLL && !HAVE_SYS_EVENT_H
+#ifdef __ANDROID__
+#define FORCE_PSELECT
+#endif
+
+#if !HAVE_PPOLL && !HAVE_SYS_EVENT_H && !defined(FORCE_PSELECT)
 #define SIGNAL_POLLING_UNSUPPORTED 1
 #endif
 

Modified: lldb/trunk/source/Host/common/MainLoop.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MainLoop.cpp?rev=313726=313725=313726=diff
==
--- lldb/trunk/source/Host/common/MainLoop.cpp (original)
+++ lldb/trunk/source/Host/common/MainLoop.cpp Tue Sep 19 23:56:46 2017
@@ -38,10 +38,6 @@
 #define POLL poll
 #endif
 
-#ifdef __ANDROID__
-#define FORCE_PSELECT
-#endif
-
 #if SIGNAL_POLLING_UNSUPPORTED
 #ifdef LLVM_ON_WIN32
 typedef int sigset_t;


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