Re: [lldb-dev] Failure connecting to lldb-server running in local virtual machine.

2017-02-21 Thread Howard Hellyer via lldb-dev
Greg Clayton  wrote on 16/02/2017 17:17:41:

> We seem to have a race condition here. Any help figuring out what 
> that might be would be great.

I think I've now identified the problem.

Normally in lldb-server when the child lldb-server process is launched 
inside GDBRemoteCommunication::StartDebugserverProcess the parent 
lldb-server waits for it to start up and write back the port is has 
selected to listen on a pipe.
When lldb-server specifies a port for the child to listen on (when we 
start lldb-server with -P  or -m  and -M ) then it 
doesn't bother setting up that pipe as it knows what the port will be. 
However not listening means the parent can reply to the lldb client with 
the port number before the debugserver process is actually up and and 
listening on that port.

Simply making sure the pipe is always setup and used to read the port 
number from (even when lldb-server knows what it will be) ensures the 
child process is ready and allows me to connect reliably with an 
unmodified client lldb.

The one second delay in the client was fixing things as inside 
PlatformRemoteGDBServer::DebugProcess there is a retry to ConnectRemote 
and the delay gave time for the child process to begin before the retry. 
This also explains why more people weren't seeing this problem as it only 
applies if you use the -P or -m/-M options. I think I only found those by 
searching the code, they don't appear in the help for lldb-server when you 
run it and I didn't manage to find any docs.

Unless anyone points out a major flaw in the logic above I'll tidy up my 
fix and put a patch up on https://reviews.llvm.org/ in the next few days. 
The code for listening on a pipe from the child process is slightly 
different when __APPLE__ is defined so I need to verify the change on Mac 
and Linux. I'll probably just always read back the port and if it comes 
back as an unexpected value that may need to be a new error case.

Would you mind if I also raised a patch to update the documentation to 
include the -P and -m/-M options? (Both in the help when you run 
lldb-server and on www/remote.html.) I originally came to this problem 
because another team couldn't connect to a remote system with only certain 
ports open and found them the options they needed by reading the source 
and I think doing things like connecting to processes in a VM or docker 
container where these options are needed are becoming more common.
Thanks,

Howard.

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


[lldb-dev] Too many ModuleSP references

2017-02-21 Thread Pavel Labath via lldb-dev
Hello all,

I've been debugging the newly added TestStepOverBreakpoint.py, which
has been failing on windows, for a very prosaic reason: after the test
completes, we are unable to run "make clean" because lldb still holds
the file open.

After some debugging, I've found that this happens because the test
case stores the SBBreakpoint object in a member variable of the python
test case class. The breakpoint class ends up transitively holding a
reference to the main executable module, which prevents the module
from being garbage-collected when the target is destroyed.

For reference, the full ownership chain is something like:
StepOverBreakpointTestCase(python) => SBBreakpoint => Breakpoint =>
BreakpointLocation => LLVMUserExpression => IRExecutionUnit =>
SymbolContext => Module.

To get the test working, we need to break this chain somewhere. A
couple of places I see are:
- BreakpointLocation: Remove the compiled expression reference when
the target is destroyed (AFAICS, it is used as a cache to avoid
recomputing the expression every time. It can be theoretically
recomputed if needed, but that shouldn't be necessary as the target is
destroyed anyway)

- SBBreakpoint: make SBBreakpoint hold a weak_ptr to the Breakpoint
object. When the target is destroyed, the SBBreakpoint object becomes
invalid (One doesn't cannot do anything useful with the breakpoint
once the target has been deleted anyway).

- StepOverBreakpointTestCase: Have the test not store the breakpoints
in the test case object. Basically, declare that this is not a bug,
and it's the users responsibility to clean up necessary objects.

Any thoughts on what is the appropriate solution here?

