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
Jonas Devlieghere