[lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[vadimcn]

Summary

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.

[ted]

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, or hit.

Thanks, Vadim!

(lldb) p main
(int (*)(int, unsigned char **)) $0 = 0x00005128
(lldb) b 0x5128
Breakpoint 1: address = 0x00005128
(lldb) br l
Current breakpoints:
1: address = 0x0000000000005128, locations = 1
  1.1: address = 0x00005128, 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: 0x00005128 factorial`main(argc=-1161904401, argv=0x00005bac) at 
factorial.c:13
   10   }
   11  
   12   int main(int argc, char **argv)
-> 13   {
   14     unsigned base;
   15  
   16   /*
(lldb) re r pc
      pc = 0x00005128  factorial`main at factorial.c:13
(lldb) br l
Current breakpoints:
1: address = 0x00005128, locations = 1, resolved = 1, hit count = 1
  1.1: address = 0x00005128, resolved, hardware, hit count = 1

[jingham]

In the future, can you generate patches with context (i.e. pass -U9999 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 in different orders or the effects of ASLR, where a given logical address will end up in the running process. So 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.

[jingham]

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

[vadimcn]

In the future, can you generate patches with context (i.e. pass -U9999 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.

[vadimcn]

Added patch context.

[jingham]

In D109738#3001122, @vadimcn wrote:

In the future, can you generate patches with context (i.e. pass -U9999 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 location. Anything else would be surprising
to me.

I think this is okay as a fallback position. IMO, the way it should work is:

1. If the address maps uniquely to a section relative address when set, it was very very likely the

user's intent that the breakpoint track that section relative address, so we should convert
it to that in order to follow the user's likely intention. Note this is clearly the case in the
example you cited as motivation for this patch.

2. If the address maps to more than one section, I think we should fail the breakpoint

and say "this maps to these shared libraries, please use the -s option to indicate the shared library."
It was pretty clear the user was trying to do something, but not capturing their intent in the breakpoint
setting they did, so this seems like the reasonable reaction.

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. Again,
on re-run even if libraries move it is not at all likely that the user's intent was to set the breakpoint on
whatever other library happened to show up at that address. If you have a good use case for this, we
can add a --force or maybe you can say --s NONE to indicate you don't want this resolved to section
relative.

4. The only other way that this breakpoint would take is if we hit the gdb-JIT notification breakpoint and

found the address in some JIT code. If this happens, it's probably okay to keep the breakpoint around non-section
relative. You run the risk that some other library loads at this address, and we set the breakpoint in data-in-code
that we don't know about and that causes the program to behave badly in ways that are difficult to diagnose.
But that's fairly unlikely.

[vadimcn]

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?

[ted]

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".

[vadimcn]

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

[jingham]

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 you should probably see if you can do anything else.

So to me the next bug is that you are trying to record breakpoint locations by raw address. They probably weren't set that way, and they certainly weren't initially resolved that way, since the only way you'll get a breakpoint inserted is (a) if some section gets loaded at that address, or (b) we are told about a JIT region. In case (a) we probably should fix the address to the section it was found in so that something predictable happens on rerun. In case (b) we really should refuse to reset the breakpoint. In the case of JIT code we don't necessarily know its structure, so we are very likely to stick our breakpoint somewhere that will cause problems the next time something gets JIT-ed there.

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.

[d-millar]

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

---

From: lldb-commits <[e-mail]> on behalf of Jim Ingham via Phabricator via lldb-commits <[e-mail]>
Sent: Friday, October 8, 2021 5:44:49 PM
To: [e-mail]; [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
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
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 you should probably see if you can do anything else.

So to me the next bug is that you are trying to record breakpoint locations by raw address. They probably weren't set that way, and they certainly weren't initially resolved that way, since the only way you'll get a breakpoint inserted is (a) if some section gets loaded at that address, or (b) we are told about a JIT region. In case (a) we probably should fix the address to the section it was found in so that something predictable happens on rerun. In case (b) we really should refuse to reset the breakpoint. In the case of JIT code we don't necessarily know its structure, so we are very likely to stick our breakpoint somewhere that will cause problems the next time something gets JIT-ed there.

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.

Repository:

rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION

https://reviews.llvm.org/D109738/new/

https://reviews.llvm.org/D109738

---

lldb-commits mailing list
[e-mail]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[d-millar]

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

---

From: lldb-commits <[e-mail]> on behalf of Jim Ingham via Phabricator via lldb-commits <[e-mail]>
Sent: Friday, October 8, 2021 5:44:49 PM
To: [e-mail]; [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]
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
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 you should probably see if you can do anything else.

So to me the next bug is that you are trying to record breakpoint locations by raw address. They probably weren't set that way, and they certainly weren't initially resolved that way, since the only way you'll get a breakpoint inserted is (a) if some section gets loaded at that address, or (b) we are told about a JIT region. In case (a) we probably should fix the address to the section it was found in so that something predictable happens on rerun. In case (b) we really should refuse to reset the breakpoint. In the case of JIT code we don't necessarily know its structure, so we are very likely to stick our breakpoint somewhere that will cause problems the next time something gets JIT-ed there.

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.

Repository:

rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION

https://reviews.llvm.org/D109738/new/

https://reviews.llvm.org/D109738

---

lldb-commits mailing list
[e-mail]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[ted]

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".

[jingham]

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.

[vadimcn]

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".

[jingham]

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) that 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.

If the module hasn't loaded yet and you know where it is, you can add the module to the target and set a breakpoint there. Then when the module loads lldb will insert the breakpoint. Or your IDE can stop on shared library load, and see if the UUID was loaded and insert the breakpoint then.

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.

lldb/source/Breakpoint/BreakpointResolverAddress.cpp

 //===-- BreakpointResolverAddress.cpp -------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Breakpoint/BreakpointResolverAddress.h"
 
 
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Log.h"

 void BreakpointResolverAddress::ResolveBreakpoint(SearchFilter &filter) {
   // If the address is not section relative, then we should not try to re-
   // resolve it, it is just some random address and we wouldn't know what to do
   // on reload.  But if it is section relative, we need to re-resolve it since
   // the section it's in may have shifted on re-run.
   bool re_resolve = false;
   if (m_addr.GetSection() || m_module_filespec)
     re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
+  else {
+    BreakpointSP breakpoint = GetBreakpoint();
+    if (breakpoint->GetNumLocations() == 0 ||
+        breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
       re_resolve = true;
+  }
 
   if (re_resolve)
     BreakpointResolver::ResolveBreakpoint(filter);
 }
 
 void BreakpointResolverAddress::ResolveBreakpointInModules(
     SearchFilter &filter, ModuleList &modules) {
   // See comment in ResolveBreakpoint.
   bool re_resolve = false;
   if (m_addr.GetSection())
     re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
+  else {
+    BreakpointSP breakpoint = GetBreakpoint();
+    if (breakpoint->GetNumLocations() == 0 ||
+        breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
       re_resolve = true;
+  }
 
   if (re_resolve)
     BreakpointResolver::ResolveBreakpointInModules(filter, modules);
 }
 
 Searcher::CallbackReturn BreakpointResolverAddress::SearchCallback(
     SearchFilter &filter, SymbolContext &context, Address *addr) {
   BreakpointSP breakpoint_sp = GetBreakpoint();
if (breakpoint.GetNumLocations() == 0) {
         Log *log(
             lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
         LLDB_LOGF(log, "Added location: %s\n", s.GetData());
       }
     } else {
       BreakpointLocationSP loc_sp = breakpoint.GetLocationAtIndex(0);
       lldb::addr_t cur_load_location =
           m_addr.GetLoadAddress(&breakpoint.GetTarget());
-      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();
       }
     }
   }
   return Searcher::eCallbackReturnStop;
 }

lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py

def address_breakpoints(self):
 
         # Now re-launch and see that we hit the breakpoint again:
         launch_info.Clear()
         launch_info.SetLaunchFlags(flags)
 
         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)