cheers,
pavel
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Zachary Turner via lldb-dev
No comments on this specifically, but +1 to reducing shared_ptr usage in
general. We use way too many, and often it feels like shared_ptr is used as
a way to avoid having to think about ownership, which leads to more
problems than it solves imo
On Tue, Feb 21, 2017 at 9:03 AM Pavel Labath via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> Hello all,
>
> I've been debugging the newly added TestStepOverBreakpoint.py, which
> has been failing on windows, for a very prosaic reason: after the test
> completes, we are unable to run "make clean" because lldb still holds
> the file open.
>
> After some debugging, I've found that this happens because the test
> case stores the SBBreakpoint object in a member variable of the python
> test case class. The breakpoint class ends up transitively holding a
> reference to the main executable module, which prevents the module
> from being garbage-collected when the target is destroyed.
>
> For reference, the full ownership chain is something like:
> StepOverBreakpointTestCase(python) => SBBreakpoint => Breakpoint =>
> BreakpointLocation => LLVMUserExpression => IRExecutionUnit =>
> SymbolContext => Module.
>
> To get the test working, we need to break this chain somewhere. A
> couple of places I see are:
> - BreakpointLocation: Remove the compiled expression reference when
> the target is destroyed (AFAICS, it is used as a cache to avoid
> recomputing the expression every time. It can be theoretically
> recomputed if needed, but that shouldn't be necessary as the target is
> destroyed anyway)
>
> - SBBreakpoint: make SBBreakpoint hold a weak_ptr to the Breakpoint
> object. When the target is destroyed, the SBBreakpoint object becomes
> invalid (One doesn't cannot do anything useful with the breakpoint
> once the target has been deleted anyway).
>
> - StepOverBreakpointTestCase: Have the test not store the breakpoints
> in the test case object. Basically, declare that this is not a bug,
> and it's the users responsibility to clean up necessary objects.
>
> Any thoughts on what is the appropriate solution here?
>
> cheers,
> pavel
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Jim Ingham via lldb-dev
Thanks for digging into this.

Maybe a better option would be to have IRExecutionUnit hold a 
SymbolContextScope, rather than a SymbolContext.  We started out with 
SymbolContext's because they were convenient but they hold too many things 
alive so when we run into SymbolContext's we've been converting them to the 
SymbolContextScope's when possible.  I can't see any need for the 
IRExecutionUnit to hold the module alive, so long as it handles failure to get 
the SymbolContext gracefully, which shouldn't be hard.

Jim

> On Feb 21, 2017, at 9:03 AM, Pavel Labath via lldb-dev 
>  wrote:
> 
> Hello all,
> 
> I've been debugging the newly added TestStepOverBreakpoint.py, which
> has been failing on windows, for a very prosaic reason: after the test
> completes, we are unable to run "make clean" because lldb still holds
> the file open.
> 
> After some debugging, I've found that this happens because the test
> case stores the SBBreakpoint object in a member variable of the python
> test case class. The breakpoint class ends up transitively holding a
> reference to the main executable module, which prevents the module
> from being garbage-collected when the target is destroyed.
> 
> For reference, the full ownership chain is something like:
> StepOverBreakpointTestCase(python) => SBBreakpoint => Breakpoint =>
> BreakpointLocation => LLVMUserExpression => IRExecutionUnit =>
> SymbolContext => Module.
> 
> To get the test working, we need to break this chain somewhere. A
> couple of places I see are:
> - BreakpointLocation: Remove the compiled expression reference when
> the target is destroyed (AFAICS, it is used as a cache to avoid
> recomputing the expression every time. It can be theoretically
> recomputed if needed, but that shouldn't be necessary as the target is
> destroyed anyway)
> 
> - SBBreakpoint: make SBBreakpoint hold a weak_ptr to the Breakpoint
> object. When the target is destroyed, the SBBreakpoint object becomes
> invalid (One doesn't cannot do anything useful with the breakpoint
> once the target has been deleted anyway).
> 
> - StepOverBreakpointTestCase: Have the test not store the breakpoints
> in the test case object. Basically, declare that this is not a bug,
> and it's the users responsibility to clean up necessary objects.
> 
> Any thoughts on what is the appropriate solution here?
> 
> cheers,
> pavel
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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


Re: [lldb-dev] About lldbHost

2017-02-21 Thread Zachary Turner via lldb-dev
On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton  wrote:

>
> The other thing is on Mac we add new flags to mmap that allow us not to
> crash if the backing store (network mount) goes away. There is also a flag
> that says "if code signature is borked, please still allow me to read the
> file without crashing. That functionality will need to be ported into the
> LLVM code somehow so we don't start crashing again. Previously we would
> crash if someone would do:
>
> (lldb) file /tmp/invalid_codesig_app
>
> And also if the network mounted share would go away on something that was
> mmap'ed and someone would touch any byte within it. Our version of mmap
> works around this and we need that to be in any version we adopt.
>
> Greg
>

Hi Greg,

I tried to get a change into LLVM to add these two flags.  However, I
started getting some buildbot failures

that only occurred on OSX with the error message "The operation is not
permitted".  Since it was only on OSX, and since it mentioned permissions,
I suspect that MAP_RESILIENT_CODESIGN and MAP_RESILIENT_MEDIA are causing a
problem.  Do you know what the requirements of using these two flags are?
And why they might fail?

I'm guessing you can reproduce my issue locally by checking out up to r295769
and then running "ninja check-llvm".  How come LLDB doesn't fail but LLVM
does when using these flags?
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] About lldbHost

