jasonmolenda updated this revision to Diff 539719. jasonmolenda added a comment.
Update to incorporate Jonas' suggestion - adding MachProcess::GetParentProcessID and MachProcess::ProcessIsBeingDebugged, as well as the DNB passthrough functions. I didn't pass back optionals for the cases were we can fail, using "invalid pid number" and "is not being debugged" for when the calls fail, which isn't the correctest but these are only ending up in error messages so i'm not too pressed about it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155037/new/ https://reviews.llvm.org/D155037 Files: lldb/include/lldb/Symbol/UnwindPlan.h lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp lldb/source/Symbol/UnwindPlan.cpp lldb/source/Target/RegisterContextUnwind.cpp lldb/tools/debugserver/source/DNB.cpp lldb/tools/debugserver/source/DNB.h lldb/tools/debugserver/source/MacOSX/MachProcess.h lldb/tools/debugserver/source/MacOSX/MachProcess.mm lldb/tools/debugserver/source/RNBRemote.cpp
Index: lldb/tools/debugserver/source/RNBRemote.cpp =================================================================== --- lldb/tools/debugserver/source/RNBRemote.cpp +++ lldb/tools/debugserver/source/RNBRemote.cpp @@ -3630,14 +3630,8 @@ // processes and step through to find the one we're looking for // (as process_does_not_exist() does). static bool process_is_already_being_debugged (nub_process_t pid) { - struct kinfo_proc kinfo; - int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid}; - size_t len = sizeof(struct kinfo_proc); - if (sysctl(mib, sizeof(mib) / sizeof(mib[0]), &kinfo, &len, NULL, 0) != 0) { - return false; // pid doesn't exist? well, it's not being debugged... - } - if (kinfo.kp_proc.p_flag & P_TRACED) - return true; // is being debugged already + if (DNBProcessIsBeingDebugged(pid) && DNBGetParentProcessID(pid) != getpid()) + return true; else return false; } Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm =================================================================== --- lldb/tools/debugserver/source/MacOSX/MachProcess.mm +++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm @@ -2821,16 +2821,21 @@ "attach to pid %d", getpid(), pid); - struct kinfo_proc kinfo; - int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid}; - size_t len = sizeof(struct kinfo_proc); - if (sysctl(mib, sizeof(mib) / sizeof(mib[0]), &kinfo, &len, NULL, 0) == 0 && len > 0) { - if (kinfo.kp_proc.p_flag & P_TRACED) { - ::snprintf(err_str, err_len, "%s - process %d is already being debugged", err.AsString(), pid); + if (ProcessIsBeingDebugged(pid)) { + nub_process_t ppid = GetParentProcessID(pid); + if (ppid == getpid()) { + snprintf(err_str, err_len, + "%s - Failed to attach to pid %d, AttachForDebug() " + "unable to ptrace(PT_ATTACHEXC)", + err.AsString(), m_pid); + } else { + snprintf(err_str, err_len, + "%s - process %d is already being debugged by pid %d", + err.AsString(), pid, ppid); DNBLogError( "[LaunchAttach] (%d) MachProcess::AttachForDebug pid %d is " - "already being debugged", - getpid(), pid); + "already being debugged by pid %d", + getpid(), pid, ppid); } } } @@ -2879,6 +2884,26 @@ return {}; } +nub_process_t MachProcess::GetParentProcessID(nub_process_t child_pid) { + struct proc_bsdshortinfo proc; + if (proc_pidinfo(child_pid, PROC_PIDT_SHORTBSDINFO, 0, &proc, + PROC_PIDT_SHORTBSDINFO_SIZE) == sizeof(proc)) { + return proc.pbsi_ppid; + } + return INVALID_NUB_PROCESS; +} + +bool MachProcess::ProcessIsBeingDebugged(nub_process_t pid) { + struct kinfo_proc kinfo; + int mib[] = {CTL_KERN, KERN_PROC, KERN_PROC_PID, pid}; + size_t len = sizeof(struct kinfo_proc); + if (sysctl(mib, sizeof(mib) / sizeof(mib[0]), &kinfo, &len, NULL, 0) == 0 && + (kinfo.kp_proc.p_flag & P_TRACED)) + return true; + else + return false; +} + #if defined(WITH_SPRINGBOARD) || defined(WITH_BKS) || defined(WITH_FBS) /// Get the app bundle from the given path. Returns the empty string if the /// path doesn't appear to be an app bundle. @@ -3401,7 +3426,13 @@ "%d (err = %i, errno = %i (%s))", m_pid, err, ptrace_err.Status(), ptrace_err.AsString()); - launch_err.SetError(NUB_GENERIC_ERROR, DNBError::Generic); + char err_msg[PATH_MAX]; + + snprintf(err_msg, sizeof(err_msg), + "Failed to attach to pid %d, LaunchForDebug() unable to " + "ptrace(PT_ATTACHEXC)", + m_pid); + launch_err.SetErrorString(err_msg); } } else { launch_err.Clear(); @@ -3777,6 +3808,10 @@ m_flags |= eMachProcessFlagsAttached; DNBLogThreadedIf(LOG_PROCESS, "successfully attached to pid %d", m_pid); } else { + launch_err.SetErrorString( + "Failed to attach to pid %d, SBLaunchForDebug() unable to " + "ptrace(PT_ATTACHEXC)", + m_pid); SetState(eStateExited); DNBLogThreadedIf(LOG_PROCESS, "error: failed to attach to pid %d", m_pid); } @@ -3996,6 +4031,10 @@ m_flags |= eMachProcessFlagsAttached; DNBLog("[LaunchAttach] successfully attached to pid %d", m_pid); } else { + launch_err.SetErrorString( + "Failed to attach to pid %d, BoardServiceLaunchForDebug() unable to " + "ptrace(PT_ATTACHEXC)", + m_pid); SetState(eStateExited); DNBLog("[LaunchAttach] END (%d) error: failed to attach to pid %d", getpid(), m_pid); Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h =================================================================== --- lldb/tools/debugserver/source/MacOSX/MachProcess.h +++ lldb/tools/debugserver/source/MacOSX/MachProcess.h @@ -126,6 +126,11 @@ static bool GetOSVersionNumbers(uint64_t *major, uint64_t *minor, uint64_t *patch); static std::string GetMacCatalystVersionString(); + + static nub_process_t GetParentProcessID(nub_process_t child_pid); + + static bool ProcessIsBeingDebugged(nub_process_t pid); + #ifdef WITH_BKS static void BKSCleanupAfterAttach(const void *attach_token, DNBError &err_str); Index: lldb/tools/debugserver/source/DNB.h =================================================================== --- lldb/tools/debugserver/source/DNB.h +++ lldb/tools/debugserver/source/DNB.h @@ -247,4 +247,9 @@ bool DNBDebugserverIsTranslated(); bool DNBGetAddressingBits(uint32_t &addressing_bits); + +nub_process_t DNBGetParentProcessID(nub_process_t child_pid); + +bool DNBProcessIsBeingDebugged(nub_process_t pid); + #endif Index: lldb/tools/debugserver/source/DNB.cpp =================================================================== --- lldb/tools/debugserver/source/DNB.cpp +++ lldb/tools/debugserver/source/DNB.cpp @@ -1848,3 +1848,11 @@ return addressing_bits > 0; } + +nub_process_t DNBGetParentProcessID(nub_process_t child_pid) { + return MachProcess::GetParentProcessID(child_pid); +} + +bool DNBProcessIsBeingDebugged(nub_process_t pid) { + return MachProcess::ProcessIsBeingDebugged(pid); +} Index: lldb/source/Target/RegisterContextUnwind.cpp =================================================================== --- lldb/source/Target/RegisterContextUnwind.cpp +++ lldb/source/Target/RegisterContextUnwind.cpp @@ -300,9 +300,8 @@ UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" PRIx64 " afa is 0x%" PRIx64 " using %s UnwindPlan", (uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()), - (uint64_t)m_cfa, - (uint64_t)m_afa, - m_full_unwind_plan_sp->GetSourceName().GetCString()); + (uint64_t)m_cfa, (uint64_t)m_afa, + m_full_unwind_plan_sp->GetSourceName()); } // Initialize a RegisterContextUnwind for the non-zeroth frame -- rely on the @@ -645,7 +644,7 @@ active_row->Dump(active_row_strm, m_fast_unwind_plan_sp.get(), &m_thread, m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr())); UnwindLogMsg("Using fast unwind plan '%s'", - m_fast_unwind_plan_sp->GetSourceName().AsCString()); + m_fast_unwind_plan_sp->GetSourceName()); UnwindLogMsg("active row: %s", active_row_strm.GetData()); } } else { @@ -661,7 +660,7 @@ &m_thread, m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr())); UnwindLogMsg("Using full unwind plan '%s'", - m_full_unwind_plan_sp->GetSourceName().AsCString()); + m_full_unwind_plan_sp->GetSourceName()); UnwindLogMsg("active row: %s", active_row_strm.GetData()); } } @@ -957,7 +956,7 @@ if (unwind_plan_sp && unwind_plan_sp->PlanValidAtAddress(m_current_pc)) { UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because the " "DynamicLoader suggested we prefer it", - unwind_plan_sp->GetSourceName().GetCString()); + unwind_plan_sp->GetSourceName()); return unwind_plan_sp; } } @@ -995,7 +994,7 @@ UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this " "is the non-call site unwind plan and this is a " "zeroth frame", - unwind_plan_sp->GetSourceName().GetCString()); + unwind_plan_sp->GetSourceName()); return unwind_plan_sp; } @@ -1007,9 +1006,10 @@ func_unwinders_sp->GetUnwindPlanArchitectureDefaultAtFunctionEntry( m_thread); if (unwind_plan_sp) { - UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we are at " - "the first instruction of a function", - unwind_plan_sp->GetSourceName().GetCString()); + UnwindLogMsgVerbose( + "frame uses %s for full UnwindPlan because we are at " + "the first instruction of a function", + unwind_plan_sp->GetSourceName()); return unwind_plan_sp; } } @@ -1024,7 +1024,7 @@ if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) { UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because this " "is the call-site unwind plan", - unwind_plan_sp->GetSourceName().GetCString()); + unwind_plan_sp->GetSourceName()); return unwind_plan_sp; } @@ -1061,9 +1061,10 @@ } if (IsUnwindPlanValidForCurrentPC(unwind_plan_sp)) { - UnwindLogMsgVerbose("frame uses %s for full UnwindPlan because we " - "failed to find a call-site unwind plan that would work", - unwind_plan_sp->GetSourceName().GetCString()); + UnwindLogMsgVerbose( + "frame uses %s for full UnwindPlan because we " + "failed to find a call-site unwind plan that would work", + unwind_plan_sp->GetSourceName()); return unwind_plan_sp; } @@ -1073,7 +1074,7 @@ UnwindLogMsgVerbose( "frame uses %s for full UnwindPlan because we are falling back " "to the arch default plan", - arch_default_unwind_plan_sp->GetSourceName().GetCString()); + arch_default_unwind_plan_sp->GetSourceName()); else UnwindLogMsg( "Unable to find any UnwindPlan for full unwind of this frame."); @@ -1337,7 +1338,7 @@ &m_thread, m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr())); UnwindLogMsg("Using full unwind plan '%s'", - m_full_unwind_plan_sp->GetSourceName().AsCString()); + m_full_unwind_plan_sp->GetSourceName()); UnwindLogMsg("active row: %s", active_row_strm.GetData()); } RegisterNumber return_address_reg; @@ -1393,7 +1394,7 @@ UnwindLogMsg( "supplying caller's saved %s (%d)'s location using %s UnwindPlan", regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB), - m_full_unwind_plan_sp->GetSourceName().GetCString()); + m_full_unwind_plan_sp->GetSourceName()); } // This is frame 0 and we're retrieving the PC and it's saved in a Return @@ -1448,7 +1449,7 @@ !m_all_registers_available) { UnwindLogMsg("%s UnwindPlan tried to restore the pc from the link " "register but this is a non-zero frame", - m_full_unwind_plan_sp->GetSourceName().GetCString()); + m_full_unwind_plan_sp->GetSourceName()); // Throw away the full unwindplan; install the arch default unwindplan if (ForceSwitchToFallbackUnwindPlan()) { @@ -1527,7 +1528,7 @@ std::string unwindplan_name; if (m_full_unwind_plan_sp) { unwindplan_name += "via '"; - unwindplan_name += m_full_unwind_plan_sp->GetSourceName().AsCString(); + unwindplan_name += m_full_unwind_plan_sp->GetSourceName(); unwindplan_name += "'"; } UnwindLogMsg("no save location for %s (%d) %s", regnum.GetName(), @@ -1845,8 +1846,8 @@ UnwindLogMsg("trying to unwind from this function with the UnwindPlan '%s' " "because UnwindPlan '%s' failed.", - m_fallback_unwind_plan_sp->GetSourceName().GetCString(), - original_full_unwind_plan_sp->GetSourceName().GetCString()); + m_fallback_unwind_plan_sp->GetSourceName(), + original_full_unwind_plan_sp->GetSourceName()); // We've copied the fallback unwind plan into the full - now clear the // fallback. @@ -1898,7 +1899,7 @@ PropagateTrapHandlerFlagFromUnwindPlan(m_full_unwind_plan_sp); UnwindLogMsg("switched unconditionally to the fallback unwindplan %s", - m_full_unwind_plan_sp->GetSourceName().GetCString()); + m_full_unwind_plan_sp->GetSourceName()); return true; } return false; Index: lldb/source/Symbol/UnwindPlan.cpp =================================================================== --- lldb/source/Symbol/UnwindPlan.cpp +++ lldb/source/Symbol/UnwindPlan.cpp @@ -443,11 +443,11 @@ LLDB_LOGF(log, "UnwindPlan is invalid -- no unwind rows for UnwindPlan " "'%s' at address %s", - m_source_name.GetCString(), s.GetData()); + m_source_name, s.GetData()); } else { LLDB_LOGF(log, "UnwindPlan is invalid -- no unwind rows for UnwindPlan '%s'", - m_source_name.GetCString()); + m_source_name); } } return false; @@ -466,12 +466,12 @@ LLDB_LOGF(log, "UnwindPlan is invalid -- no CFA register defined in row 0 " "for UnwindPlan '%s' at address %s", - m_source_name.GetCString(), s.GetData()); + m_source_name, s.GetData()); } else { LLDB_LOGF(log, "UnwindPlan is invalid -- no CFA register defined in row 0 " "for UnwindPlan '%s'", - m_source_name.GetCString()); + m_source_name); } } return false; @@ -491,9 +491,8 @@ } void UnwindPlan::Dump(Stream &s, Thread *thread, lldb::addr_t base_addr) const { - if (!m_source_name.IsEmpty()) { - s.Printf("This UnwindPlan originally sourced from %s\n", - m_source_name.GetCString()); + if (m_source_name) { + s.Printf("This UnwindPlan originally sourced from %s\n", m_source_name); } if (m_lsda_address.IsValid() && m_personality_func_addr.IsValid()) { TargetSP target_sp(thread->CalculateTarget()); @@ -561,11 +560,9 @@ } } -void UnwindPlan::SetSourceName(const char *source) { - m_source_name = ConstString(source); -} +void UnwindPlan::SetSourceName(const char *source) { m_source_name = source; } -ConstString UnwindPlan::GetSourceName() const { return m_source_name; } +const char *UnwindPlan::GetSourceName() const { return m_source_name; } const RegisterInfo *UnwindPlan::GetRegisterInfo(Thread *thread, uint32_t unwind_reg) const { Index: lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp =================================================================== --- lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp +++ lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp @@ -1554,9 +1554,10 @@ unwind_plan.SetPlanValidAddressRange(func_range); if (unwind_plan_updated) { - std::string unwind_plan_source(unwind_plan.GetSourceName().AsCString()); + std::string unwind_plan_source(unwind_plan.GetSourceName()); unwind_plan_source += " plus augmentation from assembly parsing"; - unwind_plan.SetSourceName(unwind_plan_source.c_str()); + ConstString name_in_stringpool(unwind_plan_source); + unwind_plan.SetSourceName(name_in_stringpool.AsCString()); unwind_plan.SetSourcedFromCompiler(eLazyBoolNo); unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolYes); } Index: lldb/source/Commands/CommandObjectTarget.cpp =================================================================== --- lldb/source/Commands/CommandObjectTarget.cpp +++ lldb/source/Commands/CommandObjectTarget.cpp @@ -3458,21 +3458,20 @@ if (non_callsite_unwind_plan) { result.GetOutputStream().Printf( "Asynchronous (not restricted to call-sites) UnwindPlan is '%s'\n", - non_callsite_unwind_plan->GetSourceName().AsCString()); + non_callsite_unwind_plan->GetSourceName()); } UnwindPlanSP callsite_unwind_plan = func_unwinders_sp->GetUnwindPlanAtCallSite(*target, *thread); if (callsite_unwind_plan) { result.GetOutputStream().Printf( "Synchronous (restricted to call-sites) UnwindPlan is '%s'\n", - callsite_unwind_plan->GetSourceName().AsCString()); + callsite_unwind_plan->GetSourceName()); } UnwindPlanSP fast_unwind_plan = func_unwinders_sp->GetUnwindPlanFastUnwind(*target, *thread); if (fast_unwind_plan) { - result.GetOutputStream().Printf( - "Fast UnwindPlan is '%s'\n", - fast_unwind_plan->GetSourceName().AsCString()); + result.GetOutputStream().Printf("Fast UnwindPlan is '%s'\n", + fast_unwind_plan->GetSourceName()); } result.GetOutputStream().Printf("\n"); Index: lldb/include/lldb/Symbol/UnwindPlan.h =================================================================== --- lldb/include/lldb/Symbol/UnwindPlan.h +++ lldb/include/lldb/Symbol/UnwindPlan.h @@ -465,7 +465,7 @@ const UnwindPlan::RowSP GetLastRow() const; - lldb_private::ConstString GetSourceName() const; + const char *GetSourceName() const; void SetSourceName(const char *); @@ -509,7 +509,7 @@ m_row_list.clear(); m_plan_valid_address_range.Clear(); m_register_kind = lldb::eRegisterKindDWARF; - m_source_name.Clear(); + m_source_name = nullptr; m_plan_is_sourced_from_compiler = eLazyBoolCalculate; m_plan_is_valid_at_all_instruction_locations = eLazyBoolCalculate; m_plan_is_for_signal_trap = eLazyBoolCalculate; @@ -539,8 +539,8 @@ uint32_t m_return_addr_register; // The register that has the return address // for the caller frame // e.g. the lr on arm - lldb_private::ConstString - m_source_name; // for logging, where this UnwindPlan originated from + const char + *m_source_name; // for logging, where this UnwindPlan originated from lldb_private::LazyBool m_plan_is_sourced_from_compiler; lldb_private::LazyBool m_plan_is_valid_at_all_instruction_locations; lldb_private::LazyBool m_plan_is_for_signal_trap;
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits