ConvertToStructuredArray() relies on its caller to deallocate the heap-allocated object pointer it returns. One of its call-sites, in GetRenumberedThreadIds(), fails to deallocate causing a memory/resource leak. Fix the memory leak by converting the return type to shared_ptr, and clean up the rest of the file to use the typedef-ed shared_ptr types for StructuredData for safety and consistency.
Differential Revision: https://reviews.llvm.org/D131900
This patch fixes:
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h:34:5:
error: default label in switch which covers all enumeration values
[-Werror,-Wcovered-switch-default]
and:
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp:194:21:
error: comparison of integers of different signs: 'int' and 'size_t'
(aka 'unsigned long') [-Werror,-Wsign-compare]
Add:
- `EmulateInstructionRISCV`, which can be used for riscv32 and riscv64.
- Add unittests for EmulateInstructionRISCV.
Note: Compressed instructions set (RVC) was still not supported in this patch.
Reviewed By: DavidSpickett
Differential Revision: https://reviews.llvm.org/D131759
In this switch case we didn't handle possible errors in `ResumeThread()`, it's hard to get helpful information when it goes wrong.
Reviewed By: DavidSpickett
Differential Revision: https://reviews.llvm.org/D131946
Some architectures do not have a flag register (like riscv).
In this case, we should set it to `baton.m_register_values.end()` to avoid nullptr exception.
Reviewed By: DavidSpickett
Differential Revision: https://reviews.llvm.org/D131945
On macOS, LLDB uses the DebugSymbols.framework to locate symbol rich
dSYM bundles. [1] The framework uses a variety of methods, one of them
calling into a binary or shell script to locate (and download) dSYMs.
Internally at Apple, that tool is called dsymForUUID and for simplicity
I'm just going to refer to it that way here too, even though it can be
be an arbitrary executable.
The most common use case for dsymForUUID is to fetch symbols from the
network. This can take a long time, and because the calls to the
DebugSymbols.framework are blocking, it takes a while to launch the
process. This is expected and therefore many people don't use this
functionality, but instead use add-dsym when they want symbols for a
given frame, backtrace or module. This is a little faster because you're
only fetching symbols for the module you care about, but it's still a
slow, blocking operation.
This patch introduces a hybrid approach between the two. When
symbols.enable-background-lookup is enabled, lldb will do the equivalent
of add-dsym in the background for every module that shows up in the
backtrace but doesn't have symbols for. From the user's perspective
there is no slowdown, because the process launches immediately, with
whatever symbols are available. Meanwhile, more symbol information is
added over time as the background fetching completes.
[1] https://lldb.llvm.org/use/symbols.html
rdar://76241471
Differential revision: https://reviews.llvm.org/D131328
Use the Initialize/Terminate pattern for the global thread pool to make
sure it gets flushed during teardown.
Differential revision: https://reviews.llvm.org/D131407
The per-PSB packet decoding logic was wrong because it was assuming that pt_insn_get_sync_offset was being udpated after every PSB. Silly me, that is not true. It returns the offset of the PSB packet after invoking pt_insn_sync_forward regardless of how many PSBs are visited later. Instead, I'm now following the approach described in https://github.com/intel/libipt/blob/master/doc/howto_libipt.md#parallel-decode for parallel decoding, which is basically what we need.
A nasty error that happened because of this is that when we had two PSBs (A and B), the following was happening
1. PSB A was processed all the way up to the end of the trace, which includes PSB B.
2. PSB B was then processed until the end of the trace.
The instructions emitted by step 2. were also emitted as part of step 1. so our trace had duplicated chunks. This problem becomes worse when you many PSBs.
As part of making sure this diff is correct, I added some other features that are very useful.
- Added a "synchronization point" event to the TraceCursor, so we can inspect when PSBs are emitted.
- Removed the single-thread decoder. Now the per-cpu decoder and single-thread decoder use the same code paths.
- Use the query decoder to fetch PSBs and timestamps. It turns out that the pt_insn_sync_forward of the instruction decoder can move past several PSBs (this means that we could skip some TSCs). On the other hand, the pt_query_sync_forward method doesn't skip PSBs, so we can get more accurate sync events and timing information.
- Turned LibiptDecoder into PSBBlockDecoder, which decodes single PSB blocks. It is the fundamental processing unit for decoding.
- Added many comments, asserts and improved error handling for clarity.
- Improved DecodeSystemWideTraceForThread so that a TSC is emitted always before a cpu change event. This was a bug that was annoying me before.
- SplitTraceInContinuousExecutions and FindLowestTSCInTrace are now using the query decoder, which can identify precisely each PSB along with their TSCs.
- Added an "only-events" option to the trace dumper to inspect only events.
I did extensive testing and I think we should have an in-house testing CI. The LLVM buildbots are not capable of supporting testing post-mortem traces of hundreds of megabytes. I'll leave that for later, but at least for now the current tests were able to catch most of the issues I encountered when doing this task.
A sample output of a program that I was single stepping is the following. You can see that only one PSB is emitted even though stepping happened!
```
thread #1: tid = 3578223
0: (event) trace synchronization point [offset = 0x0xef0]
a.out`main + 20 at main.cpp:29:20
1: 0x0000000000402479 leaq -0x1210(%rbp), %rax
2: (event) software disabled tracing
3: 0x0000000000402480 movq %rax, %rdi
4: (event) software disabled tracing
5: (event) software disabled tracing
6: 0x0000000000402483 callq 0x403bd4 ; std::vector<int, std::allocator<int>>::vector at stl_vector.h:391:7
7: (event) software disabled tracing
a.out`std::vector<int, std::allocator<int>>::vector() at stl_vector.h:391:7
8: 0x0000000000403bd4 pushq %rbp
9: (event) software disabled tracing
10: 0x0000000000403bd5 movq %rsp, %rbp
11: (event) software disabled tracing
```
This is another trace of a long program with a few PSBs.
```
(lldb) thread trace dump instructions -E -f thread #1: tid = 3603082
0: (event) trace synchronization point [offset = 0x0x80]
47417: (event) software disabled tracing
129231: (event) trace synchronization point [offset = 0x0x800]
146747: (event) software disabled tracing
246076: (event) software disabled tracing
259068: (event) trace synchronization point [offset = 0x0xf78]
259276: (event) software disabled tracing
259278: (event) software disabled tracing
no more data
```
Differential Revision: https://reviews.llvm.org/D131630
When targeting macOS Ventura, ld64 will use authenticated fixups for
x86_64 as well as arm64 (where that has always been the case). This
results in test failures when using an Xcode 14 toolchain on an Intel
mac running macOS Ventura:
Failed Tests (3):
lldb-api :: commands/target/basic/TestTargetCommand.py
lldb-api :: lang/c/global_variables/TestGlobalVariables.py
lldb-api :: lang/cpp/char8_t/TestCxxChar8_t.py
Rather than trying to come up with a sophisticated decorator based off
the deployment target, I marked them all as skipped with a comment
explaining why.
Differential revision: https://reviews.llvm.org/D131741
This patch should just a crash caused by a null pointer dereferencing
when dumping a type. It makes sure that the pointer is valid.
rdar://97455134
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch improve exception reporting when loading a crash report in a
scripted process. Now, we parse the `exception` dictionary from the
crash report use it the create a higher fidelity `MachException` stop info.
This patch also updates the test to reflect that change.
rdar://97096486
Differential Revision: https://reviews.llvm.org/D131086
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch removes the system library names and mangled symbol from
the expected output for the interactive crashlog tests.
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch parses CrashLog exception data from the raw
text format and adapts it to the new JSON format.
This is necessary for feature parity between the 2 formats.
Differential Revision: https://reviews.llvm.org/D131719
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
A missing call to `format` resulted in curly braces getting printed in
the reason a test was modified by a decorator. For example it would
print "{} unconditionally" instead of "skipping unconditionally" for
tests that were marked as such.
A spiritual follow up to D131032. I noticed some regex could be simplified.
This does some of the following:
1. Removes unused capture groups
2. Uses non-capturing `(?:...)` groups where grouping is needed but capturing isn't
3. Removes trailing `.*`
4. Uses `\d` over `[0-9]`
5. Uses raw strings
6. Uses `{N,}` to indicate N-or-more
Also improves the call site of a `re.findall`.
Differential Revision: https://reviews.llvm.org/D131305
Pavel Labath taught me that clang-format sorts headers automatically
using llvm's rules, and it's better not to have spaces between
So in this diff I'm removing those spaces and formatting them as well.
I used `clang-format -i` to format these files.
Fixed an inconsistency between D130985 and D130342
This should be a follow-up of D130985
Reviewed By: DavidSpickett
Differential Revision: https://reviews.llvm.org/D131667
https://reviews.llvm.org/D131658 found a bug in
ReadPseudoRegisterValue which would mean we read out
of bounds if the s register number was high enough.
This adds a memory check to vpush-1-thumb, which
should have been doing that anyway. Then copies that
test and uses the last 4 s registers instead.
Without the mentioned fix we see random values in
the final memory, with the fix it passes.
Reviewed By: fixathon
Differential Revision: https://reviews.llvm.org/D131663
Functionally broken code for reading and writing registers, likely due to typos,
and could cause out-of-bounds memory access.
Differential Revision: https://reviews.llvm.org/D131658
Added:
- Take RISC-V `ebreak` instruction as breakpoint trap code, so our breakpoint works as expected now.
Further work:
- RISC-V does not support hardware single stepping yet. A software implementation may come in future PR.
- Add support for RVC extension (the trap code, etc.).
Reviewed By: DavidSpickett
Differential Revision: https://reviews.llvm.org/D131566
This patch is based on the minimal extract of D128250.
What is implemented:
- Use the same register layout as Linux kernel and mock read/write for `x0` register (the always zero register).
- Refactor some duplicate code, and delete unused register definitions.
Reviewed By: DavidSpickett
Differential Revision: https://reviews.llvm.org/D130342
This fixes the stand-alone build configuration where LLVM_MAIN_SRC_DIR
does not exist.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D124314
This patch introduces a new option to the crashlog command to get the
the script version.
Since `crashlog.py` is not actually versioned, this returns lldb's
version instead.
rdar://98392669
Differential Revision: https://reviews.llvm.org/D131542
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Static code inspection guided fixes for the following issues:
- dead code
- buffer not null-terminated
- null-dereference
- out-of-bounds access
Differential Revision: https://reviews.llvm.org/D131554
Patch adds support for fadd, fsub, fdiv, fmul and fcmp to IR interpreter.
~~~
OS Laboratory. Huawei RRI. Saint-Petersburg
Reviewed By: clayborg
Differential Revision: https://reviews.llvm.org/D126359
We don't want the DEFINE_ macros or the array registers
being clang formatted.
RegisterInfos_arm64.h was missing the off annotation and
RegisterInfos_arm64_sve.h needed the off moving to before
the macro definitions.
1438639a2f removed a test
that was using undefined behaviour setting a non-typed enum
to a value outside its known range.
That test also checked if we formatted the value properly
when it could contain >1 valid enum value.
I don't think there's anything special about how we format
typed vs non-typed enums so I'm adding a test for ScopedEnum
that will expect to see 2 enum values plus extra.
Reviewed By: labath, Michael137, shafik
Differential Revision: https://reviews.llvm.org/D131472
This patch changes the CrashLogParser class to be both the base class
and a Factory for the JSONCrashLogParser & TextCrashLogParser.
That should help remove some code duplication and ensure both class
have a parse method.
Differential Revision: https://reviews.llvm.org/D131085
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
Sometimes, it can happen that a crash report has null images in its list
of used binaries. This manifests like such:
```
0x0 - 0xffffffffffffffff ??? (*) <00000000-0000-0000-0000-000000000000> ???
```
When fetching debug symbols to symbolicate the crashlog stackframe,
having null images causes `dsymForUUID` to hang for few seconds.
This patch addresses that by skipping null images from being load by the
scripted process.
rdar://97419487
Differential Revision: https://reviews.llvm.org/D131038
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch introduces a new option for the interactive crashlog mode,
that will prevent it from dumping the `process status` & `thread backtrace`
output to the debugger console.
This is necessary when lldb in running from an IDE, to prevent flooding
the console with information that should be already present in the UI.
rdar://96813296
Differential Revision: https://reviews.llvm.org/D131036
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
In can happen when creating stackshot crash report that that key is missing.
Moreover, we try to parse that key but don't use it, or need it, since we
fetch images and symbolicate the stackframes using the binaries UUIDs.
This is why this patch removes everything that is related to the
`process_path`/`procPath` parsing.
rdar://95054188
Differential Revision: https://reviews.llvm.org/D131033
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch updates the regular expression matching stackframes in
crashlog to allow addresses that are 7 characters long and more (vs. 8
characters previously).
It changes the `0x[0-9a-fA-F]{7}[0-9a-fA-F]+` by `0x[0-9a-fA-F]{7,}`.
rdar://97684839
Differential Revision: https://reviews.llvm.org/D131032
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch allows the crashlog script to surface its errors to lldb by
using the provided SBCommandReturnObject argument.
rdar://95048193
Differential Revision: https://reviews.llvm.org/D129614
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch introduces a new flag for the interactive crashlog mode, that
allow the user to specify, which target to use to create the scripted
process.
This can be very useful when lldb already have few targets created:
Instead of taking the first one (zeroth index), we will use that flag to
create a new target. If the user didn't provide a target path, we will rely
on the symbolicator to create a targer.If that fails and there are already
some targets loaded in lldb, we use the first one.
rdar://94682869
Differential Revision: https://reviews.llvm.org/D129611
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
This patch should fix the interactive crashlog test by checking in the
binary as a yaml to regeneate the binary with the addresses and offsets
when running the test.
rdar://93655633
Differential Revision: https://reviews.llvm.org/D129603
Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>
bot. This happens because they are building a long result into
a Python string, and the asan checker is making that very slow.
The last two tests here are both slow, but the 'test_command' is the
really slow one. I'm going to start disabling just that one and see
if that gets the ASAN bots clean.
This removes some error-prone repetition in
FormatManager::GetPossibleMatches, where the same three boolean flags
are passed in a row multiple times as arguments to recursive calls to
GetPossibleMatches.
Instead of:
```
// same flags, but with did_strip_typedef set to true.
GetPossibleMatches(..., did_strip_ptr, did_strip_ref, true);
```
we can now say
```
GetPossibleMatches(..., current_flags.WithStrippedTypedef());
```
which hopefully makes the intent clearer, and more readable in case we
add another flag.
Reviewed by: DavidSpickett, labath
Differential Revision: https://reviews.llvm.org/D131459
- Reduce indentation
- Extract caching of the DbgShellCommand and the dsymForUUID executable
(or equivalent)
- Check the DBGShellCommands before falling back to
/usr/local/bin/dsymForUUID
- Don't check ~rc/bin/dsymForUUID
- Improve error reporting
- Don't cache the value of LLDB_APPLE_DSYMFORUUID_EXECUTABLE
Differential revision: https://reviews.llvm.org/D131303
Setting an enum without a fixed underlying type to a value which is outside the
value range is undefined behavior.
The initializer needs to be a constant expression and therefore this was always
ill-formed we just were not diagnosing it before.
See D130058 and D131307 for more details.
Differential Revision: https://reviews.llvm.org/D131460
Currently a FileSpecList::FindFileIndex(...) will only match the specified FileSpec if:
- it has filename and directory and both match exactly
- if has a filename only and any filename in the list matches
Because of this, we modify our breakpoint resolving so it can handle relative paths by doing some extra code that removes the directory from the FileSpec when searching if the path is relative.
This patch is intended to fix breakpoints so they work as users expect them to by adding the following features:
- allow matches to relative paths in the file list to match as long as the relative path is at the end of the specified path at valid directory delimiters
- allow matches to paths to match if the specified path is relative and shorter than the file paths in the list
This allows us to remove the extra logic from BreakpointResolverFileLine.cpp that added support for setting breakpoints with relative paths.
This means we can still set breakpoints with relative paths when the debug info contains full paths. We add the ability to set breakpoints with full paths when the debug info contains relative paths.
Debug info contains "./a/b/c/main.cpp", the following will set breakpoints successfully:
- /build/a/b/c/main.cpp
- a/b/c/main.cpp
- b/c/main.cpp
- c/main.cpp
- main.cpp
- ./c/main.cpp
- ./a/b/c/main.cpp
- ./b/c/main.cpp
- ./main.cpp
This also ensures that we won't match partial directory names, if a relative path is in the list or is used for the match, things must match at the directory level.
The breakpoint resolving code will now use the new FileSpecList::FindCompatibleIndex(...) function to allow this fuzzy matching to work for breakpoints.
Differential Revision: https://reviews.llvm.org/D130401
@clayborg found a potential race condition when setting a static
variable. The fix seems simply to use call_once.
All relevant tests pass.
Differential Revision: https://reviews.llvm.org/D131081
The header from 62e0681afb does something with
LLVM_FALLTHROUGH. Now that llvm-project has switched to C++17 and
LLVM_FALLTHROUGH uses have been migrated to [[fallthrough]], the header is
unneeded.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D131422
This test previously was expected to fail on windows. As of my previous
patch (1d2a62afaf) this test now passes on
windows consistently. This patch adjusts the expectations of the test
accordingly.
Current application of bitwise-OR to a binary mask always results in True, which seems
inconsistent with the intent of the statement, a likely typo.
Differential Revision: https://reviews.llvm.org/D131312
Make constructors of the Process and its subclasses class protected,
to prevent accidentally constructing Process on stack when it could be
afterwards accessed via a shared_ptr (since it uses
std::enable_shared_from_this<>).
The only place where a stack allocation was used were unittests,
and fixing them via declaring an explicit public constructor
in the respective mock classes is trivial.
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D131275
Change the `<enum>_to_str` functions in lldbutil to be dynamic.
Instead of a manually maintained if/elif switch, the functions now perform
lookup in a dynamically generated dict. The names are determined by stripping
the enum's prefix, and then lowercasing the remaining suffix, ex:
eStateRunning -> "running"
Differential Revision: https://reviews.llvm.org/D131138
Patch the following issues found by static code inspection:
- Unchecked return values from lib calls
- Passing potentially negative arg into a function that requires non-negative input
- Possible socket double-close
Differential Revision: https://reviews.llvm.org/D131294
Also make the soft toolchain requirements hard. This allows
us to use C++17 features in LLVM now.
If we find patterns with C++17 that improve readability
it should be recommended in the coding standards.
Reviewed By: jhenderson, cor3ntin, MaskRay
Differential Revision: https://reviews.llvm.org/D130689
Jason noted that the stop message we print for a memory high water mark
notification (EXC_RESOURCE) could be clearer. Currently, the stop
reason looks like this:
* thread #3, queue = 'com.apple.CFNetwork.LoaderQ', stop reason =
EXC_RESOURCE RESOURCE_TYPE_MEMORY (limit=14 MB, unused=0x0)
It's hard to read the message because the exception and the type
(EXC_RESOURCE RESOURCE_TYPE_MEMORY) blend together. Additionally, the
"observed=0x0" should not be printed for memory limit exceptions.
I wanted to continue to include the resource type from
<kern/exc_resource.h> while also explaining what it actually is. I used
the wording from the comments in the header. With this path, the stop
reason now looks like this:
* thread #5, stop reason = EXC_RESOURCE (RESOURCE_TYPE_MEMORY: high
watermark memory limit exceeded) (limit=14 MB)
rdar://40466897
Differential revision: https://reviews.llvm.org/D131130
This commit combines the initial commit (7c240de609af), a fix for x86_64 Linux
(3a0581501e76) and a fix for thinko in a last minute rewrite that I really
should have run the testsuite on.
Also, make sure that all the "I need to step over watchpoint" plans execute
before we call a public stop. Otherwise, e.g. if you have N watchpoints and
a Signal, the signal stop info will get us to stop with the watchpoints in a
half-done state.
Differential Revision: https://reviews.llvm.org/D130674
Add a new "kernel" section with following schema.
```
"kernel": {
"loadAddress"?: decimal | hex string | string decimal
# This is optional. If it's not specified, use default address 0xffffffff81000000.
"file": string
# path to the kernel image
}
```
Here's more details of the diff:
- If "kernel" section exist, it means current tracing mode is //KernelMode//.
- If tracing mode is //KernelMode//, the "processes" section must be empty and the "kernel" and "cpus" section must be provided. This is tested with `TestTraceLoad`.
- "kernel" section is parsed and turned into a new process with a single module which is the kernel image. The kernel process has N fake threads, one for each cpu.
Reviewed By: wallace
Differential Revision: https://reviews.llvm.org/D130805
This reverts commit 967df65a36.
This fixes test/Shell/SymbolFile/NativePDB/find-functions.cpp. When
looking up functions with the PDB plugins, if we are looking for a
full function name, we should use `GetName` to populate the `name`
field instead of `GetLookupName` since `GetName` has the more
complete information.
Context:
When setting a breakpoint by name, we invoke Module::FindFunctions to
find the function(s) in question. However, we use a Module::LookupInfo
to first process the user-provided name and figure out exactly what
we're looking for. When we actually perform the function lookup, we
search for the basename. After performing the search, we then filter out
the results using Module::LookupInfo::Prune. For example, given
a:🅱️:foo we would first search for all instances of foo and then filter
out the results to just names that have a:🅱️:foo in them. As one can
imagine, this involves a lot of debug info processing that we do not
necessarily need to be doing. Instead of doing one large post-processing
step after finding each instance of `foo`, we can filter them as we go
to save time.
Some numbers:
Debugging LLDB and placing a breakpoint on
llvm::itanium_demangle::StringView::begin without this change takes
approximately 70 seconds and resolves 31,920 DIEs. With this change,
placing the breakpoint takes around 30 seconds and resolves 8 DIEs.
Differential Revision: https://reviews.llvm.org/D129682
This check is clearly incorrect, there's no way you should have an
eStateConnected event left on the queue if you've already launched
and hit a breakpoint in the program. This check fails running remotely
on Darwin systems and on one remote Linux platform. And if we do
find this failing somewhere, we should fix the bogus eStateConnected,
not the test.
This reverts commit d959324e1e.
The target_include_directories in the clang-fuzzer CMake files
are set to PRIVATE instead of PUBLIC to prevent the clang buildbots
from breaking when symlinking clang into llvm.
The expression evaluator fuzzer itself has been modified to prevent a
bug that occurs when running it without a target.
The kernel was rejecting sizeof(struct GPR) as it was not a multiple of
8. Add a padding field to fix that.
One also wonders whether "cpsr" is right register name for aarch64.
This test was disabled because clang struggled to emit a Windows calling
convention when targeting an Apple environment. This test is now showing
up as an XPASS so someone must have fixed this.
Make LLDB resilient against failing dyld introspection SPIs:
- dyld_process_create_for_current_task
- dyld_process_snapshot_create_for_process
- dyld_process_snapshot_get_shared_cache
These can all fail and return a nullptr. Instead of having an assert,
which doesn't really make sense, as we have no control over whether
these calls succeed or not, bail out gracefully and use the fallback
logic.
rdar://98070414
Differential revision: https://reviews.llvm.org/D131110
Make TestModuleLoadedNotifys work with a dyld from the expanded shared
cache in the DeviceSupport directory. In that case the module path is:
~/Library/Developer/Xcode/iOS DeviceSupport/<...>/Symbols/usr/lib/dyld
instead of just:
/usr/lib/dyld
This makes the test pass when running against an embedded Darwin device.
Starting with macOS 13 CoreFoundation pulls in Foundation, so we
always get the Foundation object description.
rdar://96224141
Differential Revision: https://reviews.llvm.org/D131025
Add a function to make it easier to debug a test failure caused by an
unexpected stop reason. This is similar to the assertState helper that
was added in ce825e4674.
Before:
self.assertEqual(stop_reason, lldb.eStopReasonInstrumentation)
AssertionError: 5 != 10
After:
self.assertStopReason(stop_reason, lldb.eStopReasonInstrumentation)
AssertionError: signal (5) != instrumentation (10)
Differential revision: https://reviews.llvm.org/D131083
It may be useful to search symbol table entries by mangled instead
of demangled names. Add this optional functionality in the SymbolTable
functions.
Differential Revision: https://reviews.llvm.org/D130803
Refactor the mechanism for sending interrupt packets to send them
from async thread (i.e. the same thread that sends the continue packet
preceding them and that waits for the response), rather than from
the thread requesting the interrupt. This is going to become especially
important when using the vCtrlC packet as part of the non-stop protocol,
as -- unlike the simple ^c sent in the all-stop mode -- this packet
involves an explicit reply.
Suggested by Pavel Labath in D126614.
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D131075
Currently, lldb-server was opening the executable file to determine the
process architecture (to differentiate between 32 and 64 bit
architecture flavours). This isn't a particularly trustworthy source of
information (the file could have been changed since the process was
started) and it is not always available (file could be deleted or
otherwise inaccessible).
Unfortunately, ptrace does not give us a direct API to access the
process architecture, but we can still infer it via some of its
responses -- given that the general purpose register set of 64-bit
applications is larger [citation needed] than the GPR set of 32-bit
ones, we can just ask for the application GPR set and check its size.
This is what this patch does.
Differential Revision: https://reviews.llvm.org/D130985
Add bindings for the `TraceCursor` to allow for programatic traversal of
traces.
This diff adds bindings for all public `TraceCursor` methods except
`GetHwClock` and also adds `SBTrace::CreateNewCursor`. A new unittest
has been added to TestTraceLoad.py that uses the new `SBTraceCursor` API
to test that the sequential and random access APIs of the `TraceCursor`
are equivalent.
This diff depends on D130925.
Test Plan:
`ninja lldb-dotest && ./bin/lldb-dotest -p TestTraceLoad`
Differential Revision: https://reviews.llvm.org/D130930
Slava Gurevich noticed this dead code I wrote in jmp_to_reg_p
is never executed, is duplicated, and has comments that seem to
describe the opposite behavior. Remove the dead code that cannot
be executed.
Differential Revision: https://reviews.llvm.org/D131029
D128477 adds the control flow kind for `Instruction` and displays this
in the `thread trace dump instruction -k` command.
This diff exposes the control flow kind via the new
`SBInstruction::GetControlFlowKind` method.
I've expanded `TestDisassembleRawData` to test this method, but please
let me know if there are any other unittests that should also be updated.
Test Plan:
`./bin/lldb-dotest -p TestDisassembleRawData`
Differential Revision: https://reviews.llvm.org/D131005
This either needs to be static, or forced inline, or in a separate
source file. Given that we only have one function in this
UuidCompatibility.h, I think forced inline for the handful of uses
of it may be best.
Add support to Mach-O corefiles and to live gdb remote serial protocol
connections for the corefile/remote stub to provide a list of load
addresses of binaries that should be found & loaded by lldb, and nothing
else. lldb will try to parse the binary out of memory, and if it can
find a UUID, try to find a binary & its debug information based on the
UUID, falling back to using the memory image if it must.
A bit of code unification from three parts of lldb that were loading
individual binaries already, so there is a shared method in
DynamicLoader to handle all of the variations they were doing.
Re-landing this with a uuid_is_null() implementation added to
Utility/UuidCompatibility.h for non-Darwin systems.
Differential Revision: https://reviews.llvm.org/D130813
rdar://94249937
rdar://94249384
This reverts commit d8879fba88.
Debian bot failure; I included <uuid/uuid.h> to get uuid_is_null() but
don't get it there. Will memcmp or whatever & recommit.
Add support to Mach-O corefiles and to live gdb remote serial protocol
connections for the corefile/remote stub to provide a list of load
addresses of binaries that should be found & loaded by lldb, and nothing
else. lldb will try to parse the binary out of memory, and if it can
find a UUID, try to find a binary & its debug information based on the
UUID, falling back to using the memory image if it must.
A bit of code unification from three parts of lldb that were loading
individual binaries already, so there is a shared method in
DynamicLoader to handle all of the variations they were doing.
Differential Revision: https://reviews.llvm.org/D130813
rdar://94249937
rdar://94249384
LLDB_WATCH_TYPE_IS_VALID would always return true when validating watchpoint type
by using bitwise-or instead of bitwise-and to apply validation flags.
Differential Revision: https://reviews.llvm.org/D130972
The use of `std::unique_ptr` with `TraceCursor` adds unnecessary complexity to adding `SBTraceCursor` bindings
Specifically, since `TraceCursor` is an abstract class there's no clean
way to provide "deep clone" semantics for `TraceCursorUP` short of
creating a pure virtual `clone()` method (afaict).
After discussing with @wallace, we decided there is no strong reason to
favor wrapping `TraceCursor` with `std::unique_ptr` over `std::shared_ptr`, thus this diff
replaces all usages of `std::unique_ptr<TraceCursor>` with `std::shared_ptr<TraceCursor>`.
This sets the stage for future diffs to introduce `SBTraceCursor`
bindings in a more clean fashion.
Test Plan:
Differential Revision: https://reviews.llvm.org/D130925
D130309 introduced changes to the FileSpec API which broke usages of
`GetCString()` in TraceIntelPTBundleSaver.cpp. This diff replaces usages
of `GetCString()` with `GetPath().c_str()` as suggested by D130309.
Test Plan:
Building with the trace plug-in now succeeds
Differential Revision: https://reviews.llvm.org/D130924
Support using the vCont packet to resume multiple processes
simultaneously when in non-stop mode. The new logic now assumes that:
- actions without a thread-id or with process id of "p-1" apply to all
debugged processes
- actions with a thread-id without process id apply to the current
process (m_continue_process)
As with the other continue packets, it is only possible to resume
processes that are currently stopped (or stop these that are running).
It is unsupported to resume or stop individual threads of a running
process.
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128989
If the test is too fast it can read the process list before the forked
child process actually manages to exec the process with the right
arguments.
Use our file-based synchronization primitives to ensure the child is
up-and-running before we fetch the process list.
Fixing potential int overflow and uninitialized variables.
These were found by Coverity static code inspection.
Differential Revision: https://reviews.llvm.org/D130795
- Fix a typo in the function that never returns a significant value
- Add unit tests for the getters/setters in SBBreakpointLocation
- Verified the newly added unit test succeeds after the fix:
llvm-lit -sv lldb/test/API/functionalities/breakpoint/breakpoint_locations/TestBreakpointLocations.py
Differential Revision: https://reviews.llvm.org/D130660
Checking if a path is absolute can be expensive and currently the result is not cached in the FileSpec object. This patch adds caching and also code to clear the cache if the file is modified.
Differential Revision: https://reviews.llvm.org/D130396
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
Xcode 14 no longer puts the Rosetta expanded shared cache in a directory
named "16.0". Instead, it includes the real version number (e.g. 13.0),
the build string and the architecture, similar to the device support
directory names for iOS, tvOS and watchOS.
Currently, when there are multiple directories, we might end up picking
the wrong one in GetSDKDirectoryForCurrentOSVersion. The problem is that
without the build string we have no way to differentiate between
multiple directories with the same version number. This patch fixes the
problem by using GetOSBuildString which, as the name implies, returns
the build string if known.
This also adds a test for Rosetta debugging on Apple Silicon. Depending
on whether the Rosetta expanded shared cache is present, the test
ensures that there is or isn't a diagnostic about reading out of memory.
rdar://97576121
Differential revision: https://reviews.llvm.org/D130540
I noticed that the test TestSetWatchpoint.py was failing every so often
on macOS. The failure was in the last assert, that after destroying the
SBTarget containing it, the SBWatchpoint was still saying it was valid.
IsValid in this case just meant the watchpoint weak pointer could be turned
into a shared pointer. The watchpoint shared pointers have two strong references
in general, one to the "Target::m_last_created_watchpoint", and one in the
Target::m_watchpoint_list. Target::Destroy reset the last created watchpoint
but neglected to call RemoveAll on the watchpoint list (it does the analogous
work for the internal & external breakpoint lists...) This patch does the
equivalent cleanup for the watchpoint list.
Teach libDebugInfo (llvm-dwarfdump) and lldb about DWARF tags and
attributes for pointer authentication. These values have been emitted by
Apple clang for several releases. Although upstream LLVM doesn't emit
these values yet, we hope to upstream that part sometime soon.
Differential revision: https://reviews.llvm.org/D130215
The DumpDataExtractor function had two branches for printing floating
point values. One branch (APFloat) was used if we had a Target object
around and could query it for the appropriate semantics. If we didn't
have a Target, we used host operations to read and format the value.
This patch changes second path to use APFloat as well. To make it work,
I pick reasonable defaults for different byte size. Notably, I did not
include x87 long double in that list (as it is ambibuous and
architecture-specific). This exposed a bug where we were printing
register values using the target-less branch, even though the registers
definitely belong to a target, and we had it available. Fixing this
prompted the update of several tests for register values due to slightly
different floating point outputs.
The most dubious aspect of this patch is the change in
TypeSystemClang::GetFloatTypeSemantics to recognize `10` as a valid size
for x87 long double. This was necessary because because sizeof(long
double) on x86_64 is 16 even though it only holds 10 bytes of useful
data. This generalizes the hackaround present in the target-free branch
of the dumping function.
Differential Revision: https://reviews.llvm.org/D129750
Without this patch, clang will not wrap in an ElaboratedType node types written
without a keyword and nested name qualifier, which goes against the intent that
we should produce an AST which retains enough details to recover how things are
written.
The lack of this sugar is incompatible with the intent of the type printer
default policy, which is to print types as written, but to fall back and print
them fully qualified when they are desugared.
An ElaboratedTypeLoc without keyword / NNS uses no storage by itself, but still
requires pointer alignment due to pre-existing bug in the TypeLoc buffer
handling.
---
Troubleshooting list to deal with any breakage seen with this patch:
1) The most likely effect one would see by this patch is a change in how
a type is printed. The type printer will, by design and default,
print types as written. There are customization options there, but
not that many, and they mainly apply to how to print a type that we
somehow failed to track how it was written. This patch fixes a
problem where we failed to distinguish between a type
that was written without any elaborated-type qualifiers,
such as a 'struct'/'class' tags and name spacifiers such as 'std::',
and one that has been stripped of any 'metadata' that identifies such,
the so called canonical types.
Example:
```
namespace foo {
struct A {};
A a;
};
```
If one were to print the type of `foo::a`, prior to this patch, this
would result in `foo::A`. This is how the type printer would have,
by default, printed the canonical type of A as well.
As soon as you add any name qualifiers to A, the type printer would
suddenly start accurately printing the type as written. This patch
will make it print it accurately even when written without
qualifiers, so we will just print `A` for the initial example, as
the user did not really write that `foo::` namespace qualifier.
2) This patch could expose a bug in some AST matcher. Matching types
is harder to get right when there is sugar involved. For example,
if you want to match a type against being a pointer to some type A,
then you have to account for getting a type that is sugar for a
pointer to A, or being a pointer to sugar to A, or both! Usually
you would get the second part wrong, and this would work for a
very simple test where you don't use any name qualifiers, but
you would discover is broken when you do. The usual fix is to
either use the matcher which strips sugar, which is annoying
to use as for example if you match an N level pointer, you have
to put N+1 such matchers in there, beginning to end and between
all those levels. But in a lot of cases, if the property you want
to match is present in the canonical type, it's easier and faster
to just match on that... This goes with what is said in 1), if
you want to match against the name of a type, and you want
the name string to be something stable, perhaps matching on
the name of the canonical type is the better choice.
3) This patch could expose a bug in how you get the source range of some
TypeLoc. For some reason, a lot of code is using getLocalSourceRange(),
which only looks at the given TypeLoc node. This patch introduces a new,
and more common TypeLoc node which contains no source locations on itself.
This is not an inovation here, and some other, more rare TypeLoc nodes could
also have this property, but if you use getLocalSourceRange on them, it's not
going to return any valid locations, because it doesn't have any. The right fix
here is to always use getSourceRange() or getBeginLoc/getEndLoc which will dive
into the inner TypeLoc to get the source range if it doesn't find it on the
top level one. You can use getLocalSourceRange if you are really into
micro-optimizations and you have some outside knowledge that the TypeLocs you are
dealing with will always include some source location.
4) Exposed a bug somewhere in the use of the normal clang type class API, where you
have some type, you want to see if that type is some particular kind, you try a
`dyn_cast` such as `dyn_cast<TypedefType>` and that fails because now you have an
ElaboratedType which has a TypeDefType inside of it, which is what you wanted to match.
Again, like 2), this would usually have been tested poorly with some simple tests with
no qualifications, and would have been broken had there been any other kind of type sugar,
be it an ElaboratedType or a TemplateSpecializationType or a SubstTemplateParmType.
The usual fix here is to use `getAs` instead of `dyn_cast`, which will look deeper
into the type. Or use `getAsAdjusted` when dealing with TypeLocs.
For some reason the API is inconsistent there and on TypeLocs getAs behaves like a dyn_cast.
5) It could be a bug in this patch perhaps.
Let me know if you need any help!
Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>
Differential Revision: https://reviews.llvm.org/D112374
Refactor the string conversion of the `lldb::InstructionControlFlowKind` enum out
of `Instruction::Dump` to enable reuse of this logic by the
JSON TraceDumper (to be implemented in separate diff).
Will coordinate the landing of this change with D130320 since there will be a minor merge conflict between
these changes.
Test Plan:
Run unittests
```
> ninja check-lldb
[4/5] Running lldb unit test suite
Testing Time: 10.13s
Passed: 1084
```
Verify '-k' flag's output
```
(lldb) thread trace dump instructions -k
thread #1: tid = 1375377
libstdc++.so.6`std::ostream::flush() + 43
7048: 0x00007ffff7b54dab return retq
7047: 0x00007ffff7b54daa other popq %rbx
7046: 0x00007ffff7b54da7 other movq %rbx, %rax
7045: 0x00007ffff7b54da5 cond jump je 0x11adb0 ; <+48>
7044: 0x00007ffff7b54da2 other cmpl $-0x1, %eax
libc.so.6`_IO_fflush + 249
7043: 0x00007ffff7161729 return retq
7042: 0x00007ffff7161728 other popq %rbp
7041: 0x00007ffff7161727 other popq %rbx
7040: 0x00007ffff7161725 other movl %edx, %eax
7039: 0x00007ffff7161721 other addq $0x8, %rsp
7038: 0x00007ffff7161709 cond jump je 0x87721 ; <+241>
7037: 0x00007ffff7161707 other decl (%rsi)
7036: 0x00007ffff71616fe cond jump je 0x87707 ; <+215>
7035: 0x00007ffff71616f7 other cmpl $0x0, 0x33de92(%rip) ; __libc_multiple_threads
7034: 0x00007ffff71616ef other movq $0x0, 0x8(%rsi)
7033: 0x00007ffff71616ed cond jump jne 0x87721 ; <+241>
7032: 0x00007ffff71616e9 other subl $0x1, 0x4(%rsi)
7031: 0x00007ffff71616e2 other movq 0x88(%rbx), %rsi
7030: 0x00007ffff71616e0 cond jump jne 0x87721 ; <+241>
7029: 0x00007ffff71616da other testl $0x8000, (%rbx) ; imm = 0x8000
```
Differential Revision: https://reviews.llvm.org/D130580
TestStackFromStdModule.py started failing due to f4fb72e6d4
(https://reviews.llvm.org/D128146), with a clang assertion failure:
assert(isa<InjectedClassNameType>(Decl->TypeForDecl))
This diff move the logic of `GetControlFlowKind()` from Disassembler.cpp to DisassemblerLLVMC.cpp.
Here's details:
- Actual logic of GetControlFlowKind() move to `DisassemblerLLVMC.cpp`, and we can check underlying architecture using `DisassemblerScope` there.
- With this change, passing 'triple' to `GetControlFlowKind()` is no more required.
Reviewed By: wallace
Differential Revision: https://reviews.llvm.org/D130320
This can cause a deadlock if other threads use the common pattern of
"lock the StackFrameList, get a frame, lock the StackFrame."
Differential Revision: https://reviews.llvm.org/D130524
Some codepaths lead to `namespace_map == nullptr` when we get to
`ClangASTSource::FindCompleteType`. This occurred while debugging
an lldb session that had `settings set target.import-std-module true`.
In that case, with `LLDBLog::Expressions` logging enabled, we would
dereference a `nullptr` and crash.
This commit moves the logging until after we check for `nullptr`.
**Testing**
* Fixed the specific crash I was seeing while debugging an `lldb`
session with `import-std-module` enabled.
Differential Revision: https://reviews.llvm.org/D130561
Slava Gurevich