2017-02-21 Thread Jim Ingham via lldb-dev
mman.h, which defines these flags, says:

 * The MAP_RESILIENT_* flags can be used when the caller wants to map some
 * possibly unreliable memory and be able to access it safely, possibly
 * getting the wrong contents rather than raising any exception.
 * For safety reasons, such mappings have to be read-only (PROT_READ access
 * only).
 *
 * MAP_RESILIENT_CODESIGN:
 *  accessing this mapping will not generate code-signing violations,
 *  even if the contents are tainted.
 * MAP_RESILIENT_MEDIA:
 *  accessing this mapping will not generate an exception if the contents
 *  are not available (unreachable removable or remote media, access beyond
 *  end-of-file, ...).  Missing contents will be replaced with zeroes.
 */

Are you trying to use them for read-write access?  That's all I can see.

Jim

 
> On Feb 21, 2017, at 1:47 PM, Zachary Turner via lldb-dev 
>  wrote:
> 
> 
> 
> On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton  wrote:
> 
> The other thing is on Mac we add new flags to mmap that allow us not to crash 
> if the backing store (network mount) goes away. There is also a flag that 
> says "if code signature is borked, please still allow me to read the file 
> without crashing. That functionality will need to be ported into the LLVM 
> code somehow so we don't start crashing again. Previously we would crash if 
> someone would do:
> 
> (lldb) file /tmp/invalid_codesig_app
> 
> And also if the network mounted share would go away on something that was 
> mmap'ed and someone would touch any byte within it. Our version of mmap works 
> around this and we need that to be in any version we adopt.
> 
> Greg
> 
> Hi Greg,
> 
> I tried to get a change into LLVM to add these two flags.  However, I started 
> getting some buildbot failures  that only occurred on OSX with the error 
> message "The operation is not permitted".  Since it was only on OSX, and 
> since it mentioned permissions, I suspect that MAP_RESILIENT_CODESIGN and 
> MAP_RESILIENT_MEDIA are causing a problem.  Do you know what the requirements 
> of using these two flags are?  And why they might fail?
> 
> I'm guessing you can reproduce my issue locally by checking out up to r295769 
> and then running "ninja check-llvm".  How come LLDB doesn't fail but LLVM 
> does when using these flags?
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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


Re: [lldb-dev] About lldbHost

2017-02-21 Thread Zachary Turner via lldb-dev
I bet that's it.  The 4 failing tests are:

 LLVM-Unit.Support/SupportTests.FileOutputBuffer.Test
 LLVM-Unit.Support/SupportTests.FileSystemTest.FileMapping
 LLVM.DebugInfo/PDB.pdbdump-readwrite.test
 LLVM.DebugInfo/PDB.pdbdump-write.test

All of which appear to be related to writing.  It looks like the bug is
present in LLDB as well, but it just never creates a writable mapping so
the bug is never encountered.

Thanks!

On Tue, Feb 21, 2017 at 2:02 PM Jim Ingham  wrote:

> mman.h, which defines these flags, says:
>
>  * The MAP_RESILIENT_* flags can be used when the caller wants to map some
>  * possibly unreliable memory and be able to access it safely, possibly
>  * getting the wrong contents rather than raising any exception.
>  * For safety reasons, such mappings have to be read-only (PROT_READ access
>  * only).
>  *
>  * MAP_RESILIENT_CODESIGN:
>  *  accessing this mapping will not generate code-signing violations,
>  *  even if the contents are tainted.
>  * MAP_RESILIENT_MEDIA:
>  *  accessing this mapping will not generate an exception if the
> contents
>  *  are not available (unreachable removable or remote media, access
> beyond
>  *  end-of-file, ...).  Missing contents will be replaced with zeroes.
>  */
>
> Are you trying to use them for read-write access?  That's all I can see.
>
> Jim
>
>
> > On Feb 21, 2017, at 1:47 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> >
> >
> > On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton 
> wrote:
> >
> > The other thing is on Mac we add new flags to mmap that allow us not to
> crash if the backing store (network mount) goes away. There is also a flag
> that says "if code signature is borked, please still allow me to read the
> file without crashing. That functionality will need to be ported into the
> LLVM code somehow so we don't start crashing again. Previously we would
> crash if someone would do:
> >
> > (lldb) file /tmp/invalid_codesig_app
> >
> > And also if the network mounted share would go away on something that
> was mmap'ed and someone would touch any byte within it. Our version of mmap
> works around this and we need that to be in any version we adopt.
> >
> > Greg
> >
> > Hi Greg,
> >
> > I tried to get a change into LLVM to add these two flags.  However, I
> started getting some buildbot failures  that only occurred on OSX with the
> error message "The operation is not permitted".  Since it was only on OSX,
> and since it mentioned permissions, I suspect that MAP_RESILIENT_CODESIGN
> and MAP_RESILIENT_MEDIA are causing a problem.  Do you know what the
> requirements of using these two flags are?  And why they might fail?
> >
> > I'm guessing you can reproduce my issue locally by checking out up to
> r295769 and then running "ninja check-llvm".  How come LLDB doesn't fail
> but LLVM does when using these flags?
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] About lldbHost

