| Age | Commit message (Collapse) | Author |
|---|
|
This patch fixes and eliminates the possibility of SupportFileSP ever
being nullptr. The support file was originally treated like a value
type, but became a polymorphic type and therefore has to be stored and
passed around as a pointer.
To avoid having all the callers check the validity of the pointer, I
introduced the invariant that SupportFileSP is never null and always
default constructed. However, without enforcement at the type level,
that's fragile and indeed, we already identified two crashes where
someone accidentally broke that invariant.
This PR introduces a NonNullSharedPtr to prevent that. NonNullSharedPtr
is a smart pointer wrapper around std::shared_ptr that guarantees the
pointer is never null. If default-constructed, it creates a
default-constructed instance of the contained type. Note that I'm using
private inheritance because you shouldn't inherit from standard library
classes due to the lack of virtual destructor. So while the new
abstraction looks like a `std::shared_ptr`, it is in fact **not** a
shared pointer. Given that our destructor is trivial, we could use
public inheritance, but currently there's no need for it.
rdar://164989579
|
|
(#136795)
Fix a [test
failure](https://github.com/llvm/llvm-project/pull/136236#issuecomment-2819772879)
in #136236, apply a minor renaming of statistics, and remerge. See
details below.
# Changes in #136236
Currently, `DebuggerStats::ReportStatistics()` calls
`Module::GetSymtab(/*can_create=*/false)`, but then the latter calls
`SymbolFile::GetSymtab()`. This will load symbols if haven't yet. See
stacktrace below.
The problem is that `DebuggerStats::ReportStatistics` should be
read-only. This is especially important because it reports stats for
symtab parsing/indexing time, which could be affected by the reporting
itself if it's not read-only.
This patch fixes this problem by adding an optional parameter
`SymbolFile::GetSymtab(bool can_create = true)` and receiving the
`false` value passed down from `Module::GetSymtab(/*can_create=*/false)`
when the call is initiated from `DebuggerStats::ReportStatistics()`.
---
Notes about the following stacktrace:
1. This can be reproduced. Create a helloworld program on **macOS** with
dSYM, add `settings set target.preload-symbols false` to `~/.lldbinit`,
do `lldb a.out`, then `statistics dump`.
2. `ObjectFile::GetSymtab` has `llvm::call_once`. So the fact that it
called into `ObjectFileMachO::ParseSymtab` means that the symbol table
is actually being parsed.
```
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = step over
frame #0: 0x0000000124c4d5a0 LLDB`ObjectFileMachO::ParseSymtab(this=0x0000000111504e40, symtab=0x0000600000a05e00) at ObjectFileMachO.cpp:2259:44
* frame #1: 0x0000000124fc50a0 LLDB`lldb_private::ObjectFile::GetSymtab()::$_0::operator()(this=0x000000016d35c858) const at ObjectFile.cpp:761:9
frame #5: 0x0000000124fc4e68 LLDB`void std::__1::__call_once_proxy[abi:v160006]<std::__1::tuple<lldb_private::ObjectFile::GetSymtab()::$_0&&>>(__vp=0x000000016d35c7f0) at mutex:652:5
frame #6: 0x0000000198afb99c libc++.1.dylib`std::__1::__call_once(unsigned long volatile&, void*, void (*)(void*)) + 196
frame #7: 0x0000000124fc4dd0 LLDB`void std::__1::call_once[abi:v160006]<lldb_private::ObjectFile::GetSymtab()::$_0>(__flag=0x0000600003920080, __func=0x000000016d35c858) at mutex:670:9
frame #8: 0x0000000124fc3cb0 LLDB`void llvm::call_once<lldb_private::ObjectFile::GetSymtab()::$_0>(flag=0x0000600003920080, F=0x000000016d35c858) at Threading.h:88:5
frame #9: 0x0000000124fc2bc4 LLDB`lldb_private::ObjectFile::GetSymtab(this=0x0000000111504e40) at ObjectFile.cpp:755:5
frame #10: 0x0000000124fe0a28 LLDB`lldb_private::SymbolFileCommon::GetSymtab(this=0x0000000104865200) at SymbolFile.cpp:158:39
frame #11: 0x0000000124d8fedc LLDB`lldb_private::Module::GetSymtab(this=0x00000001113041a8, can_create=false) at Module.cpp:1027:21
frame #12: 0x0000000125125bdc LLDB`lldb_private::DebuggerStats::ReportStatistics(debugger=0x000000014284d400, target=0x0000000115808200, options=0x000000014195d6d1) at Statistics.cpp:329:30
frame #13: 0x0000000125672978 LLDB`CommandObjectStatsDump::DoExecute(this=0x000000014195d540, command=0x000000016d35d820, result=0x000000016d35e150) at CommandObjectStats.cpp:144:18
frame #14: 0x0000000124f29b40 LLDB`lldb_private::CommandObjectParsed::Execute(this=0x000000014195d540, args_string="", result=0x000000016d35e150) at CommandObject.cpp:832:9
frame #15: 0x0000000124efbd70 LLDB`lldb_private::CommandInterpreter::HandleCommand(this=0x0000000141b22f30, command_line="statistics dump", lazy_add_to_history=eLazyBoolCalculate, result=0x000000016d35e150, force_repeat_command=false) at CommandInterpreter.cpp:2134:14
frame #16: 0x0000000124f007f4 LLDB`lldb_private::CommandInterpreter::IOHandlerInputComplete(this=0x0000000141b22f30, io_handler=0x00000001419b2aa8, line="statistics dump") at CommandInterpreter.cpp:3251:3
frame #17: 0x0000000124d7b5ec LLDB`lldb_private::IOHandlerEditline::Run(this=0x00000001419b2aa8) at IOHandler.cpp:588:22
frame #18: 0x0000000124d1e8fc LLDB`lldb_private::Debugger::RunIOHandlers(this=0x000000014284d400) at Debugger.cpp:1225:16
frame #19: 0x0000000124f01f74 LLDB`lldb_private::CommandInterpreter::RunCommandInterpreter(this=0x0000000141b22f30, options=0x000000016d35e63c) at CommandInterpreter.cpp:3543:16
frame #20: 0x0000000122840294 LLDB`lldb::SBDebugger::RunCommandInterpreter(this=0x000000016d35ebd8, auto_handle_events=true, spawn_thread=false) at SBDebugger.cpp:1212:42
frame #21: 0x0000000102aa6d28 lldb`Driver::MainLoop(this=0x000000016d35ebb8) at Driver.cpp:621:18
frame #22: 0x0000000102aa75b0 lldb`main(argc=1, argv=0x000000016d35f548) at Driver.cpp:829:26
frame #23: 0x0000000198858274 dyld`start + 2840
```
# Changes in this PR top of the above
Fix a [test
failure](https://github.com/llvm/llvm-project/pull/136236#issuecomment-2819772879)
in `TestStats.py`. The original version of the added test checks that
all modules have symbol count zero when `target.preload-symbols ==
false`. The test failed on macOS. Due to various reasons, on macOS,
symbols can be loaded for dylibs even with that setting, but not for the
main module. For now, the fix of the test is to limit the assertion to
only the main module. The test now passes on macOS. In the future, when
we have a way to control a specific list of plug-ins to be loaded, there
may be a configuration that this test can use to assert that all modules
have symbol count zero.
Apply a minor renaming of statistics, per the
[suggestion](https://github.com/llvm/llvm-project/pull/136226#issuecomment-2825080275)
in #136226 after merge.
|
|
This reverts commit d5b40c71f6be972f677de5d9886f91866df007b5.
This change broke greendragon lldb test:
lldb-api :: commands/statistics/basic/TestStats.py
And is therefore being reverted.
|
|
Currently, `DebuggerStats::ReportStatistics()` calls
`Module::GetSymtab(/*can_create=*/false)`, but then the latter calls
`SymbolFile::GetSymtab()`. This will load symbols if haven't yet. See
stacktrace below.
The problem is that `DebuggerStats::ReportStatistics` should be
read-only. This is especially important because it reports stats for
symtab parsing/indexing time, which could be affected by the reporting
itself if it's not read-only.
This patch fixes this problem by adding an optional parameter
`SymbolFile::GetSymtab(bool can_create = true)` and receive the `false`
value passed down from `Module::GetSymtab(/*can_create=*/false)` when
the call was initiated from `DebuggerStats::ReportStatistics()`.
|
|
LineSeqeunce is basically a vector, except that users aren't supposed to
know that. This implements the same concept in a slightly simpler
fashion.
|
|
After (too) much deliberation, I came to the conclusion that this isn't
the right abstraction, as it doesn't behave completely like the standard
upper_bound method. What I really wanted was to get the range of line
entries for a given address range -- so I implement just that.
lower_bound is still useful as a primitive for building other kinds of
lookups.
|
|
The motivation is #123622 and the fact that is hard to fine the last
line entry in a given range. `FindLineEntryByAddress(range_end-1)` is
the best we have, but it's not ideal because it has a magic -1 and that
it relies on there existing a line entry at that address (generally, it
should be there, but if for some case it isn't, we might end up ignoring
the entries that are there (or -- like my incorrect fix in #123622 did
-- iterating through the entire line table).
What we really want is to get the last entry that exists in the given
range. Or, equivalently (and more STL-like) the first entry after that
range. This is what these functions do. I've used the STL names since
they do pretty much exactly what the standard functions do (the main
head-scratcher comes from the fact that our entries represent ranges
rather than single values).
The functions can also be used to simplify the maze of `if` statements
in `FindLineEntryByAddress`, but I'm keeping that as a separate patch.
For now, I'm just adding some unit testing for that function to gain
more confidence that the patch does not change the function behavior.
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
|
