Debugger: Rework parts of report generation.

Team:
- Adjust report generation event to include a final status code for listeners.

CliContext,TeamWindow,ReportUserinterface:
- Use aforementioned status code to indicate whether report generation
  succeeded or failed.

DebugReportGenerator:
- Notify listeners if report generation fails. This may have previously
  been responsible for some bug reports where it was indicated that the
  debugger hung without exiting after being asked to save a report from
  a crashed app.
- When dumping disassembly, retrieve it directly if necessary rather than
  requesting it via the user interface listener. This also fixes the quirk
  that requesting to save a crash report while looking at the source code
  of a function would trigger switching it to disassembly visually.
- When walking the list of threads to dump, acquire references to all of
  them before starting. Otherwise, it was potentially possible for a running
  but not crashed thread to exit while we were generating the report, leaving
  us with a pointer to a deleted thread. This was most likely the cause of one
  of the crashes reported in #13082.
- When receiving the notification that source code state has changed, clear
  the waiting function. Otherwise, it was potentially possible for us to get
  other state change notifications, leading to the data semaphore being
  released too often. This would then cause later potential waits such as
  the stack frame memory dump to not actually wait when they should,
  potentially leading them to dereference objects that weren't yet ready.
  This fixes another of the crashes in #13802.

headers/private/debugger/model/Team.h

@@ -265,7 +265,7 @@ public:
 
 			// debug report related service methods
 			void				NotifyDebugReportChanged(
-									const char* reportPath);
+									const char* reportPath, status_t result);
 
 			// core file related service methods
 			void				NotifyCoreFileChanged(
@@ -434,13 +434,17 @@ protected:
 class Team::DebugReportEvent : public Event {
 public:
 								DebugReportEvent(uint32 type, Team* team,
-									const char* reportPath);
+									const char* reportPath,
+									status_t finalStatus);
 
 			const char*			GetReportPath() const	{ return fReportPath; }
+			status_t			GetFinalStatus() const	{ return fFinalStatus; }
 protected:
 			const char*			fReportPath;
+			status_t			fFinalStatus;
 };
 
+
 class Team::CoreFileChangedEvent : public Event {
 public:
 								CoreFileChangedEvent(uint32 type, Team* team,

src/apps/debugger/user_interface/cli/CliContext.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2015, Rene Gollent, [email protected].
+ * Copyright 2012-2016, Rene Gollent, [email protected].
  * Copyright 2012, Ingo Weinhold, [email protected].
  * Distributed under the terms of the MIT License.
  */
@@ -508,8 +508,13 @@ CliContext::ExpressionEvaluated(ExpressionInfo* info, status_t result,
 void
 CliContext::DebugReportChanged(const Team::DebugReportEvent& event)
 {
-	printf("Successfully saved debug report to %s\n",
+	if (event.GetFinalStatus() == B_OK) {
-		event.GetReportPath());
+		printf("Successfully saved debug report to %s\n",
+			event.GetReportPath());
+	} else {
+		fprintf(stderr, "Failed to write debug report: %s\n", strerror(
+				event.GetFinalStatus()));
+	}
 
 	_QueueEvent(new(std::nothrow) Event(EVENT_DEBUG_REPORT_CHANGED));
 	_SignalInputLoop(EVENT_DEBUG_REPORT_CHANGED);

src/apps/debugger/user_interface/gui/team_window/TeamWindow.cpp

@@ -305,9 +305,16 @@ TeamWindow::MessageReceived(BMessage* message)
 		}
 		case MSG_DEBUG_REPORT_SAVED:
 		{
+			status_t finalStatus = message->GetInt32("status", B_OK);
 			BString data;
-			data.SetToFormat("Debug report successfully saved to '%s'",
+			if (finalStatus == B_OK) {
-				message->FindString("path"));
+				data.SetToFormat("Debug report successfully saved to '%s'",
+					message->FindString("path"));
+			} else {
+				data.SetToFormat("Failed to save debug report: '%s'",
+					strerror(finalStatus));
+			}
+
 			BAlert *alert = new(std::nothrow) BAlert("Report saved",
 				data.String(), "Close");
 			if (alert == NULL)
@@ -964,6 +971,7 @@ TeamWindow::DebugReportChanged(const Team::DebugReportEvent& event)
 {
 	BMessage message(MSG_DEBUG_REPORT_SAVED);
 	message.AddString("path", event.GetReportPath());
+	message.AddInt32("status", event.GetFinalStatus());
 	PostMessage(&message);
 }
 

src/apps/debugger/user_interface/report/ReportUserInterface.cpp

@@ -237,6 +237,11 @@ ReportUserInterface::ThreadStateChanged(const Team::ThreadEvent& event)
 void
 ReportUserInterface::DebugReportChanged(const Team::DebugReportEvent& event)
 {
-	printf("Debug report saved to %s\n", event.GetReportPath());
+	if (event.GetFinalStatus() == B_OK)
+		printf("Debug report saved to %s\n", event.GetReportPath());
+	else {
+		fprintf(stderr, "Failed to write debug report: %s\n", strerror(
+				event.GetFinalStatus()));
+	}
 	release_sem(fReportSemaphore);
 }

src/kits/debugger/controllers/DebugReportGenerator.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2015, Rene Gollent, rene@gollent.com.
+ * Copyright 2012-2016, Rene Gollent, rene@gollent.com.
  * Distributed under the terms of the MIT License.
  */
 
@@ -32,6 +32,7 @@
 #include "StringUtils.h"
 #include "SystemInfo.h"
 #include "Team.h"
+#include "TeamDebugInfo.h"
 #include "Thread.h"
 #include "Type.h"
 #include "UiUtils.h"
@@ -65,7 +66,6 @@ DebugReportGenerator::DebugReportGenerator(::Team* team,
 	fCurrentBlock(NULL),
 	fBlockRetrievalStatus(B_OK),
 	fTraceWaitingThread(NULL),
-	fSourceWaitForDisassembly(false),
 	fSourceWaitingFunction(NULL)
 {
 	fTeam->AddListener(this);
@@ -128,9 +128,9 @@ DebugReportGenerator::Create(::Team* team, UserInterfaceListener* listener,
 
 
 status_t
-DebugReportGenerator::_GenerateReport(const entry_ref& outputPath)
+DebugReportGenerator::_GenerateReport(const char* outputPath)
 {
-	BFile file(&outputPath, B_WRITE_ONLY | B_CREATE_FILE | B_ERASE_FILE);
+	BFile file(outputPath, B_WRITE_ONLY | B_CREATE_FILE | B_ERASE_FILE);
 	status_t result = file.InitCheck();
 	if (result != B_OK)
 		return result;
@@ -155,10 +155,8 @@ DebugReportGenerator::_GenerateReport(const entry_ref& outputPath)
 	if (result != B_OK)
 		return result;
 
-	BPath path(&outputPath);
-
 	AutoLocker< ::Team> teamLocker(fTeam);
-	fTeam->NotifyDebugReportChanged(path.Path());
+	fTeam->NotifyDebugReportChanged(outputPath, B_OK);
 
 	return B_OK;
 }
@@ -171,8 +169,12 @@ DebugReportGenerator::MessageReceived(BMessage* message)
 		case MSG_GENERATE_DEBUG_REPORT:
 		{
 			entry_ref ref;
-			if (message->FindRef("target", &ref) == B_OK)
+			if (message->FindRef("target", &ref) == B_OK) {
-				_GenerateReport(ref);
+				BPath path(&ref);
+				status_t error = _GenerateReport(path.Path());
+				if (error != B_OK)
+					fTeam->NotifyDebugReportChanged(path.Path(), error);
+			}
 			break;
 		}
 
@@ -223,17 +225,15 @@ DebugReportGenerator::FunctionSourceCodeChanged(Function* function)
 {
 	AutoLocker< ::Team> teamLocker(fTeam);
 	if (function == fSourceWaitingFunction) {
-		function_source_state state;
+		function_source_state state = function->SourceCodeState();
-		if (fSourceWaitForDisassembly)
-			state = function->FirstInstance()->SourceCodeState();
-		else
-			state = function->SourceCodeState();
 
 		switch (state) {
 			case FUNCTION_SOURCE_LOADED:
 			case FUNCTION_SOURCE_SUPPRESSED:
 			case FUNCTION_SOURCE_UNAVAILABLE:
 			{
+				fSourceWaitingFunction->RemoveListener(this);
+				fSourceWaitingFunction = NULL;
 				release_sem(fTeamDataSem);
 				// fall through
 			}
@@ -455,8 +455,10 @@ DebugReportGenerator::_DumpRunningThreads(BFile& _output)
 	for (ThreadList::ConstIterator it = fTeam->Threads().GetIterator();
 		  (thread = it.Next());) {
 		 threads.AddItem(thread);
+		 thread->AcquireReference();
 	}
 
+	status_t error = B_OK;
 	threads.SortItems(&_CompareThreads);
 	for (int32 i = 0; (thread = threads.ItemAt(i)) != NULL; i++) {
 		try {
@@ -478,19 +480,21 @@ DebugReportGenerator::_DumpRunningThreads(BFile& _output)
 				// we need to release our lock on the team here
 				// since we might need to block and wait
 				// on the stack trace.
-				BReference< ::Thread> threadRef(thread);
 				locker.Unlock();
-				status_t error = _DumpDebuggedThreadInfo(_output, thread);
+				error = _DumpDebuggedThreadInfo(_output, thread);
-				if (error != B_OK)
-					return error;
 				locker.Lock();
+				if (error != B_OK)
+					break;
 			}
 		} catch (...) {
-			return B_NO_MEMORY;
+			error = B_NO_MEMORY;
 		}
 	}
 
-	return B_OK;
+	for (int32 i = 0; (thread = threads.ItemAt(i)) != NULL; i++)
+		thread->ReleaseReference();
+
+	return error;
 }
 
 
@@ -516,7 +520,7 @@ DebugReportGenerator::_DumpDebuggedThreadInfo(BFile& _output,
 		} while (error == B_INTERRUPTED);
 
 		if (error != B_OK)
-			break;
+			return error;
 
 		locker.Lock();
 	}
@@ -544,7 +548,6 @@ DebugReportGenerator::_DumpDebuggedThreadInfo(BFile& _output,
 				&& functionInstance->SourceCodeState()
 					== FUNCTION_SOURCE_NOT_LOADED) {
 				fSourceWaitingFunction = function;
-				fSourceWaitForDisassembly = false;
 				fSourceWaitingFunction->AddListener(this);
 				fListener->FunctionSourceCodeRequested(functionInstance);
 
@@ -555,11 +558,9 @@ DebugReportGenerator::_DumpDebuggedThreadInfo(BFile& _output,
 				} while (error == B_INTERRUPTED);
 
 				if (error != B_OK)
-					break;
+					return error;
 
 				locker.Lock();
-
-				fSourceWaitingFunction->RemoveListener(this);
 			}
 		}
 
@@ -589,7 +590,8 @@ DebugReportGenerator::_DumpDebuggedThreadInfo(BFile& _output,
 		// only dump the topmost frame
 		if (i == 0) {
 			locker.Unlock();
-			error = _DumpFunctionDisassembly(_output, frame->InstructionPointer());
+			error = _DumpFunctionDisassembly(_output,
+				frame->InstructionPointer());
 			if (error != B_OK)
 				return error;
 			error = _DumpStackFrameMemory(_output, thread->GetCpuState(),
@@ -658,55 +660,50 @@ DebugReportGenerator::_DumpFunctionDisassembly(BFile& _output,
 	AutoLocker< ::Team> teamLocker(fTeam);
 	BString data;
 	FunctionInstance* instance = NULL;
-	Statement* statement = NULL;
+	Image* image = fTeam->ImageByAddress(instructionPointer);
-	status_t error = fTeam->GetStatementAtAddress(instructionPointer, instance,
+	if (image == NULL) {
-		statement);
+		data.SetToFormat("\t\t\tUnable to retrieve disassembly for IP %#"
-	if (error != B_OK) {
+			B_PRIx64 ": address not contained in any valid image.\n",
-		data.SetToFormat("Unable to retrieve disassembly for IP %#" B_PRIx64
+			instructionPointer);
-				": %s\n", instructionPointer, strerror(error));
 		WRITE_AND_CHECK(_output, data);
 		return B_OK;
 	}
 
+	ImageDebugInfo* info = image->GetImageDebugInfo();
+	if (info == NULL) {
+		data.SetToFormat("\t\t\tUnable to retrieve disassembly for IP %#"
+			B_PRIx64 ": no debug info available for image '%s'.\n",
+			instructionPointer,	image->Name().String());
+		WRITE_AND_CHECK(_output, data);
+		return B_OK;
+	}
+
+	instance = info->FunctionAtAddress(instructionPointer);
+	if (instance == NULL) {
+		data.SetToFormat("\t\t\tUnable to retrieve disassembly for IP %#"
+			B_PRIx64 ": address does not point to a function.\n",
+			instructionPointer);
+		WRITE_AND_CHECK(_output, data);
+		return B_OK;
+	}
+
+	Statement* statement = NULL;
 	DisassembledCode* code = instance->GetSourceCode();
-	Function* function = instance->GetFunction();
+	BReference<DisassembledCode> codeReference;
 	if (code == NULL) {
-		switch (function->SourceCodeState()) {
+		status_t error = fTeam->DebugInfo()->DisassembleFunction(instance,
-			case FUNCTION_SOURCE_NOT_LOADED:
+			code);
-			case FUNCTION_SOURCE_LOADED:
+		if (error != B_OK) {
-				// FUNCTION_SOURCE_LOADED is included since, if we entered
+			data.SetToFormat("\t\t\tUnable to retrieve disassembly for IP %#"
-				// here, it implies that the high level source for the
+				B_PRIx64 ": %s.\n", instructionPointer, strerror(error));
-				// function has been loaded, but the disassembly has not.
+			WRITE_AND_CHECK(_output, data);
-				function->AddListener(this);
+			return B_OK;
-				fSourceWaitingFunction = function;
-				fSourceWaitForDisassembly = true;
-				fListener->FunctionSourceCodeRequested(instance, true);
-				// fall through
-			case FUNCTION_SOURCE_LOADING:
-			{
-				teamLocker.Unlock();
-				do {
-					error = acquire_sem(fTeamDataSem);
-				} while (error == B_INTERRUPTED);
-
-				if (error != B_OK)
-					return error;
-
-				teamLocker.Lock();
-				fSourceWaitingFunction->RemoveListener(this);
-				break;
-			}
-			default:
-				return B_OK;
 		}
 
-		if (instance->SourceCodeState() == 	FUNCTION_SOURCE_UNAVAILABLE)
+		codeReference.SetTo(code, true);
-			return B_OK;
+		statement = code->StatementAtAddress(instructionPointer);
-
+	} else
-		error = fTeam->GetStatementAtAddress(instructionPointer, instance,
+		codeReference.SetTo(code);
-			statement);
-		code = instance->GetSourceCode();
-	}
 
 	SourceLocation location = statement->StartSourceLocation();
 
@@ -741,10 +738,12 @@ DebugReportGenerator::_DumpStackFrameMemory(BFile& _output,
 		endAddress = framePointer;
 	}
 
-	status_t error;
+	if (endAddress <= startAddress)
+		return B_OK;
+
 	if (fCurrentBlock == NULL || !fCurrentBlock->Contains(startAddress)) {
+		status_t error;
 		fListener->InspectRequested(startAddress, this);
-		error = B_OK;
 		do {
 			error = acquire_sem(fTeamDataSem);
 		} while (error == B_INTERRUPTED);
@@ -827,6 +826,7 @@ DebugReportGenerator::_HandleMemoryBlockRetrieved(TeamMemoryBlock* block,
 }
 
 
+
 /*static*/ int
 DebugReportGenerator::_CompareAreas(const AreaInfo* a, const AreaInfo* b)
 {

src/kits/debugger/controllers/DebugReportGenerator.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2013, Rene Gollent, rene@gollent.com.
+ * Copyright 2012-2016, Rene Gollent, rene@gollent.com.
  * Distributed under the terms of the MIT License.
  */
 #ifndef DEBUG_REPORT_GENERATOR_H
@@ -64,7 +64,7 @@ private:
 	virtual	void				FunctionSourceCodeChanged(Function* function);
 
 private:
-			status_t			_GenerateReport(const entry_ref& outputPath);
+			status_t			_GenerateReport(const char* outputPath);
 			status_t			_GenerateReportHeader(BFile& _output);
 			status_t			_DumpLoadedImages(BFile& _output);
 			status_t			_DumpAreas(BFile& _output);
@@ -103,7 +103,6 @@ private:
 			TeamMemoryBlock*	fCurrentBlock;
 			status_t			fBlockRetrievalStatus;
 			::Thread*			fTraceWaitingThread;
-			bool				fSourceWaitForDisassembly;
 			Function*			fSourceWaitingFunction;
 };
 

src/kits/debugger/model/Team.cpp

@@ -842,12 +842,12 @@ Team::NotifyWatchpointChanged(Watchpoint* watchpoint)
 
 
 void
-Team::NotifyDebugReportChanged(const char* reportPath)
+Team::NotifyDebugReportChanged(const char* reportPath, status_t result)
 {
 	for (ListenerList::Iterator it = fListeners.GetIterator();
 			Listener* listener = it.Next();) {
 		listener->DebugReportChanged(DebugReportEvent(
-			TEAM_EVENT_DEBUG_REPORT_CHANGED, this, reportPath));
+			TEAM_EVENT_DEBUG_REPORT_CHANGED, this, reportPath, result));
 	}
 }
 
@@ -1036,10 +1036,11 @@ Team::ConsoleOutputEvent::ConsoleOutputEvent(uint32 type, Team* team,
 
 
 Team::DebugReportEvent::DebugReportEvent(uint32 type, Team* team,
-	const char* reportPath)
+	const char* reportPath, status_t finalStatus)
 	:
 	Event(type, team),
-	fReportPath(reportPath)
+	fReportPath(reportPath),
+	fFinalStatus(finalStatus)
 {
 }