Author: Jordan Rupprecht Date: 2022-12-08T17:53:54-08:00 New Revision: f9d8090a90a1f1f9ddf6aebb82e7fc4c1dbcf030
URL: https://github.com/llvm/llvm-project/commit/f9d8090a90a1f1f9ddf6aebb82e7fc4c1dbcf030 DIFF: https://github.com/llvm/llvm-project/commit/f9d8090a90a1f1f9ddf6aebb82e7fc4c1dbcf030.diff LOG: Improve error handling for invalid breakpoint `-t` and `-x` options. Breakpoint option `-t` checks that `option_arg` is empty by checking `option_arg[0] == '\0'`. This is unnecessary: the next two checks for comparing against "current" and calling `getAsInteger` already gracefully handle an empty StringRef. If the `option_arg` string is empty, this crashes (or triggers an assertion failure with asserts enabled). Also, this sets the thread id to `LLDB_INVALID_THREAD_ID` if the thread id is invalid -- it should just not set the thread id. Likewise of `-x` which checks `option_arg[0] == '\n'` unnecessarily. I believe both of these bugs are unreachable via normal LLDB usage, and are only accessible via the fuzzer -- most likely some other CLI parsing is trimming whitespace and rejecting empty inputs. Still, it doesn't hurt to simplify this bit. Added: Modified: lldb/source/Commands/CommandObjectBreakpoint.cpp Removed: ################################################################################ diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index 5d2141d767cb..62e46c59e169 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -110,24 +110,24 @@ class lldb_private::BreakpointOptionGroup : public OptionGroup { } break; case 't': { lldb::tid_t thread_id = LLDB_INVALID_THREAD_ID; - if (option_arg[0] != '\0') { - if (option_arg == "current") { - if (!execution_context) { - error.SetErrorStringWithFormat("No context to determine current " - "thread"); + if (option_arg == "current") { + if (!execution_context) { + error.SetErrorStringWithFormat("No context to determine current " + "thread"); + } else { + ThreadSP ctx_thread_sp = execution_context->GetThreadSP(); + if (!ctx_thread_sp || !ctx_thread_sp->IsValid()) { + error.SetErrorStringWithFormat("No currently selected thread"); } else { - ThreadSP ctx_thread_sp = execution_context->GetThreadSP(); - if (!ctx_thread_sp || !ctx_thread_sp->IsValid()) { - error.SetErrorStringWithFormat("No currently selected thread"); - } else { - thread_id = ctx_thread_sp->GetID(); - } + thread_id = ctx_thread_sp->GetID(); } - } else if (option_arg.getAsInteger(0, thread_id)) - error.SetErrorStringWithFormat("invalid thread id string '%s'", - option_arg.str().c_str()); + } + } else if (option_arg.getAsInteger(0, thread_id)) { + error.SetErrorStringWithFormat("invalid thread id string '%s'", + option_arg.str().c_str()); } - m_bp_opts.SetThreadID(thread_id); + if (thread_id != LLDB_INVALID_THREAD_ID) + m_bp_opts.SetThreadID(thread_id); } break; case 'T': m_bp_opts.GetThreadSpec()->SetName(option_arg.str().c_str()); @@ -137,12 +137,12 @@ class lldb_private::BreakpointOptionGroup : public OptionGroup { break; case 'x': { uint32_t thread_index = UINT32_MAX; - if (option_arg[0] != '\n') { - if (option_arg.getAsInteger(0, thread_index)) - error.SetErrorStringWithFormat("invalid thread index string '%s'", - option_arg.str().c_str()); + if (option_arg.getAsInteger(0, thread_index)) { + error.SetErrorStringWithFormat("invalid thread index string '%s'", + option_arg.str().c_str()); + } else { + m_bp_opts.GetThreadSpec()->SetIndex(thread_index); } - m_bp_opts.GetThreadSpec()->SetIndex(thread_index); } break; default: llvm_unreachable("Unimplemented option"); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
