@jasonmolenda friendly ping

@jasonmolenda I discovered the same issue in another way - create a python command and load it with "command script import". Here is test.py:

Fix test to exit lldb

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.

Prints were there for my debugging. They don't serve a purpose, so I've removed them.

Remove unneeded prints from test

[lldb] Handle malformed qfThreadInfo reply

I've got a test written. It doesn't crash like the debugger in the wild does, but it does give a tid of 0 for each thread I ask about. So I can assert if the threads don't have the correct tid.

I agree about the test. I'll work on one.

In D109738#3000608, @jingham wrote:

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

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.

LGTM. Thanks Michał!

Why would we not want to support this on other targets, like Arm? Fixes for AIX shouldn't impact other things.

Fixed formatting flagged by Lint: Pre-merge checks

Hi Paulo,
@clayborg asked me to look at this, because I've worked with systems that have multiple address spaces. I was thinking, instead of a WebAssembly specific memory read, we should implement an optional generic memory read and write with memory space support.

File llvm/test/DebugInfo/X86/vla.ll was modified so the diff did not apply anymore. object-emission was removed from it, so I'm removing it from this diff.

@jingham friendly ping

Friendly ping

Removed the code that sets m_constructor_errors when GetLoadAddressPermissions returns False, as requested by @jingham .

Wow @jingham you posted your comment and I landed the patch at the same time!

In D71372#1813703, @labath wrote:

In D71372#1813142, @ted wrote:

In D71372#1811594, @labath wrote:

In D71372#1810687, @ted wrote:

I've got another failure case for this. If the remote gdbserver doesn't implement qMemoryRegionInfo or qXfer:memory-map:read, thread step-out will fail.
....

That's a good point Ted. I think we should give targets which don't support fetching permissions the benefit of the doubt, and treat all memory as potentially executable. Would removing the return statement from the if(!GetLoadAddressPermissions) branch solve your problem? If so, can you whip up a patch for that?

Removing the return statement fixes the issue. I'll put up a patch. Keeping the m_constructor_errors.Printf line doesn't cause a failure; it might be useful to keep that in case the breakpoint can't be created for other reasons. What do you think?

I don't care much either way.. Since you have this kind of a target around, you can judge whether printing this error/warning after each "finish" would be useful or just annoying. Another possibility would be to don't print the error to the command output, but still emit something into the log...

In D71372#1811594, @labath wrote:

In D71372#1810687, @ted wrote:

I've got another failure case for this. If the remote gdbserver doesn't implement qMemoryRegionInfo or qXfer:memory-map:read, thread step-out will fail.
....

That's a good point Ted. I think we should give targets which don't support fetching permissions the benefit of the doubt, and treat all memory as potentially executable. Would removing the return statement from the if(!GetLoadAddressPermissions) branch solve your problem? If so, can you whip up a patch for that?

I've got another failure case for this. If the remote gdbserver doesn't implement qMemoryRegionInfo or qXfer:memory-map:read, thread step-out will fail.

llvm/tools/lldb/tools/driver/Driver.cpp:495:12: error: comparison of integers of different signs: 'ssize_t' (aka 'long') and 'size_t' (aka 'unsigned long') [-Werror,-Wsign-compare]

if (nrwr != commands_size) {
    ~~~~ ^  ~~~~~~~~~~~~~

1 error generated.

This doesn't look correct to me - it looks like there are 1 too many #endifs. I think the one at line 179 should be removed - it should have been replaced by the #else that is at line 180.

In D57552#1381126, @jingham wrote:

That sounds like it will have to be fixed somewhere else, and not part of this patch then. Can you file a bug to get that fixed?

In D57552#1381046, @jingham wrote:

In D57552#1380782, @ted wrote:

FYI, @jingham , this is also an issue with DYLD plugins, if the file in the link map starts with ".". "image search-paths add . /path/to/my/libraries" fails in this case, because "./library.so" gets changed to "library.so".

This should fix that issue as well, since image search-paths are also PathMappingLists. I don't see any tests for this, however. Do you have something handy you could turn into a test?

@sgraenitz I've found an issue with this patch, using the Visual Studio 2015 generator.

FYI, @jingham , this is also an issue with DYLD plugins, if the file in the link map starts with ".". "image search-paths add . /path/to/my/libraries" fails in this case, because "./library.so" gets changed to "library.so".

Another issue:

Added a test to TestAddressBreakpoints.py that sets an address breakpoint before launch, launches, and checks to see if the breakpoint was hit.

Yes, I'll add a new test to TestAddressBreakpoints.py.

@JDevlieghere I'm seeing the same issue as @alberto_magni . The Visual Studio cmake generator gives errors. We're reverting it internally, but it needs to be fixed or reverted ASAP.

In D39967#1017754, @labath wrote:

I believe lldb-server does not work around breakpoint opcodes during writing (it does for reading), but that can be fixed, of course.

Traditionally, remote gdbservers handle memory operations that overlap a software breakpoint. Writes get the new value saved off, and reads get the breakpoint opcode replaced by the saved value.

Thanks for adding me, Pavel.

We build lldb, clang and tools for Hexagon only, and call them hexagon-lldb, hexagon-clang, etc. The test infrastructure is smart enough to pick up hexagon-lldb-mi if we tell it to run with hexagon-lldb using --executable; will it be smart enough to run an in-tree hexagon-clang?

Tested with clang 3.8 on top-of-tree lldb-mi on Ubuntu 14.

We want to go down into a pointer if it's the original parent, but not into a child that's a pointer. To avoid skipping children of a top-level pointer/reference, I think we need to do the type check in the loop.

Updated with Greg's suggestion.

Added check for reference types, as Greg suggested.

I'm seeing the same issue as Nitesh, except in a different spot. The Hexagon Simulator doesn't support QSaveRegisterState, so lldb calls GDBRemoteRegisterContext::ReadAllRegisterValues. It gets a lock on the communication client, then calls ReadAllRegisters, which tries to get a lock on the communication client, and fails (hangs on Linux, crashes on Windows).

The problem this scenario has is asynchronous output from 2 different sources. The original fix made Windows (or any lldb without libedit) much better, but I think most non-Windows builds will use libedit, because it's extremely useful. lldb-mi has a different set of constraints; the position of the prompt isn't as important, since users won't use it as a cue to tell when the debugger can accept more input.

The Hexagon platform (currently in-house only) acts like the ios simulator platform, in that it doesn't connect - it fixes up the command line, then launches the Hexagon simulator (based on the debugserver launch code in the gdb-remote process plugin).