2017-02-21 Thread Zachary Turner via lldb-dev
(Correction: I just double checked and the bug is not present in LLDB)

On Tue, Feb 21, 2017 at 2:05 PM Zachary Turner  wrote:

> I bet that's it.  The 4 failing tests are:
>
>  LLVM-Unit.Support/SupportTests.FileOutputBuffer.Test
>  LLVM-Unit.Support/SupportTests.FileSystemTest.FileMapping
>  LLVM.DebugInfo/PDB.pdbdump-readwrite.test
>  LLVM.DebugInfo/PDB.pdbdump-write.test
>
> All of which appear to be related to writing.  It looks like the bug is
> present in LLDB as well, but it just never creates a writable mapping so
> the bug is never encountered.
>
> Thanks!
>
> On Tue, Feb 21, 2017 at 2:02 PM Jim Ingham  wrote:
>
> mman.h, which defines these flags, says:
>
>  * The MAP_RESILIENT_* flags can be used when the caller wants to map some
>  * possibly unreliable memory and be able to access it safely, possibly
>  * getting the wrong contents rather than raising any exception.
>  * For safety reasons, such mappings have to be read-only (PROT_READ access
>  * only).
>  *
>  * MAP_RESILIENT_CODESIGN:
>  *  accessing this mapping will not generate code-signing violations,
>  *  even if the contents are tainted.
>  * MAP_RESILIENT_MEDIA:
>  *  accessing this mapping will not generate an exception if the
> contents
>  *  are not available (unreachable removable or remote media, access
> beyond
>  *  end-of-file, ...).  Missing contents will be replaced with zeroes.
>  */
>
> Are you trying to use them for read-write access?  That's all I can see.
>
> Jim
>
>
> > On Feb 21, 2017, at 1:47 PM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> >
> >
> > On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton 
> wrote:
> >
> > The other thing is on Mac we add new flags to mmap that allow us not to
> crash if the backing store (network mount) goes away. There is also a flag
> that says "if code signature is borked, please still allow me to read the
> file without crashing. That functionality will need to be ported into the
> LLVM code somehow so we don't start crashing again. Previously we would
> crash if someone would do:
> >
> > (lldb) file /tmp/invalid_codesig_app
> >
> > And also if the network mounted share would go away on something that
> was mmap'ed and someone would touch any byte within it. Our version of mmap
> works around this and we need that to be in any version we adopt.
> >
> > Greg
> >
> > Hi Greg,
> >
> > I tried to get a change into LLVM to add these two flags.  However, I
> started getting some buildbot failures  that only occurred on OSX with the
> error message "The operation is not permitted".  Since it was only on OSX,
> and since it mentioned permissions, I suspect that MAP_RESILIENT_CODESIGN
> and MAP_RESILIENT_MEDIA are causing a problem.  Do you know what the
> requirements of using these two flags are?  And why they might fail?
> >
> > I'm guessing you can reproduce my issue locally by checking out up to
> r295769 and then running "ninja check-llvm".  How come LLDB doesn't fail
> but LLVM does when using these flags?
> > ___
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Greg Clayton via lldb-dev

> On Feb 21, 2017, at 9:10 AM, Zachary Turner via lldb-dev 
>  wrote:
> 
> No comments on this specifically, but +1 to reducing shared_ptr usage in 
> general. We use way too many, and often it feels like shared_ptr is used as a 
> way to avoid having to think about ownership, which leads to more problems 
> than it solves imo

We do a pretty good job of using shared pointers where they are needed, so I 
don't agree with the above statement. We have strong memory models in LLDB and 
certain things need to keep certain things alive. LLVM is quite the wild west 
with regard to memory management, so I don't want to base any changes on it. We 
just need to use std::weak_ptr when needed. There should be no two items that 
contain strong references to each other. 

In general think about what should keep things alive. If I have a SBModule 
variable, I should expect that it won't let my module go away. If I have a 
SBTarget, I would expect it to keep the target around as long as I have a 
reference. Things that belong to any of these items, like breakpoints, 
watchpoints, compile units, functions, shouldn't keep the module or target 
around.

