Revert "[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute." and follow-ups

This reverts commit 9429b67b8e300e638d7828bbcb95585f85c4df4d.

It broke

Revert "[NFC] Improve FileSpec internal APIs and usage in preparation for adding caching of resolved/absolute." and follow-ups

This reverts commit 9429b67b8e300e638d7828bbcb95585f85c4df4d.

It broke the build on Windows, see comments on https://reviews.llvm.org/D130309

It also reverts these follow-ups:

Revert "Fix buildbot breakage after https://reviews.llvm.org/D130309."
This reverts commit f959d815f4637890ebbacca379f1c38ab47e4e14.

Revert "Fix buildbot breakage after https://reviews.llvm.org/D130309."
This reverts commit 0bbce7a4c2d2bff622bdadd4323f93f5d90e6d24.

Revert "Cache the value for absolute path in FileSpec."
This reverts commit dabe877248b85b34878e75d5510339325ee087d0.

show more ...

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

The FileSpect APIs allow users to modify instance variables directly by getting a non const ref

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

The FileSpect APIs allow users to modify instance variables directly by getting a non const reference to the directory and filename instance variables. This makes it impossibly to control all of the times the FileSpec object is modified so we can clear the cache. This patch modifies the APIs of FileSpec so no one can modify the directory or filename 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/D130309

show more ...

[lldb/dyld-posix] Avoid reading the module list in inconsistent states

