Calling Disconnect while the read thread is running is racy because the
thread can also call Disconnect. This is a follow-up to b424b0bf, which
reorders other occurences of Disconnect/StopReadThread I can find, and also
adds an assertion to guard against new occurences being introduced.
Summary:
The code in DWARFCompileUnit::BuildAddressRangeTable tries hard to avoid
relying on DW_AT_low/high_pc for compile unit range information, and
this logic is a big cause of llvm/lldb divergence in the lowest layers
of dwarf parsing code.
The implicit assumption in that code is that this information (as opposed to
DW_AT_ranges) is unreliable. However, I have not been able to verify
that assumption. It is definitely not true for all present-day
compilers (gcc, clang, icc), and it was also not the case for the
historic compilers that I have been able to get a hold of (thanks Matt
Godbolt).
All compiler included in my research either produced correct
DW_AT_ranges or .debug_aranges entries, or they produced no DW_AT_hi/lo
pc at all. The detailed findings are:
- gcc >= 4.4: produces DW_AT_ranges and .debug_aranges
- 4.1 <= gcc < 4.4: no DW_AT_ranges, no DW_AT_high_pc, .debug_aranges
present. The upper version range here is uncertain as godbolt.org does
not have intermediate versions.
- gcc < 4.1: no versions on godbolt.org
- clang >= 3.5: produces DW_AT_ranges, and (optionally) .debug_aranges
- 3.4 <= clang < 3.5: no DW_AT_ranges, no DW_AT_high_pc, .debug_aranges
present.
- clang <= 3.3: no DW_AT_ranges, no DW_AT_high_pc, no .debug_aranges
- icc >= 16.0.1: produces DW_AT_ranges
- icc < 16.0.1: no functional versions on godbolt.org (some are present
but fail to compile)
Based on this analysis, I believe it is safe to start trusting
DW_AT_low/high_pc information in dwarf as well as remove the code for
manually reconstructing range information by traversing the DIE
structure, and just keep the line table fallback. The only compilers
where this will change behavior are pre-3.4 clangs, which are almost 7
years old now. However, the functionality should remain unchanged
because we will be able to reconstruct this information from the line
table, which seems to be needed for some line-tables-only scenarios
anyway (haven't researched this too much, but at least some compilers
seem to emit DW_AT_ranges even in these situations).
In addition, benchmarks showed that for these compilers computing the
ranges via line tables is noticably faster than doing so via the DIE
tree.
Other advantages include simplifying the code base, removing some
untested code (the only test changes are recent tests with overly
reduced synthetic dwarf), and increasing llvm convergence.
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D78489
Avoid a race between the Disconnect call in `Communication::ReadThread`
and the one in `Process::ShouldBroadcastEvent` by reordering the calls
to Disconnect and StopReadThread in `Process::ShouldBroadcastEvent`.
In D77295 Pavel suggested that that might explain the broken pipe I was
seeing. Indeed, changing the order resolved the issue.
Pavel Labath wrote in D73206:
The internal representation of DebugNames and Apple indexes is fixed by
the relevant (pseudo-)standards, so we can't really change it. The
question is how to efficiently (and cleanly) convert from the internal
representation to some common thing. The conversion from AppleIndex to
DIERef is trivial (which is not surprising as it was the first and the
overall design was optimized for that). With debug_names, the situation
gets more tricky. The internal representation of debug_names uses
CU-relative DIE offsets, but DIERef wants an absolute offset. That means
the index has to do more work to produce the common representation. And
it needs to do that for all results, even though a lot of the index
users are really interested only in a single entry. With the switch to
user_id_t, _all_ indexes would have to do some extra work to encode it,
only for their users to have to immediately decode it back. Having
a iterator/callback based api would allow us to minimize the impact of
that, as it would only need to happen for the entries that are really
used. And /I think/ we could make it interface returns DWARFDies
directly, and each index converts to that using the most direct approach
available.
Jan Kratochvil:
It also makes all the callers shorter as they no longer need to fetch
DWARFDIE from DIERef (and handling if not found by ReportInvalidDIERef)
but the callers are already served DWARFDIE which they need.
In some cases the DWARFDIE had to be fetched both by callee (DWARFIndex
implementation) and caller.
Differential Revision: https://reviews.llvm.org/D77970
Summary:
One small step in my long running quest to improve python exception handling in
LLDB. Replace GetInteger() which just returns an int with As<long long> and
friends, which return Expected types that can track python exceptions
Reviewers: labath, jasonmolenda, JDevlieghere, vadimcn
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D78462
Add the skipIfReproducer decorator to the remaining tests that fail to
replay because the GDB remote packets diverge during replay. This is
*not* expected and should be fixed, but figuring out exactly what caused
the divergence has proven pretty difficult to track down.
I've marked these tests as skipped for now so we can get clean results
and detect new regressions. I have no evidence to believe that these
failures have the same root cause, so I've not assigned them a PR.
Some tests are not expected to work with reproducers, for example tests
that completely circumvent the reproducers (i.e. using the side_effects
Python module) or that rely on changes to the file system.
Lit's to_string will just return the string when it's a `str` instance,
which in Python 2 can still contain UTF-8 characters.
Differential revision: https://reviews.llvm.org/D76955
Several SB API functions return strings using (char*, size_t) output
arguments. During capture, we serialize an empty string for the char*
because the memory can be uninitialized.
During active replay, we have custom replay redirects that ensure that
we don't override the buffer from which we're reading, but rather write
to a buffer on the heap with the given length. This is sufficient for
the active reproducer use case, where we only care about the side
effects of the API calls, not the values actually returned.
This approach does not not work for passive replay because here we
ignore all the incoming arguments, and re-execute the current function
with the arguments deserialized from the reproducer. This means that
these function will update the deserialized copy of the arguments,
rather than whatever was passed in by the SWIG wrapper.
To solve this problem, this patch extends the reproducer instrumentation
to handle this special case for passive replay. We nog ignore the
replayer in the registry and the incoming char pointer, and instead
reinvoke the current method on the deserialized class, and populate the
output argument.
Differential revision: https://reviews.llvm.org/D77759
This wasn't a great idea to begin with, as you can't really rely on the
implementation, but since it also doesn't work with MSVC I've just made
the ctors public.
Support passive replay as proposed in the RFC [1] on lldb-dev and
described in more detail on the lldb website [2].
This patch extends the LLDB_RECORD macros to re-invoke the current
function with arguments deserialized from the reproducer. This relies on
the function being called in the exact same order as during replay. It
uses the same mechanism to toggle the API boundary as during recording,
which guarantees that only boundary crossing calls are replayed.
Another major change is that before this patch we could ignore the
result of an API call, because we only cared about the observable
behavior. Now we need to be able to return the replayed result to the
SWIG bindings.
We reuse a lot of the recording infrastructure, which can be a little
confusing. We kept the existing naming to limit the amount of churn, but
might revisit that in a future patch.
[1] http://lists.llvm.org/pipermail/lldb-dev/2020-April/016100.html
[2] https://lldb.llvm.org/resources/reproducers.html
Differential revision: https://reviews.llvm.org/D77602
Add two modes to the reproducer replay script that make debugging a
little easier. Verbose mode prints stdout and stderr, regardless of
whether replay was successful. When --failure-only is passed, output is
limited to tests that failed to replay.
Summary:
...and replace it with m_last_file_spec instead.
When Source Cache is enabled, the value stored in m_last_file_sp is
already in the Source Cache, and caching it again in SourceManager
brings no extra benefit. All we need is to "remember" the last used
file, and FileSpec can serve the same purpose.
When Source Cache is disabled, the user explicitly requested no caching
of source files, and therefore, m_last_file_sp should NOT be used.
Bug: llvm.org/PR45310
Depends on D76805.
Reviewers: labath, jingham
Reviewed By: jingham
Subscribers: labath, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76806
Summary:
Lookup and subsequent insert was done using uninitialized
FileSpec object, which caused the cache to be a no-op.
Bug: llvm.org/PR45310
Depends on D76804.
Reviewers: labath, JDevlieghere
Reviewed By: labath
Subscribers: mgorny, jingham, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76805
Summary:
LLDB memory-maps large source files, and at the same time, caches
all source files in the Source Cache.
On Windows, memory-mapped source files are not writeable, causing
bad user experience in IDEs (such as errors when saving edited files).
IDEs should have the ability to disable the Source Cache at LLDB
startup, so that users can edit source files while debugging.
Bug: llvm.org/PR45310
Reviewers: labath, JDevlieghere, jingham
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D76804
Summary:
In D49685 sysroot behaviour was partially fixed. But files from local filesystem with same path still has priority over files from sysroot.
This patch fixes it by removing fallback to local filesystem from RemoteAwarePlatform::GetModuleSpec(). It is not actually required because higher level code do such fallback itself. See, for example, resolver in Platform::GetSharedModule().
Reviewers: labath, clayborg, EugeneBi
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77529
This is a regression since:
[lldb][NFC] Modernize lang/cpp/scope test
acb0b99c8e
rGacb0b99c8e4f
File "/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/lang/cpp/scope/TestCppScope.py", line 19, in test
self.assertEqual(global_var_names, expected_var_names)
AssertionError: Lists differ: ['C::a', 'A::a', 'B::a', '::a'... != ['A::a', 'B::a', 'C::a', '::a'...
First differing element 0:
C::a
A::a
- ['C::a', 'A::a', 'B::a', '::a']
+ ['A::a', 'B::a', 'C::a', '::a']
ManualDWARFIndex using NameToDIE does not sort alphabetically:
// This is only for uniqueness, not lexicographical ordering, so we can
// just compare pointers.
return uintptr_t(lhs.GetCString()) < uintptr_t(rhs.GetCString());
Since D77214 there is a testsuite regression for TestFixIts.py
on Fedora 31 x86_64.
File "/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/commands/expression/fixits/TestFixIts.py", line 148, in test_with_target
self.assertEquals(value.GetError().GetCString(), "error: No value")
AssertionError: 'error: error: Multiple internal symbols found for \'d\'\nid = {0x00000d2a}, ran [truncated]... != 'error: No value'
That is because Fedora glibc incl. libm.so contains also ELF debug
symbols and there exists a 'd' symbol:
(gdb) p d
$1 = {i = {0, 1076887552}, d = 16}
(gdb) p &d
$2 = (const number *) 0x7ffff78e8bc0 <d>
(gdb) info sym 0x7ffff78e8bc0
d in section .rodata of /lib64/libm.so.6
$ nm /lib64/libm.so.6 |grep ' d$'
00000000000bfbc0 r d
00000000000caa20 r d
00000000000caa20 r d
00000000000caa20 r d
glibc-build$ for i in `find -name "*.o"`;do nm 2>/dev/null $i|grep ' d$' && echo $i;done
0000000000000080 r d
./math/s_atan-fma4.o
0000000000000080 r d
./math/s_atan-avx.o
0000000000000080 r d
./math/s_atan.o
This patch threads an lldb::DescriptionLevel through the typesystem to
allow dumping the full Clang AST (level=verbose) of any lldb::Type in
addition to the human-readable source description (default
level=full). This type dumping interface is currently not exposed
through the SBAPI.
The application is to let lldb-test dump the clang AST of search
results. I need this to test lazy type completion of clang types in
subsequent patches.
Differential Revision: https://reviews.llvm.org/D78329
Converting a function pointer to an object pointer is illegal as nothing
requires it to be in the same address space. Add an overload for
function pointers so we don't convert do this illegal conversion, and
simply print out "function pointer".
The SIP debugserver was calling in attach_failed_due_to_sip
haven't worked for a while; remove them. To check this
properly we'd need debugsever to call out to codesign(1) to
inspect the entitlements, or the equivalant API,
and I'm not interested in adding that at this point. SIP
is has been the default on macOS for a couple of releases
and it's expected behavior now.
<rdar://problem/59198052>
The recent change in the API macros revealed that we were not printing
the pointer address for a bunch of methods, but rather the address of
the pointer. It's something I had already noticed while looking at some
reproducer traces, but hadn't made it to the top of my list yet. This
fixes the issue by providing a more specific overload.
Expand on the structure of the LLDB test suite. So far this information
has been mostly "tribal knowledge". By writing it down I hope to make it
easier to understand our test suite for anyone that's new to the
project.
Redefine the LLDB_RECORD macros in terms of a common uber-macro to
reduce code duplication across them.
Differential revision: https://reviews.llvm.org/D78141
[intel-pt] Improve the way the test determines whether to run
- Now I'm creating a default value for the new test parameter
- I fixed a small mistake in the skipping logic of the test
... I forgot to clear the cmake cache when testing my diff
Summary:
@labath raised a concern on the way I was skipping this test. I think that was
fair and I found a better way.
Now I'm skipping if the CMAKE flag LLDB_BUILD_INTEL_PT is false.
I added an enabled_plugins entry in the dotest configuration, which gets
set by lit or lldb-dotest. The only available plugin right now is
'intel-pt', but I imagine it will be useful in the future for other
kinds of plugins that get determined at configuration time. I didn't
want to add a new argument option --enable-intel-pt or something or the
sort, as it wouldn't be useful for other cases.
Reviewers: labath, clayborg
Subscribers: lldb-commits, labath
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D77452
Raphael Isemann