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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D109738#3054058 , @vadimcn wrote:

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

If you want to save the section relative address info within an lldb session, 
if you resolve a load address against the Module containing the address (or 
just resolve the load address using the target when the process is running) 
will give you a section relative address.

If you are interested in saving it across sessions, I would save the module 
UUID and the file address in that module.  Then the next time you need to 
create the breakpoint, look up the module by UUID, and then call 
ResolveFileAddress and use the SBAddress you get back to resolve the breakpoint.

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

Provided the system you are working on has reliable UUID's, the UUID will 
protect against that.  If you don't have reliable UUID's then you have to fall 
back to storing mod times or something.

>> More generally, using raw address breakpoints is fallible, and whenever you 
>> find yourself doing that you should probably see if you can do anything else.
>> ...
>> This patch as is worries me because it moves us further down the path of 
>> passing around raw addresses from run to run, and when that doesn't work it 
>> can cause hard to diagnose problems in the running of the process.
>
> I dunno, I guess I am in the camp of "Give a stern warning, but don't second 
> guess the developer".It's pretty annoying when your tool refuses to do 
> something, which you know is reasonable under the circumstances, not because 
> of a technical limitation, but "for your own good".

The alternative reading is "this is an area where you can get yourself into 
trouble if you don't do it right, so we should make it easy to do the right 
thing (always record address breakpoints with modules, and don't be smart about 
trying to "reset" them on rerun.)  Seems like lldb should take care of that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

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

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

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D109738#3052661 , @ted wrote:

> In D109738#3052624 , @jingham wrote:
>
>> That is clearly wrong: an address is never enough to restore a breakpoint 
>> from one session to the next even if you can turn ASLR off.  After all, the 
>> breakpoint could be in a dlopened library.
>
> This is usually true.
>
> In cases where you're debugging an embedded application that doesn't involve 
> shared libraries, it's not. The main area where I see this is when debugging 
> application startup code. My coworker who's responsible for that is the one 
> who reported the problem originally. In that case, the address of the 
> breakpoint is always valid on subsequent runs.
>
> On the other hand, when you're running under our RTOS, the application is a 
> PIE, and we support shared libraries, so the address is never valid on 
> subsequent runs.
>
> Maybe we need a flag to "break set" that means "this address is always valid; 
> don't garbage collect it".

I'd almost suggest making another breakpoint type (like --fixed-address or 
something).  That way you won't have to fight the normal address resolver, and 
just keep trying to write a breakpoint at that address on each load event till 
it succeeds.  Note, you could trivially do this with scripted breakpoints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

In D109738#3052624 , @jingham wrote:

> That is clearly wrong: an address is never enough to restore a breakpoint 
> from one session to the next even if you can turn ASLR off.  After all, the 
> breakpoint could be in a dlopened library.

This is usually true.

In cases where you're debugging an embedded application that doesn't involve 
shared libraries, it's not. The main area where I see this is when debugging 
application startup code. My coworker who's responsible for that is the one who 
reported the problem originally. In that case, the address of the breakpoint is 
always valid on subsequent runs.

On the other hand, when you're running under our RTOS, the application is a 
PIE, and we support shared libraries, so the address is never valid on 
subsequent runs.

Maybe we need a flag to "break set" that means "this address is always valid; 
don't garbage collect it".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[David Millar via Phabricator via lldb-commits]

d-millar added a comment.

That discussion is extremely helpful - gives me good context for what I need to 
suport.  THANKS!



From: lldb-commits  on behalf of Jim 
Ingham via Phabricator via lldb-commits 
Sent: Friday, October 8, 2021 5:44:49 PM
To: vadi...@gmail.com; jing...@apple.com
Cc: djordje.todoro...@syrmia.com; lldb-commits@lists.llvm.org; 
liburd1...@outlook.com; quic_soura...@quicinc.com; h.imai@nitech.jp; 
serhiy.re...@gmail.com; david.spick...@linaro.org
Subject: [Lldb-commits] [PATCH] D109738 : 
[lldb] Fix bug 38317 - Address breakpoints don't work if set before the process 
launches

jingham added a comment.

I realized I typed this all down a while ago but forgot to hit submit.  I think 
I still agree with former me...

In D109738#3002257  
https://reviews.llvm.org/D109738#3002257, @vadimcn wrote:

> Hi Jim,
> I think there's a bit of confusion going on here:
> The original bug was opened by Ted Woodward, and I think his scenario was
> motivated by embedded debugging.   He also provided that example with a
> breakpoint being set in main.
>
> I've rediscovered this bug independently, while working on an IDE
> integration project.  In my case, the IDE saves just the address of the
> instruction where a breakpoint was set in the previous debugging session.

That is clearly wrong: an address is never enough to restore a breakpoint from 
one session to the next even if you can turn ASLR off.  After all, the 
breakpoint could be in a dlopened library.  That makes getting this breakpoint 
back to where it came from dependent on the path of execution.  For instance, 
if one library had been dlopened at the address in question and then dlclosed 
and then your target library dlopened in the same spot before the stop that set 
the breakpoint, then next time round you will set the breakpoint in the first 
library loaded not the second one, which is again not right.  Remember also 
that setting breakpoints is not a risk-free process.  On x86-64, for instance, 
you could end up setting the breakpoint not on an instruction boundary, you 
could set it in some data, etc.  All leading to hard-to-diagnose failures in 
the running process.  So some caution is warranted here.

Is it possible to get your IDE to record the module & the address?  WIth that 
pair, you can always set the breakpoint at the right place, and lldb allows you 
to specify a module as a filter when you set an address breakpoint, so there 
would be no problem with resetting it this way using the SB API's.

> In the next session, the debug adapter is expected to restore instruction
> breakpoints using just the absolute addresses; the protocol does not
> provide for any user interaction.  Also, this happens via the SB API, not
> via LLDB commands.   Fortunately, when ASLR is disabled, modules are
> usually loaded at the same address, so things tend to work out 99% of the
> time.  The other 1%... well, if you are debugging on the assembly
> instruction level, you are kinda expected to know what you are doing and be
> prepared to deal with problems like those you've described above.

lldb use on iOS is far above 1% of the lldb users.

> When the target is created, the modules for the target are loaded and
>
>> mapped at whatever address is their default load address.
>
> Clearly, this isn't happening.  From what I can see, section addresses are
> not known until the process starts running.   Maybe this is another bug
> that needs fixing?

Breakpoints eventually resolve to "breakpoint locations", which are specified 
by an lldb_private::Address.  When you set a file & line breakpoint, or a 
symbol name breakpoint we make always make a Section relative Address for the 
breakpoint location.  When the binary loads, provided its UUID hasn't changed 
we don't re-resolve the breakpoint, which would be wasted effort since we 
already know its section relative offset, and can use that to calculate the 
load address to send to the debug monitor.

So it is the case that we can make section relative addresses pre-run and 
resolve them on running.  In fact, when you ask lldb "image lookup -n main" 
before run, we will get the Address for the main symbol - a section relative 
Address - and print it at the "default" load address of the library.  So we 
deal with Section relative Addresses all the time pre run.  The only thing we 
don't know is the "load address" of the section, because the SectionLoadList 
isn't filled in pre-run.

When you set a breakpoint by address pre-run, if the address is uniquely in the 
default load range of some binary, we certainly CAN figure out the section and 
resolve it.  The important point about Ted's report is that in a case where we 
clearly could do that, we aren't.

> Even so, I think that address breakpoints should still function when
> section addresses aren't known at breakpoint creation time (for whatever
> 

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

[David Millar via Phabricator via lldb-commits]

d-millar added subscribers: DavidSpickett, djtodoro, HirokiImai, jingham, 
ZeeZeeMorin, SouraVX, serhiy.redko, vadimcn, d-millar.
d-millar added a comment.

That discussion is extremely helpful - gives me good context for what I need to 
suport.  THANKS!



From: lldb-commits  on behalf of Jim 
Ingham via Phabricator via lldb-commits 
Sent: Friday, October 8, 2021 5:44:49 PM
To: vadi...@gmail.com; jing...@apple.com
Cc: djordje.todoro...@syrmia.com; lldb-commits@lists.llvm.org; 
liburd1...@outlook.com; quic_soura...@quicinc.com; h.imai@nitech.jp; 
serhiy.re...@gmail.com; david.spick...@linaro.org
Subject: [Lldb-commits] [PATCH] D109738 : 
[lldb] Fix bug 38317 - Address breakpoints don't work if set before the process 
launches

jingham added a comment.

I realized I typed this all down a while ago but forgot to hit submit.  I think 
I still agree with former me...

In D109738#3002257  
https://reviews.llvm.org/D109738#3002257, @vadimcn wrote:

> Hi Jim,
> I think there's a bit of confusion going on here:
> The original bug was opened by Ted Woodward, and I think his scenario was
> motivated by embedded debugging.   He also provided that example with a
> breakpoint being set in main.
>
> I've rediscovered this bug independently, while working on an IDE
> integration project.  In my case, the IDE saves just the address of the
> instruction where a breakpoint was set in the previous debugging session.

That is clearly wrong: an address is never enough to restore a breakpoint from 
one session to the next even if you can turn ASLR off.  After all, the 
breakpoint could be in a dlopened library.  That makes getting this breakpoint 
back to where it came from dependent on the path of execution.  For instance, 
if one library had been dlopened at the address in question and then dlclosed 
and then your target library dlopened in the same spot before the stop that set 
the breakpoint, then next time round you will set the breakpoint in the first 
library loaded not the second one, which is again not right.  Remember also 
that setting breakpoints is not a risk-free process.  On x86-64, for instance, 
you could end up setting the breakpoint not on an instruction boundary, you 
could set it in some data, etc.  All leading to hard-to-diagnose failures in 
the running process.  So some caution is warranted here.

Is it possible to get your IDE to record the module & the address?  WIth that 
pair, you can always set the breakpoint at the right place, and lldb allows you 
to specify a module as a filter when you set an address breakpoint, so there 
would be no problem with resetting it this way using the SB API's.

> In the next session, the debug adapter is expected to restore instruction
> breakpoints using just the absolute addresses; the protocol does not
> provide for any user interaction.  Also, this happens via the SB API, not
> via LLDB commands.   Fortunately, when ASLR is disabled, modules are
> usually loaded at the same address, so things tend to work out 99% of the
> time.  The other 1%... well, if you are debugging on the assembly
> instruction level, you are kinda expected to know what you are doing and be
> prepared to deal with problems like those you've described above.

lldb use on iOS is far above 1% of the lldb users.

> When the target is created, the modules for the target are loaded and
>
>> mapped at whatever address is their default load address.
>
> Clearly, this isn't happening.  From what I can see, section addresses are
> not known until the process starts running.   Maybe this is another bug
> that needs fixing?

Breakpoints eventually resolve to "breakpoint locations", which are specified 
by an lldb_private::Address.  When you set a file & line breakpoint, or a 
symbol name breakpoint we make always make a Section relative Address for the 
breakpoint location.  When the binary loads, provided its UUID hasn't changed 
we don't re-resolve the breakpoint, which would be wasted effort since we 
already know its section relative offset, and can use that to calculate the 
load address to send to the debug monitor.

So it is the case that we can make section relative addresses pre-run and 
resolve them on running.  In fact, when you ask lldb "image lookup -n main" 
before run, we will get the Address for the main symbol - a section relative 
Address - and print it at the "default" load address of the library.  So we 
deal with Section relative Addresses all the time pre run.  The only thing we 
don't know is the "load address" of the section, because the SectionLoadList 
isn't filled in pre-run.

When you set a breakpoint by address pre-run, if the address is uniquely in the 
default load range of some binary, we certainly CAN figure out the section and 
resolve it.  The important point about Ted's report is that in a case where we 
clearly could do that, we aren't.

> Even so, I 

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I realized I typed this all down a while ago but forgot to hit submit.  I think 
I still agree with former me...

In D109738#3002257 , @vadimcn wrote:

> Hi Jim,
> I think there's a bit of confusion going on here:
> The original bug was opened by Ted Woodward, and I think his scenario was
> motivated by embedded debugging.   He also provided that example with a
> breakpoint being set in main.
>
> I've rediscovered this bug independently, while working on an IDE
> integration project.  In my case, the IDE saves just the address of the
> instruction where a breakpoint was set in the previous debugging session.

That is clearly wrong: an address is never enough to restore a breakpoint from 
one session to the next even if you can turn ASLR off.  After all, the 
breakpoint could be in a dlopened library.  That makes getting this breakpoint 
back to where it came from dependent on the path of execution.  For instance, 
if one library had been dlopened at the address in question and then dlclosed 
and then your target library dlopened in the same spot before the stop that set 
the breakpoint, then next time round you will set the breakpoint in the first 
library loaded not the second one, which is again not right.  Remember also 
that setting breakpoints is not a risk-free process.  On x86-64, for instance, 
you could end up setting the breakpoint not on an instruction boundary, you 
could set it in some data, etc.  All leading to hard-to-diagnose failures in 
the running process.  So some caution is warranted here.

