JDevlieghere added a comment. In D78588#1996222 <https://reviews.llvm.org/D78588#1996222>, @labath wrote:
> The presence of `llvm_unreachable` here is questionable, but I am surprised > that this comes up in the context of reproducers. If the reproducers cause > this function to be called with a different ArchSpec, then it sounds like > there are bigger problems that need to be solved.. I only mentioned the reproducers to show that we can reach this code with an invalid ArchSpec and that this is not just a speculative fix. Since you're interested in what triggered this: TestRecognizeBreakpoint.py is a GDBRemoteTestBase, which starts with an empty Target (hence the invalid ArchSpec later on) and then connects to the test's GDB remote. It does so through `ConnectRemote` which for `ProcessGDBRemote` calls `ConnectToDebugserver`, which bypasses redirecting the connection to the GDB replay server. Fixing the reproducers issue is just a matter of moving that code down, so we connect to the replay server instead of the test's gdb server (which isn't there during active replay). > If we do want to do something about the crash, then I think we ought to just > remove the `llvm_unreachable`. I don't think it makes sense to bail out on > invalid ArchSpecs, but blow up on not-yet-supported architectures. If > anything, I would say it should be the opposite -- I don't think it makes > sense for anyone to call this function with an invalid/empty ArchSpec, but it > may be reasonable to enable some degraded behavior for architectures which > are not fully supported. Sounds reasonable to me. We can add an assert `arch.IsValid()` to enforce that precondition and have the default case return `0`. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78588/new/ https://reviews.llvm.org/D78588 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