> On Tue, Feb 21, 2017 at 9:03 AM Pavel Labath via lldb-dev 
> mailto:lldb-dev@lists.llvm.org>> wrote:
> Hello all,
> 
> I've been debugging the newly added TestStepOverBreakpoint.py, which
> has been failing on windows, for a very prosaic reason: after the test
> completes, we are unable to run "make clean" because lldb still holds
> the file open.
> 
> After some debugging, I've found that this happens because the test
> case stores the SBBreakpoint object in a member variable of the python
> test case class. The breakpoint class ends up transitively holding a
> reference to the main executable module, which prevents the module
> from being garbage-collected when the target is destroyed.
> 
> For reference, the full ownership chain is something like:
> StepOverBreakpointTestCase(python) => SBBreakpoint => Breakpoint =>
> BreakpointLocation => LLVMUserExpression => IRExecutionUnit =>
> SymbolContext => Module.
> 
> To get the test working, we need to break this chain somewhere. A
> couple of places I see are:
> - BreakpointLocation: Remove the compiled expression reference when
> the target is destroyed (AFAICS, it is used as a cache to avoid
> recomputing the expression every time. It can be theoretically
> recomputed if needed, but that shouldn't be necessary as the target is
> destroyed anyway)

We should absolutely be clearing these useless expressions when the process is 
killed. Compiling expressions take 300-500 ms at the very least and we do cache 
the breakpoint expressions in each location, which is good for the lifetime of 
the process, but these locations should go away when the process goes away or 
execs.

> 
> - SBBreakpoint: make SBBreakpoint hold a weak_ptr to the Breakpoint
> object. When the target is destroyed, the SBBreakpoint object becomes
> invalid (One doesn't cannot do anything useful with the breakpoint
> once the target has been deleted anyway).

This is also valid. If the breakpoint is still around, you will be able to use 
it as the weak_ptr will make a shared pointer with a valid value, and if it 
isn't it won't be able to do anything anyway.

> 
> - StepOverBreakpointTestCase: Have the test not store the breakpoints
> in the test case object. Basically, declare that this is not a bug,
> and it's the users responsibility to clean up necessary objects.

It would be nice to avoid this.

> Any thoughts on what is the appropriate solution here?


So I vote for:
1 - remove all breakpoint expressions when the target stops and the locations 
becomes unresolved.
2 - SBBreakpoint, SBBreakpointLocation, SBWatchpoint and can switch over to 
using std::weak_ptr

The other fix is that might be worth it is to make a SymbolContextRef, kind of 
like we have the ExecutionContext (with shared pointers) and 
ExecutionContextRef (with weak pointers). We would then have things like 
IRExecutionUnit switch over to use a SymbolContextRef. We should also look for 
other places where people are storing SymbolContext as member variables and 
possibly switch them over to use SymbolContextRef.

class SymbolContextRef
{
  lldb::TargetWP target_wp; ///< The Target for a given query
  lldb::ModuleWP module_wp; ///< The Module for a given query
  CompileUnit *comp_unit;   ///< The CompileUnit for a given query
  Function *function;   ///< The Function for a given query
  Block *block; ///< The Block for a given query
  LineEntry line_entry; ///< The LineEntry for a given query
  Symbol *symbol;   ///< The Symbol for a given query
  Variable *variable;   ///< The global variable matching the given query
};
 
Then a SymbolContext would add a constructor that takes a SymbolContextRef:

SymbolContextRef sym_ctx_ref = ...;

Anytime you want to use a SymbolContextRef, you first must turn it into a 
SymbolContext:

SymbolContext sym_ctx(sym_ctx_ref)

Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Greg Clayton via lldb-dev

> On Feb 21, 2017, at 9:36 AM, Jim Ingham via lldb-dev 
>  wrote:
> 
> Thanks for digging into this.
> 
> Maybe a better option would be to have IRExecutionUnit hold a 
> SymbolContextScope, rather than a SymbolContext.  We started out with 
> SymbolContext's because they were convenient but they hold too many things 
> alive so when we run into SymbolContext's we've been converting them to the 
> SymbolContextScope's when possible.  I can't see any need for the 
> IRExecutionUnit to hold the module alive, so long as it handles failure to 
> get the SymbolContext gracefully, which shouldn't be hard.

SymbolContextScope doesn't hold ownership as it is just a pointer. This pointer 
would go bad if the module goes away and it would crash when you tried to use 
it. See my other email for another approach with SymbolContextRef.

Greg

> 
> Jim
> 
>> On Feb 21, 2017, at 9:03 AM, Pavel Labath via lldb-dev 
>>  wrote:
>> 
>> Hello all,
>> 
>> I've been debugging the newly added TestStepOverBreakpoint.py, which
>> has been failing on windows, for a very prosaic reason: after the test
>> completes, we are unable to run "make clean" because lldb still holds
>> the file open.
>> 
>> After some debugging, I've found that this happens because the test
>> case stores the SBBreakpoint object in a member variable of the python
>> test case class. The breakpoint class ends up transitively holding a
>> reference to the main executable module, which prevents the module
>> from being garbage-collected when the target is destroyed.
>> 
>> For reference, the full ownership chain is something like:
>> StepOverBreakpointTestCase(python) => SBBreakpoint => Breakpoint =>
>> BreakpointLocation => LLVMUserExpression => IRExecutionUnit =>
>> SymbolContext => Module.
>> 
>> To get the test working, we need to break this chain somewhere. A
>> couple of places I see are:
>> - BreakpointLocation: Remove the compiled expression reference when
>> the target is destroyed (AFAICS, it is used as a cache to avoid
>> recomputing the expression every time. It can be theoretically
>> recomputed if needed, but that shouldn't be necessary as the target is
>> destroyed anyway)
>> 
>> - SBBreakpoint: make SBBreakpoint hold a weak_ptr to the Breakpoint
>> object. When the target is destroyed, the SBBreakpoint object becomes
>> invalid (One doesn't cannot do anything useful with the breakpoint
>> once the target has been deleted anyway).
>> 
>> - StepOverBreakpointTestCase: Have the test not store the breakpoints
>> in the test case object. Basically, declare that this is not a bug,
>> and it's the users responsibility to clean up necessary objects.
>> 
>> Any thoughts on what is the appropriate solution here?
>> 
>> cheers,
>> pavel
>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> 
> ___
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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


Re: [lldb-dev] About lldbHost

2017-02-21 Thread Greg Clayton via lldb-dev

> On Feb 21, 2017, at 1:47 PM, Zachary Turner  wrote:
> 
> 
> 
> On Wed, Feb 15, 2017 at 11:02 AM Greg Clayton  > wrote:
> 
> The other thing is on Mac we add new flags to mmap that allow us not to crash 
> if the backing store (network mount) goes away. There is also a flag that 
> says "if code signature is borked, please still allow me to read the file 
> without crashing. That functionality will need to be ported into the LLVM 
> code somehow so we don't start crashing again. Previously we would crash if 
> someone would do:
> 
> (lldb) file /tmp/invalid_codesig_app
> 
> And also if the network mounted share would go away on something that was 
> mmap'ed and someone would touch any byte within it. Our version of mmap works 
> around this and we need that to be in any version we adopt.
> 
> Greg
> 
> Hi Greg,
> 
> I tried to get a change into LLVM to add these two flags.  However, I started 
> getting some buildbot failures 
> 
>   that only occurred on OSX with the error message "The operation is not 
> permitted".  Since it was only on OSX, and since it mentioned permissions, I 
> suspect that MAP_RESILIENT_CODESIGN and MAP_RESILIENT_MEDIA are causing a 
> problem.  Do you know what the requirements of using these two flags are?  
> And why they might fail?

We don't seem to have problems with them in LLDB. Not sure how they are being 
used in LLVM/Clang and why that would cause any problems. You might check the 
system console and see if the sandbox is not letting you use those. We haven't 
had any problems with them in LLDB. Maybe some bots are running an older 
version of the OS that doesn't support those?

> 
> I'm guessing you can reproduce my issue locally by checking out up to r295769 
> and then running "ninja check-llvm".  How come LLDB doesn't fail but LLVM 
> does when using these flags?

No idea. Check the system console.

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


Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Zachary Turner via lldb-dev
On Tue, Feb 21, 2017 at 4:24 PM Greg Clayton via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> On Feb 21, 2017, at 9:10 AM, Zachary Turner via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
> No comments on this specifically, but +1 to reducing shared_ptr usage in
> general. We use way too many, and often it feels like shared_ptr is used as
> a way to avoid having to think about ownership, which leads to more
> problems than it solves imo
>
>
> We do a pretty good job of using shared pointers where they are needed, so
> I don't agree with the above statement. We have strong memory models in
> LLDB and certain things need to keep certain things alive. LLVM is quite
> the wild west with regard to memory management, so I don't want to base any
> changes on it. We just need to use std::weak_ptr when needed. There should
> be no two items that contain strong references to each other.
>
> In general think about what should keep things alive. If I have a SBModule
> variable, I should expect that it won't let my module go away. If I have a
> SBTarget, I would expect it to keep the target around as long as I have a
> reference. Things that belong to any of these items, like breakpoints,
> watchpoints, compile units, functions, shouldn't keep the module or target
> around.
>

Agree with all of this, but I think most of the time you only need 1 thing
keeping an object alive (i.e. unique_ptr), and from there you can just hand
out references.  In the case where an object might die while a reference is
outstanding, then a lot of times I think the design can be adjusted so that
doesn't happen, but when it's still necessary, you can store a shared_ptr
and vend out weak_ptrs.  You don't need everything in the system keeping
everything else alive.

I'm speaking from a purely general standpoint, I haven't done a deep dive
into LLDB to see where this does / doesn't apply, but in my experience
shared_ptrs are one of the most overused things in all of C++.  So when I
see a lot of them, I just default to "I bet these can be reduced
significantly"
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev


Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Jim Ingham via lldb-dev

> On Feb 21, 2017, at 4:26 PM, Greg Clayton  wrote:
> 
>> 
>> On Feb 21, 2017, at 9:36 AM, Jim Ingham via lldb-dev 
>>  wrote:
>> 
>> Thanks for digging into this.
>> 
>> Maybe a better option would be to have IRExecutionUnit hold a 
>> SymbolContextScope, rather than a SymbolContext.  We started out with 
>> SymbolContext's because they were convenient but they hold too many things 
>> alive so when we run into SymbolContext's we've been converting them to the 
>> SymbolContextScope's when possible.  I can't see any need for the 
>> IRExecutionUnit to hold the module alive, so long as it handles failure to 
>> get the SymbolContext gracefully, which shouldn't be hard.
> 
> SymbolContextScope doesn't hold ownership as it is just a pointer. This 
> pointer would go bad if the module goes away and it would crash when you 
> tried to use it. See my other email for another approach with 
> SymbolContextRef.

Brain, brain, what is brain?

Yeah, I was conflating ExecutionContextRef & SymbolContext.  But still, having a SymbolContext equivalent to ExecutionContextRef 
seems the correct solution to me, even though we don't have the pre-baked class 
for it.

Jim


> 
> Greg
> 
>> 
>> Jim
>> 
>>> On Feb 21, 2017, at 9:03 AM, Pavel Labath via lldb-dev 
>>>  wrote:
>>> 
>>> Hello all,
>>> 
>>> I've been debugging the newly added TestStepOverBreakpoint.py, which
>>> has been failing on windows, for a very prosaic reason: after the test
>>> completes, we are unable to run "make clean" because lldb still holds
>>> the file open.
>>> 
>>> After some debugging, I've found that this happens because the test
>>> case stores the SBBreakpoint object in a member variable of the python
>>> test case class. The breakpoint class ends up transitively holding a
>>> reference to the main executable module, which prevents the module
>>> from being garbage-collected when the target is destroyed.
>>> 
>>> For reference, the full ownership chain is something like:
>>> StepOverBreakpointTestCase(python) => SBBreakpoint => Breakpoint =>
>>> BreakpointLocation => LLVMUserExpression => IRExecutionUnit =>
>>> SymbolContext => Module.
>>> 
>>> To get the test working, we need to break this chain somewhere. A
>>> couple of places I see are:
>>> - BreakpointLocation: Remove the compiled expression reference when
>>> the target is destroyed (AFAICS, it is used as a cache to avoid
>>> recomputing the expression every time. It can be theoretically
>>> recomputed if needed, but that shouldn't be necessary as the target is
>>> destroyed anyway)
>>> 
>>> - SBBreakpoint: make SBBreakpoint hold a weak_ptr to the Breakpoint
>>> object. When the target is destroyed, the SBBreakpoint object becomes
>>> invalid (One doesn't cannot do anything useful with the breakpoint
>>> once the target has been deleted anyway).
>>> 
>>> - StepOverBreakpointTestCase: Have the test not store the breakpoints
>>> in the test case object. Basically, declare that this is not a bug,
>>> and it's the users responsibility to clean up necessary objects.
>>> 
>>> Any thoughts on what is the appropriate solution here?
>>> 
>>> cheers,
>>> pavel
>>> ___
>>> lldb-dev mailing list
>>> lldb-dev@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>> 
>> ___
>> lldb-dev mailing list
>> lldb-dev@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

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


Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Jim Ingham via lldb-dev

> On Feb 21, 2017, at 4:24 PM, Greg Clayton via lldb-dev 
>  wrote:
> 
>> - StepOverBreakpointTestCase: Have the test not store the breakpoints
>> in the test case object. Basically, declare that this is not a bug,
>> and it's the users responsibility to clean up necessary objects.
> 
> It would be nice to avoid this.
> 
>> 

I don't agree with this.  I think trying to force folks using the API from 
Python to manually clear all stored objects would be really obnoxious.  If 
anything, we should figure out how to make this accidental failure into an 
intended failure so we can make sure we don't end up requiring this kind of 
micro-management.

Jim


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


Re: [lldb-dev] Too many ModuleSP references

2017-02-21 Thread Jim Ingham via lldb-dev