New glibc versions (since 2.34 or including this
<https://github.com/bminor/glibc/commit/ed3ce71f5c64c5f07cbde0ef03554ea8950d8f

[lldb/dyld-posix] Avoid reading the module list in inconsistent states

New glibc versions (since 2.34 or including this
<https://github.com/bminor/glibc/commit/ed3ce71f5c64c5f07cbde0ef03554ea8950d8f2c>
patch) trigger the rendezvous breakpoint after they have already added
some modules to the list. This did not play well with our dynamic
loader plugin which was doing a diff of the the reported modules in the
before (RT_ADD) and after (RT_CONSISTENT) states. Specifically, it
caused us to miss some of the modules.

While I think the old behavior makes more sense, I don't think that lldb
is doing the right thing either, as the documentation states that we
should not be expecting a consistent view in the RT_ADD (and RT_DELETE)
states.

Therefore, this patch changes the lldb algorithm to compare the module
list against the previous consistent snapshot. This fixes the previous
issue, and I believe it is more correct in general. It also reduces the
number of times we are fetching the module info, which should speed up
the debugging of processes with many shared libraries.

The change in RefreshModules ensures we don't broadcast the loaded
notification for the dynamic loader (ld.so) module more than once.

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

show more ...

[lldb] fix stepping through POSIX trampolines

The DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan() function was
doing the symbol lookup using the demangled name. This stopped working
with http

[lldb] fix stepping through POSIX trampolines

The DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan() function was
doing the symbol lookup using the demangled name. This stopped working
with https://reviews.llvm.org/D118814. To get things working again, just
use the mangled name for the lookup instead.

Reviewed By: labath

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

show more ...

Add the ability to debug through an exec into ld

A previous commit enabled LLDB to be able to debug a program launched via ld: https://reviews.llvm.org/D108061.

This commit adds the ability to debu

Add the ability to debug through an exec into ld

A previous commit enabled LLDB to be able to debug a program launched via ld: https://reviews.llvm.org/D108061.

This commit adds the ability to debug a program launched via ld when it happens during an exec into the dynamic loader. There was an issue where after the exec we would locate the rendezvous structure right away but it didn't contain any valid values and we would try to set the dyanamic loader breakpoint at address zero. This patch fixes that and adds a test.

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

show more ...

[lldb] Avoid duplicate vdso modules when opening core files

When opening core files (and also in some other situations) we could end
up with two vdso modules. This could happen because the vdso modu

[lldb] Avoid duplicate vdso modules when opening core files

When opening core files (and also in some other situations) we could end
up with two vdso modules. This could happen because the vdso module is
very special, and over the years, we have accumulated various ways to
load it.

In D10800, we added one mechanism for loading it, which took the form of
a generic load-from-memory capability. Unfortunately loading an elf file
from memory is not possible (because the loader never loads the entire
file), and our attempts to do so were causing crashes. So, in D34352, we
partially reverted D10800 and implemented a custom mechanism specific to
the vdso.

Unfortunately, enough of D10800 remained such that, under the right
circumstances, it could end up loading a second (non-functional) copy of
the vdso module. This happened when the process plugin did not support
the extended MemoryRegionInfo query (added in D22219, to workaround a
different bug), which meant that the loader plugin was not able to
recognise that the linux-vdso.so.1 module (this is how the loader calls
it) is in fact the same as the [vdso] module (the name used in
/proc/$PID/maps) we loaded before. This typically happened in a core
file, as they don't store this kind of information.

This patch fixes the issue by completing the revert of D10800 -- the
memory loading code is removed completely. It also reduces the scope of
the hackaround introduced in D22219 -- it isn't completely sound and is
only relevant for fairly old (but still supported) versions of android.

I added the memory loading logic to the wasm dynamic loader, which has
since appeared and is relying on this feature (it even has a test). As
far as I can tell loading wasm modules from memory is possible and
reliable. MachO memory loading is not affected by this patch, as it uses
a completely different code path.

Since the scenarios/patches I described came without test cases, I have
created two new gdb-client tests cases for them. They're not
particularly readable, but right now, this is the best way we can
simulate the behavior (bugs) of a particular dynamic linker.

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

show more ...

[lldb] Remove process->LoadModules call from DynamicLoaderPOSIXDYLD

The call is useless, as any modules loaded there will be removed in
ResolveExecutableModule. Modules will be reloaded again throug

[lldb] Remove process->LoadModules call from DynamicLoaderPOSIXDYLD

The call is useless, as any modules loaded there will be removed in
ResolveExecutableModule. Modules will be reloaded again through the
GetLoadedModuleList call in DYLDRendezvous.cpp.

show more ...

[lldb] Rename Logging.h to LLDBLog.h and clean up includes

Most of our code was including Log.h even though that is not where the
"lldb" log channel is defined (Log.h defines the generic logging
inf

[lldb] Rename Logging.h to LLDBLog.h and clean up includes

Most of our code was including Log.h even though that is not where the
"lldb" log channel is defined (Log.h defines the generic logging
infrastructure). This worked because Log.h included Logging.h, even
though it should.

After the recent refactor, it became impossible the two files include
each other in this direction (the opposite inclusion is needed), so this
patch removes the workaround that was put in place and cleans up all
files to include the right thing. It also renames the file to LLDBLog to
better reflect its purpose.

show more ...

[lldb] Convert "LLDB" log channel to the new API

Fix bugprone argument comments.

Identified by bugprone-argument-comment.

Remove ConstString from DynamicLoader, JITLoader and Instruction plugin names

[lldb] Return StringRef from PluginInterface::GetPluginName

There is no reason why this function should be returning a ConstString.

While modifying these files, I also fixed several instances where

[lldb] Return StringRef from PluginInterface::GetPluginName

There is no reason why this function should be returning a ConstString.

While modifying these files, I also fixed several instances where
GetPluginName and GetPluginNameStatic were returning different strings.

I am not changing the return type of GetPluginNameStatic in this patch, as that
would necessitate additional changes, and this patch is big enough as it is.

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

show more ...

Fix rendezvous for rebase_exec=true case

When rebase_exec=true in DidAttach(), all modules are loaded
before the rendezvous breakpoint is set, which means the
LoadInterpreterModule() method is not c

Fix rendezvous for rebase_exec=true case

When rebase_exec=true in DidAttach(), all modules are loaded
before the rendezvous breakpoint is set, which means the
LoadInterpreterModule() method is not called and m_interpreter_module
is not initialized.

This causes the very first rendezvous breakpoint hit with
m_initial_modules_added=false to accidentally unload the
module_sp that corresponds to the dynamic loader.

This bug (introduced in D92187) was causing the rendezvous
mechanism to not work in Android 28. The mechanism works
fine on older/newer versions of Android.

Test: Verified rendezvous on Android 28 and 29
Test: Added dlopen test

Reviewed By: labath

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

show more ...

[lldb] Remove PluginInterface::GetPluginVersion

In all these years, we haven't found a use for this function (it has
zero callers). Lets just remove the boilerplate.

Differential Revision: https://

[lldb] Remove PluginInterface::GetPluginVersion

In all these years, we haven't found a use for this function (it has
zero callers). Lets just remove the boilerplate.

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

show more ...

[lldb] Add support for debugging via the dynamic linker.

This patch adds support for shared library load when the executable is
called through ld.so.

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

[lldb] Add support for debugging via the dynamic linker.

This patch adds support for shared library load when the executable is
called through ld.so.

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

show more ...

Revert "[lldb] Add support for debugging via the dynamic linker."

This reverts commit 1cbdc07ec015d83038c08dc562d55eccfd808591.

Buildbot failure started after this patch with failure in
api/multith

Revert "[lldb] Add support for debugging via the dynamic linker."

This reverts commit 1cbdc07ec015d83038c08dc562d55eccfd808591.

