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
SupportFileSP should never be null, and instead should use a default constructed SupportFile to represent an invalid instance. This is because the class used to be a value type before it became polymorphic. We have various places in LLDB where we check this precondition, including in DisplaySourceLinesWithLineNumbers. The assertion was tripped when calling GetStartLineSourceInfo which starts by resetting the SupportFileSP and has a series of early returns which leave the shared pointer in that state. rdar://161607247
ArrayRef has a constructor that accepts std::nullopt. This constructor dates back to the days when we still had llvm::Optional. Since the use of std::nullopt outside the context of std::optional is kind of abuse and not intuitive to new comers, I would like to move away from the constructor and eventually remove it. This patch takes care of the lldb side of migration.
All uses have been replaced by GetAddressRange*s* or GetAddress. Also fix two internal uses of the range member.
When searching for the end of prologue, I'm only iterating through the address range (~basic block) which contains the function entry point. The reason for that is that even if some other range somehow contained the end-of-prologue marker, the fact that it's in a different range would imply it's reachable through some form of control flow, and that's usually not a good place to set an function entry breakpoint.
While looking at how to make Function::GetEndLineSourceInfo (which is only used in "command source") work with discontinuous functions, I realized there are other corner cases that this function doesn't handle. The code assumed that the last line entry in the function will also correspond to the last source line. This is probably true for unoptimized code, but I don't think we can rely on the optimizer to preserve this property. What's worse, the code didn't check that the last line entry belonged to the same file as the first one, so if this line entry was the result of inlining, we could end up using a line from a completely different file. To fix this, I change the algorithm to iterate over all line entries in the function (which belong to the same file) and find the max line number out of those. This way we can naturally handle the discontinuous case as well. This implementation is going to be slower than the previous one, but I don't think that matters, because: - this command is only used rarely, and interactively - we have plenty of other code which iterates through the line table I added some basic tests for the function operation. I don't claim the tests to be comprehensive, or that the function handles all edge cases, but test framework created here could be used for testing other fixes/edge cases as well.
This is the behavior expected by DWARF. It also requires some fixups to algorithms which were storing the addresses of some objects (Blocks and Variables) relative to the beginning of the function. There are plenty of things that still don't work in this setups, but this change is sufficient for the expression evaluator to correctly recognize the entry point of a function in this case.
The main change is to permit the disassembler class to process/store multiple (discontinuous) ranges of addresses. The result is not ambiguous because each instruction knows its size (in addition to its address), so we can check for discontinuity by looking at whether the next instruction begins where the previous ends. This patch doesn't handle the "disassemble" CLI command, which uses a more elaborate mechanism for disassembling and printing instructions.
Many calls to Function::GetAddressRange() were not interested in the range itself. Instead they wanted to find the address of the function (its entry point) or the base address for relocation of function-scoped entities (technically, the two don't need to be the same, but there's isn't good reason for them not to be). This PR creates a separate function for retrieving this, and changes the existing (non-controversial) uses to call that instead.
This reverts commit 2526d5b1689389da9b194b5ec2878cfb2f4aca93, reapplying ba14dac481564000339ba22ab867617590184f4c after fixing the conflict with #117532. The change is that Function::GetAddressRanges now recomputes the returned value instead of returning the member. This means it now returns a value instead of a reference type.