This doesnt cause any regressions on 32-bit arm testsuite. Tested on Ubuntu Xenial and Bionic 32bit arm host docker container. I hope this fixes all the redhat issues as well. Thank you so much Jan for all the effort on this and Thanks to Pavel for the reviews and assistance.

Some new observations:

Some new information below:

In D71498#1786341, @clayborg wrote:

As I am reading this, I just wanted to send out a note of something else that can cause crashes in ARM/Thumb code. For anyone working with ARM/Thumb on systems that don't use the ARM and Thumb BKPT instruction when setting software breakpoints (like all lldb linux and android flavors IIRC): if you try to overwrite a 32 bit thumb instruction that is a conditional instruction in a Thumb IT instruction with a 16 bit trap or illegal instruction you can crash your program. The issue arises for code like:

0x1000: xx xx         ITTTEE
0x1002: 00 11 22 33   32 bit thumb instruction (if condition)
0x1006: 44 55 66 77   32 bit thumb instruction (if condition)
0x100a: 88 99 aa bb   32 bit thumb instruction (else condition) 
0x100e: cc dd ee ff   32 bit thumb instruction (else condition)

If you try to set a breakpoint at any of the instructions in [0x1002-0x100e) using a 16 bit trap or illegal instruction (I use "bb bb" below for this trap for example purposes), you change the size of the instructions and which instructions are conditional. If we try to write "bb bb" to 0x1002 we now have:

0x1000: xx xx         ITTTEE
0x1002: bb bb         (if condition) the first conditional instruction is now 16 bit instead of 32 bit
0x1004: 22 33 44 55   (if condition) this has the last half of the previous instruction 
0x1008: 66 77 88 99   (else condition) this has the last half of the previous instruction 
0x100c: aa bb         (else condition) this has the last half of the previous instruction 
0x100e: cc dd ee ff   32 bit thumb instruction (NOT conditional anymore)

This will work if using the BKPT instruction only. Sorry for the noise if lldb-server is already using the BKPT instruction. But I just wanted to throw this out there in case this issue if affecting anyone.

Symbols sizes are being zeroed out

This issue is being caused by wrong address being written to memory somewhere while single stepping though i have reached the exact problem but the logs seem to suggest it.

I still seem to get the same issue after applying this patch and D63540.

@jankratochvil I have verified both D71498 and D71514. They do not fix the issues introduced by D63540.

I have updated this patch after incorporating previous suggestions like moving InvalidateAllRegisters declaration to NativeRegisterContextLinux and also relocated calls to InvalidateAllRegisters from NativeProcessLinux to NativeThreadLinux. We now call InvalidateAllRegisters just before we resume or single step via ptrace. Other than that on any register write register will be be invalidated by WriteGPR and WriteFPR as was happening previously. We dont really need a invalidate on detach because thread wont exist after that so I have removed it InvalidateAllRegisters which was being called at detach.

Any comments or LGTM?

Hi Jan,

Sorry for late response on this. I got side tracked into other issues and wanted performance testing before writing an update.

Re-opening this because I have reverted the commit due to failures seen on LLDB AArch64 buildbot with this commit.

Added a unittest as suggested by @labath.

Thanks rovka.

In D63540#1744110, @jankratochvil wrote:

It is still the same (no arm32 regression) with 16bdcc809c72c639a2888b6b859dca88453e3c28 and this patch reapplied.

In D70154#1743904, @labath wrote:

It looks like you're replacing everything with tabs. I suppose that's fine, as that is the prevalent local convention, but it might be better to go for spaces, as we don't use tabs anywhere else.

I have updated the patch as suggested and will move out the triple fix and whitespace issue in separate patches. Committing remaining changes shortly.

Hi Jan,

We ll be dealing with Linux user mode and mostly aarch64 data registers except for cpsr, fpsr and fpcr. I think we should be fine but let me confirm this again from documentation.

This corrects LLDB regressions.
LGTM.

In D68783#1709000, @sdesmalen wrote:

In D68783#1708963, @omjavaid wrote:

This revision does not fix LLDB regression on AArch64.

http://lab.llvm.org:8014/builders/lldb-aarch64-ubuntu

All tests in functionalities/thread/concurrent_events/* category still failing.

Correct, those tests are fixed by D66935. That fix was reverted because of a failure with msan on test/DebugInfo/AArch64/asan-stack-vars.mir and test/DebugInfo/AArch64/compiler-gen-bbs-livedebugvalues.mir, which this patch intends to fix. I kept the fixes separate to make them easier to review. Do you want me to merge the two patches?

This revision does not fix LLDB regression on AArch64.

This change was reverted and hence causes LLDB AArch64 test failures again.

Thanks @clayborg suggestions noted and will be fixed in committed patch.

This rev breaks LLDB for AArch64. functionalities/thread/concurrent_events/* tests are failing after r368987.

Fixes typo in first rev.

This change breaks arm/aarch64 linux builds.

This was much needed. LGTM

LGTM

Hi I have updated diff with corrections.

This patch has following updates:

In D29669#670896, @clayborg wrote:

I would prefer to see NativeBreakpoint struct expanded to have more member variables instead of adding a new hardware breakpoint list. Then you just ask any breakpoint to enable/disable/remove itself and the structure contains all of the info we need. Keeping two lists means we have to check two lists. Let me know if any of my inline comments weren't clear?

I have added look up for dladdr and it works for android debug info build.

So I have given this a thought and have a inclination towards sticking to my first patch that was testing for following:

Guarding against HAVE_DLOPEN now. Works fine.

There is not exact solution that satisfies all corner cases. Abandoning for now until I come up with a solution that covers us from all corners.

Although this patch fixes the test case in question but in theory EphemeralMode watchpoint enable/disable cycles should be independent of step-over watchpoint enable disable cycle.