From f9d8090a90a1f1f9ddf6aebb82e7fc4c1dbcf030 Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht Date: Thu, 8 Dec 2022 17:53:54 -0800 Subject: 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. --- lldb/source/Commands/CommandObjectBreakpoint.cpp | 40 ++++++++++++------------ 1 file changed, 20 insertions(+), 20 deletions(-) (limited to 'lldb/source/Commands/CommandObjectBreakpoint.cpp') diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index 5d2141d..62e46c5 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -110,24 +110,24 @@ public: } 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 @@ public: 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"); -- cgit v1.1