Is it possible to get your IDE to record the module & the address?  WIth that 
pair, you can always set the breakpoint at the right place, and lldb allows you 
to specify a module as a filter when you set an address breakpoint, so there 
would be no problem with resetting it this way using the SB API's.

> In the next session, the debug adapter is expected to restore instruction
> breakpoints using just the absolute addresses; the protocol does not
> provide for any user interaction.  Also, this happens via the SB API, not
> via LLDB commands.   Fortunately, when ASLR is disabled, modules are
> usually loaded at the same address, so things tend to work out 99% of the
> time.  The other 1%... well, if you are debugging on the assembly
> instruction level, you are kinda expected to know what you are doing and be
> prepared to deal with problems like those you've described above.

lldb use on iOS is far above 1% of the lldb users.

> When the target is created, the modules for the target are loaded and
>
>> mapped at whatever address is their default load address.
>
> Clearly, this isn't happening.  From what I can see, section addresses are
> not known until the process starts running.   Maybe this is another bug
> that needs fixing?

Breakpoints eventually resolve to "breakpoint locations", which are specified 
by an lldb_private::Address.  When you set a file & line breakpoint, or a 
symbol name breakpoint we make always make a Section relative Address for the 
breakpoint location.  When the binary loads, provided its UUID hasn't changed 
we don't re-resolve the breakpoint, which would be wasted effort since we 
already know its section relative offset, and can use that to calculate the 
load address to send to the debug monitor.

So it is the case that we can make section relative addresses pre-run and 
resolve them on running.  In fact, when you ask lldb "image lookup -n main" 
before run, we will get the Address for the main symbol - a section relative 
Address - and print it at the "default" load address of the library.  So we 
deal with Section relative Addresses all the time pre run.  The only thing we 
don't know is the "load address" of the section, because the SectionLoadList 
isn't filled in pre-run.

When you set a breakpoint by address pre-run, if the address is uniquely in the 
default load range of some binary, we certainly CAN figure out the section and 
resolve it.  The important point about Ted's report is that in a case where we 
clearly could do that, we aren't.

> Even so, I think that address breakpoints should still function when
> section addresses aren't known at breakpoint creation time (for whatever
> reason), if only on a best-effort basis.
>
> 3. If the address doesn't fall into any sections, then it would be fine to
>
>> assign it to the first section that shows up at that address, but I would
>> convert it to section relative at that point.
>
> This is what I am attempting to do with my patch.  Is there something else
> I should do to make sure I am not regressing other scenarios?

The fact that Ted's case doesn't work means that at some point we stopped 
recording address breakpoints as section relative in cases where we should.  
That to me is the primary bug, but maybe not so interesting to you.

More generally, using raw address breakpoints is fallible, and whenever you 
find yourself doing that 

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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

In D109738#3000608 , @jingham wrote:

> BTW, do you know what change made this stop working?

https://github.com/llvm-mirror/lldb/commit/43793406 in the old multirepo. In 
the monorepo, it's https://github.com/llvm/llvm-project/commit/5ec7cd7 .

We've got a conflict between "clean up old breakpoint addresses left behind 
when a section is deleted" vs "keep around breakpoint addressed that are set 
explicitly, but aren't part of a loaded section".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

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

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

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

> mapped at whatever address is their default load address.

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

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D109738#3001122 , @vadimcn wrote:

>> In the future, can you generate patches with context (i.e. pass -U to
>> "git diff" or "git show"), it's a lot easier to read patches with context.
>
> Sure thing, will do.
>
> This doesn't seem like the right solution to the problem to me.  The way
>
>> address breakpoints SHOULD work  is that when you set the breakpoint, we
>> resolve the address you pass in against the images in the target.  If the
>> address is in an image in the list, then we convert the load address to a
>> section relative address before making the breakpoint.  Address breakpoints
>> made that way will always return true for m_addr.GetSection(), and so set
>> re_resolve to true before getting to the test you modified.
>
> No, actually.   In my scenario, the breakpoint is being created by load
> address (via SBTarget::BreakpointCreateByAddress(addr_t address)), right
> after the target has been created.   Since the process hasn't been launched
> at this point yet, there are no code sections mapped, so the breakpoint
> address stays non-section-relative.   My understanding is that in such a
> case, LLDB should attempt to re-resolve it upon loading each new module.

When the target is created, the modules for the target are loaded and mapped at 
whatever address is their default load address.
So all the libraries that lldb can find will be listed in the shared library 
list at that point, mapped at their pre-load addresses.  
On Darwin systems the dynamic loader plugin reads the closure of the loaded 
libraries from the binary's load commands, and loads them all.  
That's not a requirement of the dynamic loader, however, so you may just have 
the executable loaded.  But you will always have that.

The initial example you showed when proposing the patch has you finding the 
address of main, then getting its address and setting
an address breakpoint on that address.  So clearly there was some addresses 
associated with the main executable at that point, and
we could have re-resolved the address to a section relative one correctly.

There are problems with this approach for sure.  Mainly that there's no 
guarantee before run that all the libraries lldb detects
will have unique addresses.  It's pretty common for the linker to assign a 
default load address, often 0x0, for all shared libraries.
So before run, an address breakpoint is ambiguous, it could map to many 
section/offset pairs.

lldb doesn't usually fail to make breakpoints (it does, for instance, for a 
regex breakpoint where the regex doesn't compile...).  
But in this case, I think it should because there's no way to tell what the 
user's intentions were.  For instance, in your initial example 
where you got the symbol for main pre-run, then used that address to set an 
address breakpoint, say offset a bit from main or whatever.  
If on run, the breakpoint was resolved to whatever library happened to load at 
that address, you would not be best pleased.

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

lldb ASKS to disable ASLR by default.  The systems that lldb runs on are under 
no obligation to honor 
that request, and haven't on all iOS devices for the past 5 years or 
thereabouts.  You can't assume this will
get you out of trouble.

> Also, please see Ted's comment about embedded.

If you mean the comment with the example of getting main's address and then 
setting an address
breakpoint there, as I argued above, I think that more tends to argue we want 
to convert addresses
to section relative whenever we can.  The user's clear intention was to set the 
breakpoint on the address
that "main" was going to show up at, not at whatever code happens to end up at 
the pre-load address of main.

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

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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn updated this revision to Diff 372636.
vadimcn added a comment.

Added patch context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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


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


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

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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

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

Sure thing, will do.

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

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

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

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

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

BTW, do you know what change made this stop working?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

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

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

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

In your case, you have an address that's clearly in a section, so it should 
have been handled by the if(m_addr.GetSection() || m_module_filespec) test.  So 
the correct fix is to figure out how you ended up with a non-section relative 
address for the breakpoint.

I don't want lldb to silently reset non-section relative addresses.  The only 
way those should show up is if you set the breakpoint on some code that was NOT 
in any section, maybe JIT code or something, and lldb has no way of knowing 
what to do with those addresses on re-run.  Refusing to re-resolve them may be 
too harsh, but we should at least warn about them.  Whereas if the address is 
section relative, we can always do the right thing.

So the most important thing here is to make sure that address breakpoints 
always have section relative addresses if we can by any means identify the 
section they belong in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

I created the original bug because a change made LLDB stop working on address 
breakpoints if they were created before the program is run. Setting a 
breakpoint on an address is a typical case with embedded applications, 
especially if you want to debug startup code.

I took this patch and applied it to the latest downstream Hexagon repo. It 
behaved as expected - I set a breakpoint at the address of main, ran, and it 
stopped at the breakpoint. Before this patch, the breakpoint wouldn't be 
resolved.

Thanks, Vadim!

(lldb) p main
(int (*)(int, unsigned char **)) $0 = 0x5128
(lldb) b 0x5128
Breakpoint 1: address = 0x5128
(lldb) br l
Current breakpoints:
1: address = 0x5128, locations = 1

  1.1: address = 0x5128, unresolved, hit count = 0 

(lldb) r
Process 1 launched: '/usr2/tedwood/lldb_test/factorial' (hexagon)
Process 1 stopped

- thread #1, name = 'T1', stop reason = breakpoint 1.1 frame #0: 0x5128 
factorial`main(argc=-1161904401, argv=0x5bac) at factorial.c:13 10   } 11 
12   int main(int argc, char **argv)

-> 13   {

  14 unsigned base;
  15  
  16   /*

(lldb) re r pc

  pc = 0x5128  factorial`main at factorial.c:13

(lldb) br l
Current breakpoints:
1: address = 0x5128, locations = 1, resolved = 1, hit count = 1

  1.1: address = 0x5128, resolved, hardware, hit count = 1 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn created this revision.
vadimcn added a reviewer: jingham.
vadimcn added a project: LLDB.
Herald added a subscriber: JDevlieghere.
vadimcn requested review of this revision.
Herald added a subscriber: lldb-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109738

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


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


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