Buildbot failure started after this patch with failure in
api/multithreaded/TestMultithreaded.py:
https://lab.llvm.org/buildbot/#/builders/68/builds/17556

show more ...

[lldb] Add support for debugging via the dynamic linker.

This patch adds support for shared library load when the executable is
called through ld.so.

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

[lldb] Add support for debugging via the dynamic linker.

This patch adds support for shared library load when the executable is
called through ld.so.

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

show more ...

Revert "Make sure the interpreter module was loaded before making checks against it"

This reverts commit a83a825e9902b54b315870e9ed85723525208f09.

Make sure the interpreter module was loaded before making checks against it

This issue was introduced in https://reviews.llvm.org/D92187.
The guard I'm changing were is supposed to act when linux is

Make sure the interpreter module was loaded before making checks against it

This issue was introduced in https://reviews.llvm.org/D92187.
The guard I'm changing were is supposed to act when linux is loading the linker for the second time (due to differences in paths like symlinks).
This is done by checking `module_sp != m_interpreter_module.lock()` however this will be true when `m_interpreter_module` wasn't initialized, making linux unload the linker module (the most visible result here is that lldb will stop getting notified about new modules loaded by the process, because it can't set the rendezvous breakpoint again after the stepping over it once).
The `m_interpreter_module` is not getting initialize when it goes through this path: https://github.com/llvm/llvm-project/blob/dbfdb139f75470a9abc78e7c9faf743fdd963c2d/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L332, which happens when lldb was able to read the address from the dynamic section of the executable.

What I'm not sure about though, is if when we go through this path if we still load the linker twice on linux. If that's the case then it means we need to somehow set the m_interpreter_module instead of the fix I provide here. I've only tested this on Android.

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

show more ...

[lldb] [POSIX-DYLD] Update the cached exe path after attach

Fix the POSIX-DYLD plugin to update the cached executable path after
attaching. Previously, the path was cached in DYLDRendezvous
constru

[lldb] [POSIX-DYLD] Update the cached exe path after attach

Fix the POSIX-DYLD plugin to update the cached executable path after
attaching. Previously, the path was cached in DYLDRendezvous
constructor and not updated afterwards. This meant that if LLDB was
attaching to a process (e.g. via connecting to lldb-server), the code
stored the empty path before DidAttach() resolved it. The fix updates
the cached path in DidAttach().

This fixes a new instance of https://llvm.org/pr17880

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

show more ...

[lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

Explicitly consider the libraries reported on the initial rendezvous
breakpoint hit added. This is necessary on FreeBSD since the

[lldb] [POSIX-DYLD] Add libraries from initial rendezvous brkpt hit

Explicitly consider the libraries reported on the initial rendezvous
breakpoint hit added. This is necessary on FreeBSD since the dynamic
loader issues only a single 'consistent' state rendezvous breakpoint hit
for all the libraries present in DT_NEEDED. It is also helpful on Linux
where it ensures that ld-linux is considered loaded as well
as the shared system libraries reported afterwards.

Reenable memory maps on FreeBSD since this fixed the issue triggered
by them.

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

show more ...

[lldb/Plugins] Rename initializers to match their plugin name.

Use LLDB_PLUGIN_DEFINE_ADV to make the name of the generated initializer
match the name of the plugin. This is a step towards generatin

[lldb/Plugins] Rename initializers to match their plugin name.

Use LLDB_PLUGIN_DEFINE_ADV to make the name of the generated initializer
match the name of the plugin. This is a step towards generating the
initializers with a def file. I'm landing this change in pieces so I can
narrow down what exactly breaks the Windows bot.

show more ...

Revert "[lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin"

This is still failing spectacularly on the Windows bot and I still have
no clue what's going on.

Re-land "[lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin"

This patch changes the way we initialize and terminate the plugins in
the system initializer. It uses an approach s

Re-land "[lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin"

This patch changes the way we initialize and terminate the plugins in
the system initializer. It uses an approach similar to LLVM's
TARGETS_TO_BUILD with a def file that enumerates the plugins.

The previously landed patch got reverted because it was lacking:

(1) A plugin definition for the Objective-C language runtime,
(2) The dependency between the Static and WASM dynamic loader,
(3) Explicit initialization of ScriptInterpreterNone for lldb-test.

All issues have been addressed in this patch.

Differential revision: https://reviews.llvm.org/D73067

show more ...

Revert "[lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin"

This temporarily reverts commit 7d6da329dee1eda1761430d9097d1323f32c4c0c
because it's causing test failures on the b

Revert "[lldb/CMake] Auto-generate the Initialize and Terminate calls for plugin"

This temporarily reverts commit 7d6da329dee1eda1761430d9097d1323f32c4c0c
because it's causing test failures on the bots.

show more ...