Debug Kit: Restore support for symbol lookup by remote memory access.

It's been broken since clone_area was changed to block cloning of
areas without B_CLONEABLE_AREA set on them. We here introduce a
B_DEBUG_MESSAGE_CLONE_AREA debug nub message, which clones the areas
of the debugged team for the debugger.

Also fix some bugs in SymbolLookup::_FindLoadedImageAt methods:
they didn't work properly when *next was NULL, so they would
always fail when iterating over the full list.

Note that this technically breaks libdebug.so and the debugger
protocol ABI. However, nothing out-of-tree that I know of uses
the private libdebug.so, and while GDB does use the debugger
protocol, it doesn't actually use any of the messages past
the first block, so it should still work after this.

Fixes #15251.

Change-Id: I71ccbee4afd17dae30d5dacbc7590d1e2175a90e
Reviewed-on: https://review.haiku-os.org/c/haiku/+/8821
Tested-by: Commit checker robot <no-reply+buildbot@haiku-os.org>
Reviewed-by: waddlesplash <waddlesplash@gmail.com>

headers/os/kernel/debugger.h

@@ -165,6 +165,7 @@ typedef enum {
 	B_DEBUG_MESSAGE_GET_SIGNAL_MASKS,	//  the debugger is interested in
 	B_DEBUG_MESSAGE_SET_SIGNAL_HANDLER,	// set/get the team's signal handler for
 	B_DEBUG_MESSAGE_GET_SIGNAL_HANDLER,	//  a signal
+	B_DEBUG_MESSAGE_CLONE_AREA,			// clone a team area into the debugger team
 
 	B_DEBUG_MESSAGE_PREPARE_HANDOVER,	// prepares the debugged team for being
 										// handed over to another debugger;
@@ -174,7 +175,7 @@ typedef enum {
 	B_DEBUG_START_PROFILER,				// start/stop sampling
 	B_DEBUG_STOP_PROFILER,				//
 
-	B_DEBUG_WRITE_CORE_FILE				// write a core file
+	B_DEBUG_WRITE_CORE_FILE,			// write a core file
 } debug_nub_message;
 
 // messages sent to the debugger
@@ -254,6 +255,18 @@ typedef struct {
 	int32		size;			// the number of bytes actually written
 } debug_nub_write_memory_reply;
 
+// B_DEBUG_MESSAGE_CLONE_AREA
+
+typedef struct {
+	port_id		reply_port;		// port to send the reply to
+	const void	*address;		// address within area to clone
+} debug_nub_clone_area;
+
+typedef struct {
+	area_id		area;			// the ID of the newly cloned area, or an error
+	const void	*address;		// corresponding address in clone
+} debug_nub_clone_area_reply;
+
 // B_DEBUG_MESSAGE_SET_TEAM_FLAGS
 
 typedef struct {
@@ -445,6 +458,7 @@ typedef struct {
 typedef union {
 	debug_nub_read_memory			read_memory;
 	debug_nub_write_memory			write_memory;
+	debug_nub_clone_area			clone_area;
 	debug_nub_set_team_flags		set_team_flags;
 	debug_nub_set_thread_flags		set_thread_flags;
 	debug_nub_continue_thread		continue_thread;

headers/private/debug/debug_support.h

@@ -62,7 +62,7 @@ status_t debug_get_stack_frame(debug_context *context,
 typedef struct debug_symbol_lookup_context debug_symbol_lookup_context;
 typedef struct debug_symbol_iterator debug_symbol_iterator;
 
-status_t debug_create_symbol_lookup_context(team_id team, image_id image,
+status_t debug_create_symbol_lookup_context(debug_context *context, image_id image,
 			debug_symbol_lookup_context **lookupContext);
 				// imageID can be -1 if all images in the target team are
 				// desired, otherwise a valid image id is expected.

src/bin/debug/profile/SharedImage.cpp

@@ -38,10 +38,12 @@ SharedImage::~SharedImage()
 status_t
 SharedImage::Init(team_id owner, image_id imageID)
 {
+	debug_context debugContext = {owner, -1, -1};
+
 	// we need a temporary symbol lookup context
 	debug_symbol_lookup_context* lookupContext;
-	status_t error = debug_create_symbol_lookup_context(owner, imageID,
-		&lookupContext);
+	status_t error = debug_create_symbol_lookup_context(&debugContext,
+		imageID, &lookupContext);
 	if (error != B_OK) {
 		fprintf(stderr, "%s: Failed to create symbol lookup context "
 			"for team %" B_PRId32 ": %s\n",

src/kits/debug/SymbolLookup.cpp

@@ -12,6 +12,7 @@
 
 #include <new>
 
+#include <debug_support.h>
 #include <runtime_loader.h>
 #include <syscalls.h>
 
@@ -32,19 +33,38 @@ using namespace BPrivate::Debug;
 
 // PrepareAddress
 const void *
-Area::PrepareAddress(const void *address)
+Area::PrepareAddress(debug_context* debugContext, const void *address)
 {
 	TRACE(("Area::PrepareAddress(%p): area: %" B_PRId32 "\n", address, fRemoteID));
 
 	// clone the area, if not done already
 	if (fLocalID < 0) {
-		fLocalID = clone_area("cloned area", &fLocalAddress, B_ANY_ADDRESS,
-			B_READ_AREA, fRemoteID);
+		debug_nub_clone_area message;
+		message.reply_port = debugContext->reply_port;
+		message.address = address;
+
+		debug_nub_clone_area_reply reply;
+		status_t error = send_debug_message(debugContext, B_DEBUG_MESSAGE_CLONE_AREA,
+			&message, sizeof(message), &reply, sizeof(reply));
+		if (error != B_OK)
+			throw Exception(error);
+
+		fLocalID = reply.area;
 		if (fLocalID < 0) {
 			TRACE(("Area::PrepareAddress(): Failed to clone area %" B_PRId32
 				": %s\n", fRemoteID, strerror(fLocalID)));
 			throw Exception(fLocalID);
 		}
+
+		area_info info;
+		error = get_area_info(fLocalID, &info);
+		if (error < 0) {
+			TRACE(("Area::PrepareAddress(): Failed to get info for %" B_PRId32
+				": %s\n", fLocalID, strerror(info)));
+			throw Exception(fLocalID);
+		}
+
+		fLocalAddress = info.address;
 	}
 
 	// translate the address
@@ -60,8 +80,8 @@ Area::PrepareAddress(const void *address)
 // #pragma mark -
 
 // constructor
-RemoteMemoryAccessor::RemoteMemoryAccessor(team_id team)
-	: fTeam(team),
+RemoteMemoryAccessor::RemoteMemoryAccessor(debug_context* debugContext)
+	: fDebugContext(debugContext),
 	  fAreas()
 {
 }
@@ -80,16 +100,16 @@ RemoteMemoryAccessor::~RemoteMemoryAccessor()
 status_t
 RemoteMemoryAccessor::Init()
 {
-	// If the team is the kernel team, we don't try to clone the areas. Only
-	// SymbolLookup's image file functionality will be available.
-	if (fTeam == B_SYSTEM_TEAM)
+	// If we don't have a debug context, then there's nothing we can do.
+	// Only SymbolLookup's image file functionality will be available.
+	if (fDebugContext == NULL || fDebugContext->nub_port < 0)
 		return B_OK;
 
 	// get a list of the team's areas
 	area_info areaInfo;
 	ssize_t cookie = 0;
 	status_t error;
-	while ((error = get_next_area_info(fTeam, &cookie, &areaInfo)) == B_OK) {
+	while ((error = get_next_area_info(fDebugContext->team, &cookie, &areaInfo)) == B_OK) {
 		TRACE(("area %" B_PRId32 ": address: %p, size: %ld, name: %s\n",
 			areaInfo.area, areaInfo.address, areaInfo.size, areaInfo.name));
 
@@ -120,7 +140,7 @@ RemoteMemoryAccessor::PrepareAddress(const void *remoteAddress,
 		throw Exception(B_BAD_VALUE);
 	}
 
-	return _FindArea(remoteAddress, size).PrepareAddress(remoteAddress);
+	return _FindArea(remoteAddress, size).PrepareAddress(fDebugContext, remoteAddress);
 }
 
 
@@ -135,7 +155,7 @@ RemoteMemoryAccessor::PrepareAddressNoThrow(const void *remoteAddress,
 	if (area == NULL)
 		return NULL;
 
-	return area->PrepareAddress(remoteAddress);
+	return area->PrepareAddress(fDebugContext, remoteAddress);
 }
 
 
@@ -221,9 +241,9 @@ private:
 
 
 // constructor
-SymbolLookup::SymbolLookup(team_id team, image_id image)
+SymbolLookup::SymbolLookup(debug_context* debugContext, image_id image)
 	:
-	RemoteMemoryAccessor(team),
+	RemoteMemoryAccessor(debugContext),
 	fDebugArea(NULL),
 	fImages(),
 	fImageID(image)
@@ -249,14 +269,14 @@ SymbolLookup::Init()
 	if (error != B_OK)
 		return error;
 
-	if (fTeam != B_SYSTEM_TEAM) {
+	if (fDebugContext->team != B_SYSTEM_TEAM) {
 		TRACE(("SymbolLookup::Init(): searching debug area...\n"));
 
 		// find the runtime loader debug area
 		runtime_loader_debug_area *remoteDebugArea = NULL;
 		ssize_t cookie = 0;
 		area_info areaInfo;
-		while (get_next_area_info(fTeam, &cookie, &areaInfo) == B_OK) {
+		while (get_next_area_info(fDebugContext->team, &cookie, &areaInfo) == B_OK) {
 			if (strcmp(areaInfo.name, RUNTIME_LOADER_DEBUG_AREA_NAME) == 0) {
 				remoteDebugArea = (runtime_loader_debug_area*)areaInfo.address;
 				break;
@@ -287,7 +307,7 @@ SymbolLookup::Init()
 	if (fImageID < 0) {
 		// create a list of the team's images
 		int32 cookie = 0;
-		while (get_next_image_info(fTeam, &cookie, &imageInfo) == B_OK) {
+		while (get_next_image_info(fDebugContext->team, &cookie, &imageInfo) == B_OK) {
 			error = _LoadImageInfo(imageInfo);
 			if (error != B_OK)
 				return error;
@@ -325,7 +345,7 @@ SymbolLookup::LookupSymbolAddress(addr_t address, addr_t *_baseAddress,
 		_symbolName, _symbolNameLen, _exactMatch);
 
 	TRACE(("SymbolLookup::LookupSymbolAddress(): done: symbol: %p, image name: "
-		"%s, exact match: %d\n", symbolFound, image->name, exactMatch));
+		"%s, exact match: %d\n", symbolFound, image->Name(), _exactMatch ? *_exactMatch : -1));
 
 	if (symbolFound != NULL)
 		return B_OK;
@@ -432,9 +452,11 @@ SymbolLookup::_FindLoadedImageAtAddress(addr_t address) const
 		return NULL;
 
 	// iterate through the loaded images
-	for (const image_t *image = &Read(*Read(fDebugArea->loaded_images->head));
-		 image;
-		 image = &Read(*image->next)) {
+	const image_t *_image = Read(fDebugArea->loaded_images->head);
+	while (_image != NULL) {
+		const image_t *image = &Read(*_image);
+		_image = image->next;
+
 		if (image->regions[0].vmstart <= address
 			&& address < image->regions[0].vmstart + image->regions[0].size) {
 			return image;
@@ -449,13 +471,17 @@ SymbolLookup::_FindLoadedImageAtAddress(addr_t address) const
 const image_t*
 SymbolLookup::_FindLoadedImageByID(image_id id) const
 {
+	TRACE(("SymbolLookup::_FindLoadedImageByID(%" B_PRId32 ")\n", id));
+
 	if (fDebugArea == NULL)
 		return NULL;
 
-	// iterate through the images
-	for (const image_t *image = &Read(*Read(fDebugArea->loaded_images->head));
-		 image;
-		 image = &Read(*image->next)) {
+	// iterate through the loaded images
+	const image_t *_image = Read(fDebugArea->loaded_images->head);
+	while (_image != NULL) {
+		const image_t *image = &Read(*_image);
+		_image = image->next;
+
 		if (image->id == id)
 			return image;
 	}
@@ -512,7 +538,7 @@ SymbolLookup::_LoadImageInfo(const image_info& imageInfo)
 	status_t error = B_OK;
 
 	Image* image;
-	if (fTeam == B_SYSTEM_TEAM) {
+	if (fDebugContext->team == B_SYSTEM_TEAM) {
 		// kernel image
 		KernelImage* kernelImage = new(std::nothrow) KernelImage;
 		if (kernelImage == NULL)

src/kits/debug/SymbolLookup.h

@@ -15,6 +15,7 @@
 #include <util/DoublyLinkedList.h>
 
 
+struct debug_context;
 struct image_t;
 struct runtime_loader_debug_area;
 
@@ -79,7 +80,7 @@ public:
 			&& (addr_t)address < (addr_t)fLocalAddress + fSize;
 	}
 
-	const void *PrepareAddress(const void *address);
+	const void *PrepareAddress(debug_context* debugContext, const void *address);
 
 private:
 	area_id		fRemoteID;
@@ -93,7 +94,7 @@ private:
 // RemoteMemoryAccessor
 class RemoteMemoryAccessor {
 public:
-	RemoteMemoryAccessor(team_id team);
+	RemoteMemoryAccessor(debug_context* debugContext);
 	~RemoteMemoryAccessor();
 
 	status_t Init();
@@ -120,7 +121,7 @@ private:
 	typedef DoublyLinkedList<Area>	AreaList;
 
 protected:
-	team_id		fTeam;
+	debug_context* fDebugContext;
 
 private:
 	AreaList	fAreas;
@@ -137,7 +138,7 @@ struct SymbolIterator {
 // SymbolLookup
 class SymbolLookup : private RemoteMemoryAccessor {
 public:
-	SymbolLookup(team_id team, image_id image);
+	SymbolLookup(debug_context* debugContext, image_id image);
 	~SymbolLookup();
 
 	status_t Init();

src/kits/debug/debug_support.cpp

@@ -345,10 +345,10 @@ debug_get_stack_frame(debug_context *context, void *stackFrameAddress,
 
 // debug_create_symbol_lookup_context
 status_t
-debug_create_symbol_lookup_context(team_id team, image_id image,
+debug_create_symbol_lookup_context(debug_context *context, image_id image,
 	debug_symbol_lookup_context **_lookupContext)
 {
-	if (team < 0 || !_lookupContext)
+	if (context == NULL || _lookupContext == NULL)
 		return B_BAD_VALUE;
 
 	// create the lookup context
@@ -359,7 +359,7 @@ debug_create_symbol_lookup_context(team_id team, image_id image,
 	ObjectDeleter<debug_symbol_lookup_context> contextDeleter(lookupContext);
 
 	// create and init symbol lookup
-	SymbolLookup *lookup = new(std::nothrow) SymbolLookup(team, image);
+	SymbolLookup *lookup = new(std::nothrow) SymbolLookup(context, image);
 	if (lookup == NULL)
 		return B_NO_MEMORY;
 	ObjectDeleter<SymbolLookup> lookupDeleter(lookup);

src/kits/debugger/debugger_interface/local/LocalDebuggerInterface.cpp

@@ -606,10 +606,12 @@ status_t
 LocalDebuggerInterface::GetSymbolInfos(team_id team, image_id image,
 	BObjectList<SymbolInfo>& infos)
 {
+	DebugContextGetter contextGetter(fDebugContextPool);
+
 	// create a lookup context
 	debug_symbol_lookup_context* lookupContext;
-	status_t error = debug_create_symbol_lookup_context(team, image,
-		&lookupContext);
+	status_t error = debug_create_symbol_lookup_context(contextGetter.Context(),
+		image, &lookupContext);
 	if (error != B_OK)
 		return error;
 
@@ -651,10 +653,12 @@ status_t
 LocalDebuggerInterface::GetSymbolInfo(team_id team, image_id image, const char* name,
 	int32 symbolType, SymbolInfo& info)
 {
+	DebugContextGetter contextGetter(fDebugContextPool);
+
 	// create a lookup context
 	debug_symbol_lookup_context* lookupContext;
-	status_t error = debug_create_symbol_lookup_context(team, image,
-		&lookupContext);
+	status_t error = debug_create_symbol_lookup_context(contextGetter.Context(),
+		image, &lookupContext);
 	if (error != B_OK)
 		return error;
 

src/servers/debug/DebugServer.cpp

@@ -957,7 +957,7 @@ TeamDebugHandler::_PrintStackTrace(thread_id thread)
 	if (error == B_OK) {
 		// create a symbol lookup context
 		debug_symbol_lookup_context *lookupContext = NULL;
-		error = debug_create_symbol_lookup_context(fTeam, -1, &lookupContext);
+		error = debug_create_symbol_lookup_context(&fDebugContext, -1, &lookupContext);
 		if (error != B_OK) {
 			debug_printf("debug_server: Failed to create symbol lookup "
 				"context: %s\n", strerror(error));

src/system/kernel/debug/user_debugger.cpp

@@ -1752,6 +1752,7 @@ debug_nub_thread(void *)
 		union {
 			debug_nub_read_memory_reply			read_memory;
 			debug_nub_write_memory_reply		write_memory;
+			debug_nub_clone_area_reply			clone_area;
 			debug_nub_get_cpu_state_reply		get_cpu_state;
 			debug_nub_set_breakpoint_reply		set_breakpoint;
 			debug_nub_set_watchpoint_reply		set_watchpoint;
@@ -1835,6 +1836,51 @@ debug_nub_thread(void *)
 				break;
 			}
 
+			case B_DEBUG_MESSAGE_CLONE_AREA:
+			{
+				// get the parameters
+				replyPort = message.clone_area.reply_port;
+				const void *address = message.clone_area.address;
+				area_id result = 0;
+
+				// check the parameters
+				if (!IS_USER_ADDRESS(address))
+					result = B_NOT_ALLOWED;
+
+				// find the area
+				area_id sourceArea;
+				addr_t addressOffset;
+				if (result == B_OK) {
+					sourceArea = _user_area_for((void*)address);
+					if (sourceArea < 0) {
+						result = sourceArea;
+					} else {
+						area_info info;
+						result = get_area_info(sourceArea, &info);
+						addressOffset = (addr_t)address - (addr_t)info.address;
+					}
+				}
+
+				// clone it
+				if (result == B_OK) {
+					void* newAddress = NULL;
+					result = vm_clone_area(nubThread->team->debug_info.debugger_team,
+						"debugger-cloned area", &newAddress, B_ANY_ADDRESS, B_READ_AREA,
+						REGION_NO_PRIVATE_MAP, sourceArea, true);
+					reply.clone_area.address = (void*)((addr_t)newAddress + addressOffset);
+				}
+
+				reply.clone_area.area = result;
+
+				TRACE(("nub thread %" B_PRId32 ": B_DEBUG_MESSAGE_CLONE_AREA: "
+					"reply port: %" B_PRId32 ", address: %p, result: %" B_PRIx32 "\n",
+					nubThread->id, replyPort, address, result));
+
+				sendReply = true;
+				replySize = sizeof(debug_nub_clone_area_reply);
+				break;
+			}
+
 			case B_DEBUG_MESSAGE_SET_TEAM_FLAGS:
 			{
 				// get the parameters