[lldb] Convert ProcessDebugger.cpp to new Status API (NFC)

[lldb] Support partial memory reads on windows (#106981)

ReadProcessMemory will not perform the read if part of the memory is
unreadable (and even though the API has a `number_of_bytes_read`
argum

[lldb] Support partial memory reads on windows (#106981)

ReadProcessMemory will not perform the read if part of the memory is
unreadable (and even though the API has a `number_of_bytes_read`
argument). To make this work, I explicitly inspect the memory region
being read and only read the accessible part.

show more ...

[lldb] Adapt Plugins/Process/Windows to new Status API

[lldb] Turn lldb_private::Status into a value type. (#106163)

This patch removes all of the Set.* methods from Status.

This cleanup is part of a series of patches that make it harder use the
ant

[lldb] Turn lldb_private::Status into a value type. (#106163)

This patch removes all of the Set.* methods from Status.

This cleanup is part of a series of patches that make it harder use the
anti-pattern of keeping a long-lives Status object around and updating
it while dropping any errors it contains on the floor.

This patch is largely NFC, the more interesting next steps this enables
is to:
1. remove Status.Clear()
2. assert that Status::operator=() never overwrites an error
3. remove Status::operator=()

Note that step (2) will bring 90% of the benefits for users, and step
(3) will dramatically clean up the error handling code in various
places. In the end my goal is to convert all APIs that are of the form

` ResultTy DoFoo(Status& error)
`
to

` llvm::Expected<ResultTy> DoFoo()
`
How to read this patch?

The interesting changes are in Status.h and Status.cpp, all other
changes are mostly

` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git
grep -l SetErrorString lldb/source)
`
plus the occasional manual cleanup.

show more ...

[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

Resubmission of https://reviews.llvm.org/D130309 with the 2 patches that fixed the linux buildb

[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute.

Resubmission of https://reviews.llvm.org/D130309 with the 2 patches that fixed the linux buildbot, and new windows fixes.

The FileSpec APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossible to control all of the times the FileSpec object is modified so we can clear cached member variables like m_resolved and with an upcoming patch caching if the file is relative or absolute. This patch modifies the APIs of FileSpec so no one can modify the directory or filename instance variables directly by adding set accessors and by removing the get accessors that are non const.

Many clients were using FileSpec::GetCString(...) which returned a unique C string from a ConstString'ified version of the result of GetPath() which returned a std::string. This caused many locations to use this convenient function incorrectly and could cause many strings to be added to the constant string pool that didn't need to. Most clients were converted to using FileSpec::GetPath().c_str() when possible. Other clients were modified to use the newly renamed version of this function which returns an actualy ConstString:

ConstString FileSpec::GetPathAsConstString(bool denormalize = true) const;

This avoids the issue where people were getting an already uniqued "const char *" that came from a ConstString only to put the "const char *" back into a "ConstString" object. By returning the ConstString instead of a "const char *" clients can be more efficient with the result.

The patch:
- Removes the non const GetDirectory() and GetFilename() get accessors
- Adds set accessors to replace the above functions: SetDirectory() and SetFilename().
- Adds ClearDirectory() and ClearFilename() to replace usage of the FileSpec::GetDirectory().Clear()/FileSpec::GetFilename().Clear() call sites
- Fixed all incorrect usage of FileSpec::GetCString() to use FileSpec::GetPath().c_str() where appropriate, and updated other call sites that wanted a ConstString to use the newly returned ConstString appropriately and efficiently.

Differential Revision: https://reviews.llvm.org/D130549

show more ...

[lldb][Windows] Fix memory region base addresses when a range is split

Previously we recorded AllocationBase as the base address of the region
we get from VirtualQueryEx. However, this is the base o

[lldb][Windows] Fix memory region base addresses when a range is split

Previously we recorded AllocationBase as the base address of the region
we get from VirtualQueryEx. However, this is the base of the allocation,
which can later be split into more regions.

So you got stuff like:
[0x00007fff377c0000-0x00007fff377c1000) r-- PECOFF header
[0x00007fff377c0000-0x00007fff37840000) r-x .text
[0x00007fff377c0000-0x00007fff37870000) r-- .rdata

Where all the base addresses were the same.

Instead, use BaseAddress as the base of the region. So we get:
[0x00007fff377c0000-0x00007fff377c1000) r-- PECOFF header
[0x00007fff377c1000-0x00007fff37840000) r-x .text
[0x00007fff37840000-0x00007fff37870000) r-- .rdata

https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-memory_basic_information

The added test checks for any overlapping regions which means
if we get the base or size wrong it'll fail. This logic
applies to any OS so the test isn't restricted to Windows.

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D129272

show more ...

[lldb] Remove an outdated comment. NFC.

This comment became outdated in 053eb35651906e693906fad6c695fce11415ade7
(but was moved along); that commit moved the code and the comment
to a separate funct

[lldb] Remove an outdated comment. NFC.

This comment became outdated in 053eb35651906e693906fad6c695fce11415ade7
(but was moved along); that commit moved the code and the comment
to a separate function, with a separate local variable
`num_of_bytes_read`. On error, the possibly garbage value is never
copied back to the caller's reference, thus the comment is no longer
relevant (and slightly confusing as is).

Differential Revision: https://reviews.llvm.org/D128226

show more ...

[lldb] Convert ProcessWindowsLog to the new API

Revert "[lldb] Remove non address bits when looking up memory regions"

This reverts commit 6f5ce43b433706c3ae5c37022d6c0964b6bfadf8 due to
build failure on Windows.

[lldb] Remove non address bits when looking up memory regions

On AArch64 we have various things using the non address bits
of pointers. This means when you lookup their containing region
you won't f

[lldb] Remove non address bits when looking up memory regions

On AArch64 we have various things using the non address bits
of pointers. This means when you lookup their containing region
you won't find it if you don't remove them.

This changes Process GetMemoryRegionInfo to a non virtual method
that uses the current ABI plugin to remove those bits. Then it
calls DoGetMemoryRegionInfo.

That function does the actual work and is virtual to be overriden
by Process implementations.

A test case is added that runs on AArch64 Linux using the top
byte ignore feature.

Reviewed By: omjavaid

Differential Revision: https://reviews.llvm.org/D102757

show more ...

Reland "[lldb] Don't send invalid region addresses to lldb server"

This reverts commit c65627a1fe3be7521fc232d633bb6df577f55269.

The test immediately after the new invalid symbol test was
failing o

Reland "[lldb] Don't send invalid region addresses to lldb server"

This reverts commit c65627a1fe3be7521fc232d633bb6df577f55269.

The test immediately after the new invalid symbol test was
failing on Windows. This was because when we called
VirtualQueryEx to get the region info for 0x0,
even if it succeeded we would call GetLastError.

Which must have picked up the last error that was set while
trying to lookup "not_an_address". Which happened to be 2.
("The system cannot find the file specified.")

To fix this only call GetLastError when we know VirtualQueryEx
has failed. (when it returns 0, which we were also checking for anyway)

Also convert memory region to an early return style
to make the logic clearer.

Reviewed By: labath, stella.stamenova

Differential Revision: https://reviews.llvm.org/D88229

show more ...

[lldb/Process/Windows] Attempting to kill exited/detached process in not an error

The lldb test-suite on Windows reports a 'CLEANUP ERROR' when attempting to kill
an exited/detached process. This ch

[lldb/Process/Windows] Attempting to kill exited/detached process in not an error

The lldb test-suite on Windows reports a 'CLEANUP ERROR' when attempting to kill
an exited/detached process. This change makes ProcessWindows consistent with
the other processes which only log the error. After this change a number of
'CLEANUP ERROR' messages are now removed.

Differential Revision: https://reviews.llvm.org/D84957

show more ...

[lldb] Fix build break in ProcessDebugger due to StringRef usage changes

[lldb][NFC] Fix all formatting errors in .cpp file headers

Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp ----------------------------------------

[lldb][NFC] Fix all formatting errors in .cpp file headers

Summary:
A *.cpp file header in LLDB (and in LLDB) should like this:
```
//===-- TestUtilities.cpp -------------------------------------------------===//
```
However in LLDB most of our source files have arbitrary changes to this format and
these changes are spreading through LLDB as folks usually just use the existing
source files as templates for their new files (most notably the unnecessary
editor language indicator `-*- C++ -*-` is spreading and in every review
someone is pointing out that this is wrong, resulting in people pointing out that this
is done in the same way in other files).

This patch removes most of these inconsistencies including the editor language indicators,
all the different missing/additional '-' characters, files that center the file name, missing
trailing `===//` (mostly caused by clang-format breaking the line).

Reviewers: aprantl, espindola, jfb, shafik, JDevlieghere

Reviewed By: JDevlieghere

Subscribers: dexonsmith, wuzish, emaste, sdardis, nemanjai, kbarton, MaskRay, atanasyan, arphaman, jfb, abidh, jsji, JDevlieghere, usaxena95, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D73258

show more ...

[LLDB] [Windows] Fix Windows-specific race condition in LLDB for session lifetime

This can e.g. happen if the debugged executable exits before the initial
stop, e.g. if it fails to load dependent DL

[LLDB] [Windows] Fix Windows-specific race condition in LLDB for session lifetime

This can e.g. happen if the debugged executable exits before the initial
stop, e.g. if it fails to load dependent DLLs.

Add a virtual destructor to ProcessDebugger and let it clean up the
session, and make ProcessWindows::OnExitProcess call
ProcessDebugger::OnExitProcess for shared parts.

Fix suggestion by Adrian McCarthy.

Differential Revision: https://reviews.llvm.org/D69503

show more ...

Try again to move common functionality from ProcessWindows into ProcessDebugger

This reverts commit ed499a36b67cf46cbf66052cfe374c80a595f1c1 and addresses
a problem causing a Windows build bot to ha

Try again to move common functionality from ProcessWindows into ProcessDebugger

This reverts commit ed499a36b67cf46cbf66052cfe374c80a595f1c1 and addresses
a problem causing a Windows build bot to hang.

llvm-svn: 365592

show more ...

Move common functionality from processwindows into processdebugger

Summary:
This change extracts functionalities from processwindows into a
introduced processdebugger that can be reused in native pr

Move common functionality from processwindows into processdebugger

Summary:
This change extracts functionalities from processwindows into a
introduced processdebugger that can be reused in native process
debugging.

The main reason is that the native process debugging
can't directly be based on processwindows or be implemented
as a pass-through to this plugin since the plugin has ties to
Target and Process classes that are needed in host debugging but
not necessary in native debugging.

Reviewers: labath, Hui, jfb, clayborg, amccarth

Reviewed By: labath

Subscribers: amccarth, dexonsmith, mgorny, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D63166

llvm-svn: 364210

show more ...