> On Feb 21, 2017, at 4:49 PM, Jim Ingham via lldb-dev 
>  wrote:
> 
> 
>> On Feb 21, 2017, at 4:24 PM, Greg Clayton via lldb-dev 
>>  wrote:
>> 
>>> - StepOverBreakpointTestCase: Have the test not store the breakpoints
>>> in the test case object. Basically, declare that this is not a bug,
>>> and it's the users responsibility to clean up necessary objects.
>> 
>> It would be nice to avoid this.
>> 
>>> 
> 
> I don't agree with this.  I think trying to force folks using the API from 
> Python to manually clear all stored objects would be really obnoxious.  If 
> anything, we should figure out how to make this accidental failure into an 
> intended failure so we can make sure we don't end up requiring this kind of 
> micro-management.

It's possible you meant "it would be nice to avoid it's being the user's 
responsibility to clean up necessary objects", in which case sorry for 
mis-reading but happy we agree...

Jim


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

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


Re: [lldb-dev] Debugging ELF relocatable files using LLDB

2017-02-21 Thread Ramana via lldb-dev
On Thu, Feb 16, 2017 at 10:26 PM, Greg Clayton  wrote:
>
> On Feb 16, 2017, at 3:51 AM, Ramana via lldb-dev 
> wrote:
>
> It looks like LLDB doesn't like ELF relocatable files for debugging
> and asserts with the following message when tried
>
> /lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2228:
> unsigned int ObjectFileELF::RelocateSection(.):  Assertion `false
> && "unexpected relocation type"' failed.
>
> Are we not supposed to debug ELF relocatable files on LLDB or am I
> missing something?
>
> If we cannot debug the relocatable files, is it _simply_ because those
> files lack program headers (program memory map) and relocations are
> yet to be processed (for debug info) or there are other reasons?
>
> For our target, the assembler output itself is a self contained ELF
> and hence will not have external references (both code and data). I am
> wondering if I can debug these ELF files on LLDB with minimal changes
> which does not require a full (or proper) linking step and would
> appreciate any pointers on that.
>
> Thanks,
> Ramana
>
>
> Looks like you just need to add support for the 32 bit relocations:
>
>
> if (hdr->Is32Bit()) {
>   switch (reloc_type(rel)) {
>   case R_386_32:
>   case R_386_PC32:
>   default:
> assert(false && "unexpected relocation type");
>   }
> } else {
>   switch (reloc_type(rel)) {
>   case R_X86_64_64: {
> symbol = symtab->FindSymbolByID(reloc_symbol(rel));
> if (symbol) {
>   addr_t value = symbol->GetAddressRef().GetFileAddress();
>   DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
>   uint64_t *dst = reinterpret_cast(
>   data_buffer_sp->GetBytes() + rel_section->GetFileOffset() +
>   ELFRelocation::RelocOffset64(rel));
>   *dst = value + ELFRelocation::RelocAddend64(rel);
> }
> break;
>   }
>   case R_X86_64_32:
>   case R_X86_64_32S: {
> symbol = symtab->FindSymbolByID(reloc_symbol(rel));
> if (symbol) {
>   addr_t value = symbol->GetAddressRef().GetFileAddress();
>   value += ELFRelocation::RelocAddend32(rel);
>   assert(
>   (reloc_type(rel) == R_X86_64_32 && (value <= UINT32_MAX)) ||
>   (reloc_type(rel) == R_X86_64_32S &&
>((int64_t)value <= INT32_MAX && (int64_t)value >=
> INT32_MIN)));
>   uint32_t truncated_addr = (value & 0x);
>   DataBufferSP &data_buffer_sp = debug_data.GetSharedDataBuffer();
>   uint32_t *dst = reinterpret_cast(
>   data_buffer_sp->GetBytes() + rel_section->GetFileOffset() +
>   ELFRelocation::RelocOffset32(rel));
>   *dst = truncated_addr;
> }
> break;
>   }
>   case R_X86_64_PC32:
>   default:
> assert(false && "unexpected relocation type");
>   }
> }
>
>
> I am guessing you will do something similar to the x86-64 stuff.

I tried to mimic the x86_64 relocations handling for our target but
getting segmentation fault while trying to write to the 'dst'
location.

In fact, the x86_64 also segfaults while trying to write to 'dst'
location. I just tried to debug the following simple program for
x86_64.

main.c:
int main () {
   return 0;
}

$ clang main.c -o main_64b.o --target=x86_64 -c -g
$  lldb main_64b.o
(lldb) target create "main_64b.o"
Current executable set to 'main_64b.o' (x86_64).
(lldb) source list
Segmentation fault (core dumped)

Am I doing something wrong or support for debugging the x86_64 ELF
relocatable files using LLDB is broken?

BTW, I am using LLVM v3.6 and LLDB v3.6.

Regards,
Ramana
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev