Fix display of function breakpoints

src/common/scripting/dap/BreakpointManager.cpp

@@ -31,6 +31,7 @@ int BreakpointManager::AddInvalidBreakpoint(
 	breakpoint.message = reason;
 	breakpoint.source = source;
 	breakpoint.verified = false;
+	breakpoint.reason = "failed";
 	if (line)
 	{
 		breakpoint.line = line;
@@ -49,7 +50,8 @@ bool BreakpointManager::AddBreakpointInfo(
 	void *p_instrRef,
 	int offset,
 	BreakpointInfo::Type type,
-	std::vector<dap::Breakpoint> &r_bpoint)
+	std::vector<dap::Breakpoint> &r_bpoint,
+	const std::string &funcText)
 {
 	// Only call this with positional breakpoints (line, script function, instruction)
 	assert(p_instrRef != nullptr);
@@ -81,6 +83,7 @@ bool BreakpointManager::AddBreakpointInfo(
 	auto &binfo = m_breakpoints[instrRef].emplace_back();
 	binfo.type = type;
 	binfo.ref = sourceRef;
+	binfo.funcBreakpointText = funcText;
 	binfo.bpoint.id = breakpointId;
 	binfo.bpoint.line = line;
 	binfo.bpoint.instructionReference = AddrToString(function, p_instrRef);
@@ -284,8 +287,10 @@ dap::ResponseOrError<dap::SetFunctionBreakpointsResponse> BreakpointManager::Set
 			BreakpointInfo bpoint_info;
 			bpoint_info.type = BreakpointInfo::Type::Function;
 			bpoint_info.ref = -1;
+			bpoint_info.funcBreakpointText = fullFuncName;
 			bpoint_info.bpoint.id = GetBreakpointID();
 			bpoint_info.bpoint.line = 1;
+			bpoint_info.bpoint.verified = true;
 			m_nativeFunctionBreakpoints[func->QualifiedName] = bpoint_info;
 			response.breakpoints.push_back(bpoint_info.bpoint);
 			continue;
@@ -308,7 +313,7 @@ dap::ResponseOrError<dap::SetFunctionBreakpointsResponse> BreakpointManager::Set
 		auto lineNum = scriptFunction->LineInfo[0].LineNumber;
 		auto instructionNum = scriptFunction->LineInfo[0].InstructionIndex;
 		void *instrRef = scriptFunction->Code + instructionNum;
-		AddBreakpointInfo(binary, scriptFunction, lineNum, instrRef, 0, BreakpointInfo::Type::Function, response.breakpoints);
+		AddBreakpointInfo(binary, scriptFunction, lineNum, instrRef, 0, BreakpointInfo::Type::Function, response.breakpoints, fullFuncName);
 	}
 	return response;
 }
@@ -390,27 +395,56 @@ void BreakpointManager::ClearBreakpointsForScript(int ref, BreakpointInfo::Type
 
 bool BreakpointManager::GetExecutionIsAtValidBreakpoint(VMFrameStack *stack, VMReturn *ret, int numret, const VMOP *pc)
 {
-#define CLEAR_AND_RETURN   m_last_seen = nullptr; return false
-	if (!IsFunctionNative(stack->TopFrame()->Func) && m_breakpoints.find((void *)pc) != m_breakpoints.end())
-	{
-		return true;
-	}
-	else if (IsFunctionNative(stack->TopFrame()->Func) && m_nativeFunctionBreakpoints.find(stack->TopFrame()->Func->QualifiedName) != m_nativeFunctionBreakpoints.end())
-	{
-		return true;
-	}
-	return CLEAR_AND_RETURN;
-#undef CLEAR_AND_RETURN
+	return m_breakpoints.find((void *)pc) != m_breakpoints.end() || (!m_nativeFunctionBreakpoints.empty() && IsAtNativeBreakpoint(stack));
 }
 
-
-bool BreakpointManager::HasSeenBreakpoint(BreakpointManager::BreakpointInfo *info)
+inline bool BreakpointManager::IsAtNativeBreakpoint(VMFrameStack *stack)
 {
-	if (!m_last_seen || m_last_seen != info)
+	return PCIsAtNativeCall(stack->TopFrame())
+		&& m_nativeFunctionBreakpoints.find(GetCalledFunction(stack->TopFrame())->QualifiedName) != m_nativeFunctionBreakpoints.end();
+}
+
+void BreakpointManager::SetBPStoppedEventInfo(VMFrameStack *stack, dap::StoppedEvent &event)
+{
+	std::vector<dap::integer> breakpoints;
+	if (!stack->HasFrames())
 	{
-		return false;
+		return;
 	}
-	return true;
+	auto frame = stack->TopFrame();
+	std::string description = "Paused on breakpoint";
+	if (m_breakpoints.find((void *)frame->PC) != m_breakpoints.end())
+	{
+		for (auto &bpoint : m_breakpoints[(void *)frame->PC])
+		{
+			breakpoints.push_back(bpoint.bpoint.id.value(-1));
+		}
+	}
+	if (IsAtNativeBreakpoint(stack))
+	{
+		auto func = GetCalledFunction(frame);
+		auto &bpoint_info = m_nativeFunctionBreakpoints[func->QualifiedName];
+		description = std::string("Paused on breakpoint at '") + bpoint_info.funcBreakpointText + "'";
+		if (!CaseInsensitiveEquals(bpoint_info.funcBreakpointText, func->QualifiedName))
+		{
+			event.text = description + " (" + func->QualifiedName + ")";
+		}
+		else
+		{
+			event.text = description;
+		}
+		breakpoints.push_back(m_nativeFunctionBreakpoints[func->QualifiedName].bpoint.id.value(-1));
+	}
+	if (breakpoints.empty())
+	{
+		LogInternalError("No breakpoints found for stopped event");
+	}
+	if (!description.empty())
+	{
+		event.description = description;
+	}
+	event.reason = "breakpoint";
+	event.hitBreakpointIds = breakpoints;
 }
 
 dap::ResponseOrError<dap::SetInstructionBreakpointsResponse> BreakpointManager::SetInstructionBreakpoints(const dap::SetInstructionBreakpointsRequest &request)

src/common/scripting/dap/BreakpointManager.h

@@ -25,6 +25,8 @@ class BreakpointManager
 		};
 		Type type;
 		int ref;
+		std::string funcBreakpointText;
+		const char *nativeFuncName;
 		dap::Breakpoint bpoint;
 	};
 
@@ -48,24 +50,23 @@ class BreakpointManager
 		void *p_instrRef,
 		int offset,
 		BreakpointInfo::Type type,
-		std::vector<dap::Breakpoint> &r_bpoint);
+		std::vector<dap::Breakpoint> &r_bpoint,
+		const std::string &funcText = {});
 	void GetBpointsForResponse(BreakpointInfo::Type type, std::vector<dap::Breakpoint> &responseBpoints);
 	dap::ResponseOrError<dap::SetBreakpointsResponse> SetBreakpoints(const dap::Source &src, const dap::SetBreakpointsRequest &request);
-	bool AddPositionalBpoint(VMFunction *p_func, void *address, int64_t line);
 	dap::ResponseOrError<dap::SetFunctionBreakpointsResponse> SetFunctionBreakpoints(const dap::SetFunctionBreakpointsRequest &request);
 	dap::ResponseOrError<dap::SetInstructionBreakpointsResponse> SetInstructionBreakpoints(const dap::SetInstructionBreakpointsRequest &request);
 	void ClearBreakpoints(bool emitChanged = false);
 	void ClearBreakpointsForScript(int ref, BreakpointInfo::Type type, bool emitChanged = false);
 	bool GetExecutionIsAtValidBreakpoint(VMFrameStack *stack, VMReturn *ret, int numret, const VMOP *pc);
+	inline bool IsAtNativeBreakpoint(VMFrameStack *stack);
+	void SetBPStoppedEventInfo(VMFrameStack *stack, dap::StoppedEvent &event);
 	private:
-	void ClearFunctionBreakpoints();
-	bool HasSeenBreakpoint(BreakpointInfo *info);
 
 	PexCache *m_pexCache;
 	std::map<void *, std::vector<BreakpointInfo>> m_breakpoints;
 	// set of case-insensitive strings
-	caseless_path_map<BreakpointInfo> m_nativeFunctionBreakpoints;
-	BreakpointInfo *m_last_seen = nullptr;
+	std::map<std::string_view, BreakpointInfo, ci_less> m_nativeFunctionBreakpoints;
 	IdProvider m_idProvider;
 	int64_t m_CurrentID = 1;
 	size_t times_seen = 0;

src/common/scripting/dap/DebugExecutionManager.cpp

@@ -199,6 +199,10 @@ void DebugExecutionManager::HandleInstruction(VMFrameStack *stack, VMReturn *ret
 			dap::StoppedEvent event;
 			event.allThreadsStopped = true;
 			event.reason = pauseReasonStrings[(int)pauseReason];
+			if (pauseReason == pauseReason::breakpoint)
+			{
+				m_breakpointManager->SetBPStoppedEventInfo(stack, event);
+			}
 			event.threadId = 1;
 			m_session->send(event);
 		}

src/common/scripting/dap/GameInterfaces.h

@@ -1019,8 +1019,7 @@ static bool PCIsAtNativeCall(VMFrame *frame)
 		{
 			ptr = frame->GetRegA()[frame->PC->a];
 		}
-		VMFunction *call = (VMFunction *)ptr;
-		if (IsFunctionNative(call))
+		if (IsFunctionNative((VMFunction *)ptr))
 		{
 			return true;
 		}

src/common/scripting/dap/Nodes/StackStateNode.cpp

@@ -47,7 +47,7 @@ bool StackStateNode::GetChildNames(std::vector<std::string> &names)
 	size_t frameNum = 0;
 	for (size_t i = 0; i < frames.size(); i++)
 	{
-		if (i != 0 && PCIsAtNativeCall(frames.at(i)))
+		if (PCIsAtNativeCall(frames.at(i)))
 		{
 			names.push_back(std::to_string(frameNum));
 			m_children[frameNum] = std::make_shared<StackFrameStateNode>(GetCalledFunction(frames.at(i)), frames.at(i));

src/common/scripting/dap/Utilities.h

@@ -18,11 +18,18 @@ struct ci_less
 	{
 		bool operator()(const unsigned char &c1, const unsigned char &c2) const { return tolower(c1) < tolower(c2); }
 	};
-	bool operator()(const std::string &s1, const std::string &s2) const
+	template <
+		typename T,
+		typename U,
+		typename = std::enable_if_t<
+			(std::is_same_v<T, std::string> || std::is_same_v<T, std::string_view>) && (std::is_same_v<U, std::string> || std::is_same_v<U, std::string_view>)>>
+	bool operator()(T const &s1, U const &s2) const
 	{
 		return std::lexicographical_compare(
-			s1.begin(), s1.end(), // source range
-			s2.begin(), s2.end(), // dest range
+			s1.begin(),
+			s1.end(), // source range
+			s2.begin(),
+			s2.end(), // dest range
 			nocase_compare()); // comparison
 	}
 };