From 2c9560581b97ef720b0eb92ed8554b98616e9396 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Fri, 10 Jan 2025 21:10:08 -0500 Subject: [PATCH] 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 Reviewed-by: waddlesplash --- headers/os/kernel/debugger.h | 16 +++- headers/private/debug/debug_support.h | 2 +- src/bin/debug/profile/SharedImage.cpp | 6 +- src/kits/debug/SymbolLookup.cpp | 76 +++++++++++++------ src/kits/debug/SymbolLookup.h | 9 ++- src/kits/debug/debug_support.cpp | 6 +- .../local/LocalDebuggerInterface.cpp | 12 ++- src/servers/debug/DebugServer.cpp | 2 +- src/system/kernel/debug/user_debugger.cpp | 46 +++++++++++ 9 files changed, 134 insertions(+), 41 deletions(-) diff --git a/headers/os/kernel/debugger.h b/headers/os/kernel/debugger.h index f8be98bb3c..3dca2158fc 100644 --- a/headers/os/kernel/debugger.h +++ b/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; diff --git a/headers/private/debug/debug_support.h b/headers/private/debug/debug_support.h index d7676af9e7..0c0371b057 100644 --- a/headers/private/debug/debug_support.h +++ b/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. diff --git a/src/bin/debug/profile/SharedImage.cpp b/src/bin/debug/profile/SharedImage.cpp index d3dcc599bd..fd7b17b3b0 100644 --- a/src/bin/debug/profile/SharedImage.cpp +++ b/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", diff --git a/src/kits/debug/SymbolLookup.cpp b/src/kits/debug/SymbolLookup.cpp index 5715a14d8e..f862b6a5ee 100644 --- a/src/kits/debug/SymbolLookup.cpp +++ b/src/kits/debug/SymbolLookup.cpp @@ -12,6 +12,7 @@ #include +#include #include #include @@ -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) diff --git a/src/kits/debug/SymbolLookup.h b/src/kits/debug/SymbolLookup.h index 4631c02662..7bc0aaaec9 100644 --- a/src/kits/debug/SymbolLookup.h +++ b/src/kits/debug/SymbolLookup.h @@ -15,6 +15,7 @@ #include +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 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(); diff --git a/src/kits/debug/debug_support.cpp b/src/kits/debug/debug_support.cpp index e6277d1976..396d6c88c0 100644 --- a/src/kits/debug/debug_support.cpp +++ b/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 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 lookupDeleter(lookup); diff --git a/src/kits/debugger/debugger_interface/local/LocalDebuggerInterface.cpp b/src/kits/debugger/debugger_interface/local/LocalDebuggerInterface.cpp index 68ea324e2d..d14ca8ddea 100644 --- a/src/kits/debugger/debugger_interface/local/LocalDebuggerInterface.cpp +++ b/src/kits/debugger/debugger_interface/local/LocalDebuggerInterface.cpp @@ -606,10 +606,12 @@ status_t LocalDebuggerInterface::GetSymbolInfos(team_id team, image_id image, BObjectList& 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; diff --git a/src/servers/debug/DebugServer.cpp b/src/servers/debug/DebugServer.cpp index f4553d5edd..2fd7accd9c 100644 --- a/src/servers/debug/DebugServer.cpp +++ b/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)); diff --git a/src/system/kernel/debug/user_debugger.cpp b/src/system/kernel/debug/user_debugger.cpp index 997dd12035..d4f38edcf8 100644 --- a/src/system/kernel/debug/user_debugger.cpp +++ b/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