Merge pull request #22000 from hrydgard/debugger-improvements

Turn off "Ignore bad memory accesses" in headless

Core/CmdLine.cpp

@@ -173,8 +173,8 @@ static const CommandLineParam g_autoParams[] = {
 	{POFF(appendConfig), CmdParamType::String, "appendconfig", '\0', "Merge config FILE into the current configuration"},
 	{POFF(root), CmdParamType::String, "root", 'r', "Mount root directory"},
 	{POFF(stateToLoad), CmdParamType::String, "state", '\0', "Load state from specified file"},
-	{POFF(compare), CmdParamType::Bool, "compare", 'c', "Enable comparison mode"},
-	{POFF(bench), CmdParamType::Bool, "bench", 'b', "Enable benchmark mode"},
+	{POFF(compare), CmdParamType::Bool, "compare", 'c', "Enable comparison mode", CmdLineMode::Headless},
+	{POFF(bench), CmdParamType::Bool, "bench", 'b', "Enable benchmark mode", CmdLineMode::Headless},
 	{POFF(oldAtrac), CmdParamType::Bool, "old-atrac", '\0', "Use old ATRAC decoder"},
 	{POFF(log), CmdParamType::String, "log", '\0', "Output log to FILE"},
 	{POFF(screenshotFilename), CmdParamType::String, "screenshot", '\0', "Take a screenshot and save to FILE"},
@@ -188,7 +188,7 @@ static const CommandLineParam g_autoParams[] = {
 	{POFF(breakAction), CmdParamType::Enum, "break", '\0', "Set the action for break exceptions", CmdLineMode::Both, g_ExceptionActionValues, ARRAY_SIZE(g_ExceptionActionValues)},
 	{POFF(logNativeCrashes), CmdParamType::Bool, "log-native-crashes", '\0', "Log a native stack trace (Windows only) on an otherwise-unhandled crash", CmdLineMode::Both},
 	{POFF(verbose), CmdParamType::Bool, "verbose", '\0', "Enable verbose output", CmdLineMode::Both},
-
+	{POFF(printEqualLines), CmdParamType::Bool, "print-equal-lines", '\0', "Print lines that are equal during comparison", CmdLineMode::Headless},
 	// TODO: At some point we should maybe simply expose all config settings to be set directly from the command line automatically?
 };
 

Core/CmdLine.h

@@ -91,6 +91,7 @@ struct CommandLineOptions {
 	std::optional<bool> bench;
 	std::optional<bool> verbose;
 	std::optional<double> timeout;
+	std::optional<bool> printEqualLines;
 
 	std::optional<std::string> screenshotFilename;
 	std::optional<std::string> screenshotFilenameSave;

Core/CoreParameter.h

@@ -58,6 +58,7 @@ struct CoreParameter {
 	Path mountIso;  // If non-empty, and fileToStart is an ELF or PBP, will mount this ISO in the background to umd1:.
 	Path mountRoot;  // If non-empty, and fileToStart is an ELF or PBP, mount this as host0: / umd0:.
 	std::string errorString;
+	bool loadGameConfigs = true;
 
 	bool startBreak = false;
 	std::string *collectDebugOutput = nullptr;

Core/HLE/HLE.cpp

@@ -1155,9 +1155,12 @@ void hleDoLogInternal(Log t, LogLevel level, u64 res, const char *file, int line
 	const char *errStr = nullptr;
 	switch (retmask) {
 	case 'x':
-		// Truncate the high bits of the result (from any sign extension.)
-		res = (u32)res;
-		if ((int)res < 0 && (errStr = KernelErrorToString((u32)res))) {
+	case 'X':
+		if (retmask == 'x') {
+			// Truncate the high bits of the result (from any sign extension.)
+			res = (u32)res;
+		}
+		if (retmask == 'x' && (int)res < 0 && (errStr = KernelErrorToString((u32)res))) {
 			// It's a known syscall error code, let's display it as string.
 			fmt = "%sSCE_KERNEL_ERROR_%s=%s(%s)%s";
 		} else {
@@ -1167,7 +1170,7 @@ void hleDoLogInternal(Log t, LogLevel level, u64 res, const char *file, int line
 		break;
 	case 'i':
 	case 'I':
-		if ((int)res < 0 && (errStr = KernelErrorToString((u32)res))) {
+		if (retmask == 'i' && (int)res < 0 && (errStr = KernelErrorToString((u32)res))) {
 			// It's a known syscall error code, let's display it as string.
 			fmt = "%s%s=%s(%s)%s";
 		} else {

Core/HLE/sceKernelMbx.cpp

@@ -122,28 +122,25 @@ struct Mbx : public KernelObject
 		Memory::Write_U32(ptr, beforePtr);
 	}
 
-	int ReceiveMessage(u32 receivePtr)
-	{
+	int ReceiveMessage(u32 receivePtr) {
 		u32 ptr = nmb.packetListHead;
+		if (!Memory::IsValidAddress(nmb.packetListHead)) {
+			return SCE_KERNEL_ERROR_ILLEGAL_ADDR;
+		}
 
 		// Check over the linked list and reset the head.
 		int c = 0;
-		while (true)
-		{
+		while (true) {
 			u32 next = Memory::Read_U32(nmb.packetListHead);
 			if (!Memory::IsValidAddress(next))
 				return SCE_KERNEL_ERROR_ILLEGAL_ADDR;
-			if (next == ptr)
-			{
-				if (nmb.packetListHead != ptr)
-				{
+			if (next == ptr) {
+				if (nmb.packetListHead != ptr) {
 					next = Memory::Read_U32(next);
 					Memory::Write_U32(next, nmb.packetListHead);
 					nmb.packetListHead = next;
 					break;
-				}
-				else
-				{
+				} else {
 					if (c < nmb.numMessages - 1)
 						return PSP_MBX_ERROR_DUPLICATE_MSG;
 
@@ -159,12 +156,10 @@ struct Mbx : public KernelObject
 		// Tell the receiver about the message.
 		Memory::Write_U32(ptr, receivePtr);
 		nmb.numMessages--;
-
 		return 0;
 	}
 
-	void DoState(PointerWrap &p) override
-	{
+	void DoState(PointerWrap &p) override {
 		auto s = p.Section("Mbx", 1);
 		if (!s)
 			return;
@@ -565,8 +560,13 @@ int sceKernelReferMbxStatus(SceUID id, u32 infoAddr) {
 	if (!info.IsValid())
 		return hleLogError(Log::sceKernel, -1, "invalid pointer");
 
-	for (int i = 0, n = m->nmb.numMessages; i < n; ++i)
-		m->nmb.packetListHead = Memory::Read_U32(m->nmb.packetListHead);
+	u32 packet = m->nmb.packetListHead;
+	for (int i = 0, n = m->nmb.numMessages; i < n; ++i) {
+		if (packet == 0 || !Memory::IsValidAddress(packet)) {
+			return hleLogError(Log::sceKernel, -1, "invalid packet list head");
+		}
+		packet = Memory::ReadUnchecked_U32(packet);
+	}
 
 	HLEKernel::CleanupWaitingThreads(WAITTYPE_MBX, id, m->waitingThreads);
 

Core/HLE/sceKernelVTimer.cpp

@@ -409,8 +409,14 @@ u32 sceKernelSetVTimerHandler(SceUID uid, u32 scheduleAddr, u32 handlerFuncAddr,
 	}
 
 	hleEatCycles(2000);
+	u64 schedule;
+	if (!Memory::IsValidAddress(scheduleAddr)) {
+		ERROR_LOG(Log::sceKernel, "sceKernelSetVTimerHandler: invalid schedule address %08x", scheduleAddr);
+		schedule = 0;
+	} else {
+		schedule = Memory::ReadUnchecked_U64(scheduleAddr);
+	}
 
-	u64 schedule = Memory::Read_U64(scheduleAddr);
 	vt->nvt.handlerAddr = handlerFuncAddr;
 	if (handlerFuncAddr) {
 		vt->nvt.commonAddr = commonAddr;

Core/HLE/sceReg.cpp

@@ -952,6 +952,11 @@ const KeyValue ROOT[] = {
 
 // Updater checks for CONFIG/SYSTEM/XMB.
 
+// not sure what modes exist, this is conjecture.
+enum RegOpenMode {
+	REG_OPEN_READONLY = 2,
+};
+
 void __RegInit() {
 	g_openRegistryMode = 0;
 	g_handleGen = 1337;
@@ -1012,6 +1017,11 @@ int sceRegOpenRegistry(u32 regParamAddr, int mode, u32 regHandleAddr) {
 		Memory::WriteUnchecked_U32(0, regHandleAddr);
 	}
 	g_openRegistryMode = mode;
+
+	if (g_openRegistryMode != REG_OPEN_READONLY) {
+		WARN_LOG(Log::HLE, "sceRegOpenRegistry: Opening registry in non-readonly mode. This is not yet supported (we'll simply emulate it as read-only anyway).");
+	}
+
 	return hleLogInfo(Log::sceReg, 0);
 }
 
@@ -1120,12 +1130,12 @@ int sceRegGetKeys(int catHandle, u32 bufAddr, int num) {
 		return hleLogError(Log::sceReg, 0, "Not an open category");
 	}
 
-	if (!Memory::IsValidRange(bufAddr, num * 27)) {
+	const int keyLen = 27; // 27 bytes per key name, including null terminator. For some reason?!?
+
+	if (!Memory::IsValidRange(bufAddr, num * keyLen)) {
 		return hleLogError(Log::sceReg, -1, "bad output addr");
 	}
 
-	const int addrLen = 27;  // for some reason
-
 	int count = 0;
 	const KeyValue *keyvals = LookupCategory(iter->second.path, &count);
 	if (!keyvals) {
@@ -1135,8 +1145,8 @@ int sceRegGetKeys(int catHandle, u32 bufAddr, int num) {
 	count = std::min(count, num);
 
 	for (int i = 0; i < num; i++) {
-		char *dest = (char *)Memory::GetPointerWrite(bufAddr + i * 27);
-		strncpy(dest, keyvals[i].name.c_str(), 27);
+		char *dest = (char *)Memory::GetPointerWrite(bufAddr + i * keyLen);
+		strncpy(dest, keyvals[i].name.c_str(), keyLen);
 	}
 
 	return hleLogInfo(Log::sceReg, 0);

Core/MemMap.h

@@ -158,6 +158,14 @@ inline const u8* GetPointerUnchecked(const u32 address) {
 #endif
 }
 
+inline u64 ReadUnchecked_U64(const u32 address) {
+#ifdef MASKED_PSP_MEMORY
+	return *(u64_le *)(base + (address & MEMVIEW32_MASK));
+#else
+	return *(u64_le *)(base + address);
+#endif
+}
+
 inline u32 ReadUnchecked_U32(const u32 address) {
 #ifdef MASKED_PSP_MEMORY
 	return *(u32_le *)(base + (address & MEMVIEW32_MASK));

Core/PSPLoaders.cpp

@@ -339,7 +339,7 @@ static Path NormalizePath(const Path &path) {
 #endif
 }
 
-bool Load_PSP_ELF_PBP(FileLoader *fileLoader, std::string_view discId, std::string *error_string) {
+bool Load_PSP_ELF_PBP(FileLoader *fileLoader, std::string_view discId, bool loadGameConfigs, std::string *error_string) {
 	// This is really just for headless, might need tweaking later.
 	if (PSP_CoreParameter().mountIsoLoader != nullptr) {
 		std::shared_ptr<BlockDevice> bd(ConstructBlockDevice(PSP_CoreParameter().mountIsoLoader, error_string));
@@ -441,7 +441,9 @@ bool Load_PSP_ELF_PBP(FileLoader *fileLoader, std::string_view discId, std::stri
 			File::Rename(oldNamePrefix.WithExtraExtension(".jpg"), newPrefix.WithExtraExtension(".jpg"));
 	}
 
-	g_Config.LoadGameConfig(discID);
+	if (loadGameConfigs) {
+		g_Config.LoadGameConfig(discID);
+	}
 
 	return __KernelLoadExec(finalName.c_str(), 0, error_string);
 }

Core/PSPLoaders.h

@@ -24,7 +24,7 @@ class FileLoader;
 class BlockDevice;
 
 bool Load_PSP_ISO(FileLoader *fileLoader, std::string *error_string);
-bool Load_PSP_ELF_PBP(FileLoader *fileLoader, std::string_view discId, std::string *error_string);
+bool Load_PSP_ELF_PBP(FileLoader *fileLoader, std::string_view discId, bool loadGameConfigs, std::string *error_string);
 bool Load_PSP_GE_Dump(FileLoader *fileLoader, std::string *error_string);
 
 bool MountGameISO(FileLoader *fileLoader, std::string *errorString);

Core/System.cpp

@@ -467,7 +467,7 @@ static bool CPU_Init(FileLoader *fileLoader, IdentifiedFileType type, std::strin
 			dir = ResolvePBPDirectory(Path(dir)).ToString();
 			pspFileSystem.SetStartingDirectory("ms0:/" + dir.substr(pos));
 		}
-		if (!Load_PSP_ELF_PBP(fileLoader, discId, errorString)) {
+		if (!Load_PSP_ELF_PBP(fileLoader, discId, g_CoreParameter.loadGameConfigs, errorString)) {
 			return false;
 		}
 		break;
@@ -478,7 +478,7 @@ static bool CPU_Init(FileLoader *fileLoader, IdentifiedFileType type, std::strin
 	case IdentifiedFileType::PSP_ELF:
 	{
 		INFO_LOG(Log::Loader, "File is an ELF or loose PBP %s", fileLoader->GetPath().c_str());
-		if (!Load_PSP_ELF_PBP(fileLoader, discId, errorString)) {
+		if (!Load_PSP_ELF_PBP(fileLoader, discId, g_CoreParameter.loadGameConfigs, errorString)) {
 			ERROR_LOG(Log::Loader, "Failed to load ELF or loose PBP: %s", errorString->c_str());
 			return false;
 		}

Core/Util/PortManager.cpp

@@ -21,10 +21,10 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
+
 // Most of the code are based on https://github.com/RJ/libportfwd and updated to the latest miniupnp library
 // All credit goes to him and the official miniupnp project! http://miniupnp.free.fr/
 
-
 #include <algorithm>  // find_if
 #include <cstring>
 #include <string>
@@ -43,10 +43,10 @@
 #include "Core/Util/PortManager.h"
 
 PortManager g_PortManager;
-bool upnpServiceRunning = false;
-std::thread upnpServiceThread;
-std::recursive_mutex upnpLock;
-std::deque<UPnPArgs> upnpReqs;
+static bool upnpServiceRunning = false;
+static std::thread upnpServiceThread;
+static std::recursive_mutex upnpLock;
+static std::deque<UPnPArgs> upnpReqs;
 
 PortManager::PortManager(): 
 	m_InitState(UPNP_INITSTATE_NONE),
@@ -549,4 +549,3 @@ void UPnP_Remove(const char* protocol, unsigned short port) {
 	std::lock_guard<std::recursive_mutex> upnpGuard(upnpLock);
 	upnpReqs.push_back({ UPNP_CMD_REMOVE, protocol, port, port });
 }
-

headless/Compare.cpp

@@ -189,7 +189,7 @@ std::string GetTestName(const Path &bootFilename)
 	return ChopEnd(ChopFront(ChopFront(bootFilename.ToString(), "tests/"), "pspautotests/tests/"), ".prx");
 }
 
-bool CompareOutput(const Path &bootFilename, const std::string &output, bool verbose) {
+bool CompareOutput(const Path &bootFilename, const std::string &output, bool verbose, bool printEqualLines) {
 	Path expect_filename = bootFilename.GetFileExtension() == ".prx" ? bootFilename.WithReplacedExtension(".prx", ".expected") : bootFilename.WithExtraExtension(".expected");
 	std::unique_ptr<FileLoader> expect_loader(ConstructFileLoader(expect_filename));
 
@@ -203,8 +203,14 @@ bool CompareOutput(const Path &bootFilename, const std::string &output, bool ver
 
 		bool failed = false;
 		while (expected.HasLines()) {
-			if (expected.Compare(actual))
+			std::string value = expected.Peek(0);
+			if (expected.Compare(actual)) {  // note: Compare actually advances if equal. This is pretty ugly.
+				if (printEqualLines) {
+					printf("= %s\n", value.c_str());
+				}
+				// Lines were equal.
 				continue;
+			}
 
 			if (!failed) {
 				GitHubActionsPrint("error", "Incorrect output for %s", currentTestName.c_str());
@@ -274,8 +280,8 @@ bool CompareOutput(const Path &bootFilename, const std::string &output, bool ver
 	}
 }
 
-static inline double CompareChannel(int pix1, int pix2) {
-	double diff = pix1 - pix2;
+static inline float CompareChannel(int pix1, int pix2) {
+	float diff = pix1 - pix2;
 	return diff * diff;
 }
 
@@ -284,7 +290,6 @@ static inline double ComparePixel(u32 pix1, u32 pix2) {
 	double r = CompareChannel(pix1 & 0xFF, pix2 & 0xFF);
 	double g = CompareChannel((pix1 >> 8) & 0xFF, (pix2 >> 8) & 0xFF);
 	double b = CompareChannel((pix1 >> 16) & 0xFF, (pix2 >> 16) & 0xFF);
-
 	return r + g + b;
 }
 

headless/Compare.h

@@ -30,7 +30,7 @@ Path ExpectedFromFilename(const Path &bootFilename);
 Path ExpectedScreenshotFromFilename(const Path &bootFilename);
 std::string GetTestName(const Path &bootFilename);
 
-bool CompareOutput(const Path &bootFilename, const std::string &output, bool verbose);
+bool CompareOutput(const Path &bootFilename, const std::string &output, bool verbose, bool printEqualLines);
 std::vector<u32> TranslateDebugBufferToCompare(const GPUDebugBuffer *buffer, u32 stride, u32 h);
 
 class ScreenshotComparer {

headless/Headless.cpp

@@ -7,6 +7,8 @@
 // > --root pspautotests/tests/../ --compare --timeout=5 --graphics=software pspautotests/tests/cpu/cpu_alu/cpu_alu.prx
 // Example command line for taking screenshots from a frame dump:
 // > -l --graphics=vulkan --screenshot-save=vt_ref.bmp "D:\PSP ISO\dump\Depth\11578 Virtua Tennis pause menu ULES00126_0002.zip" --resolution-scale=2
+// Example command line for messing with the vsh:
+// > -l --vsh --memread=break --memwrite=break --break=break
 //
 // NOTE: In MSVC, don't forget to set the working directory to $ProjectDir\.. in debug settings.
 
@@ -249,9 +251,10 @@ struct AutoTestOptions {
 	bool compare;
 	bool verbose;
 	bool bench;
+	bool printEqualLines;
 };
 
-bool RunAutoTest(HeadlessHost *headlessHost, CoreParameter &coreParameter, const AutoTestOptions &opt) {
+static bool RunAutoTest(HeadlessHost *headlessHost, CoreParameter &coreParameter, const AutoTestOptions &opt) {
 	using namespace Draw;
 
 	// Kinda ugly, trying to guesstimate the test name from filename...
@@ -353,7 +356,7 @@ bool RunAutoTest(HeadlessHost *headlessHost, CoreParameter &coreParameter, const
 	}
 
 	if (opt.compare && passed) {
-		passed = CompareOutput(coreParameter.fileToStart, output, opt.verbose);
+		passed = CompareOutput(coreParameter.fileToStart, output, opt.verbose, opt.printEqualLines);
 	}
 
 	return passed;
@@ -451,6 +454,7 @@ int main(int argc, const char* argv[]) {
 	testOptions.bench = cmdLineOptions.bench.value_or(false);
 	testOptions.timeout = cmdLineOptions.timeout.value_or(std::numeric_limits<double>::infinity());
 	testOptions.verbose = cmdLineOptions.verbose.value_or(false);
+	testOptions.printEqualLines = cmdLineOptions.printEqualLines.value_or(false);
 
 	bool fullLog = false;
 	const char *stateToLoad = 0;
@@ -550,7 +554,7 @@ int main(int argc, const char* argv[]) {
 	g_Config.iDumpFileTypes = 0;
 	g_Config.bEnableSound = false;
 	g_Config.bFirstRun = false;
-	g_Config.bIgnoreBadMemAccess = true;  // NOTE: A few tests rely on this, which is BAD: threads/mbx/refer/refer , threads/mbx/send/send, threads/vtimers/interrupt
+	g_Config.bIgnoreBadMemAccess = false;
 	// Never report from tests.
 	g_Config.sReportHost.clear();
 	g_Config.bAutoSaveSymbolMap = false;
@@ -601,6 +605,7 @@ int main(int argc, const char* argv[]) {
 	coreParameter.mountRoot = mountRoot.empty() ? Path() : Path(mountRoot);
 	coreParameter.startBreak = false;
 	coreParameter.headLess = true;
+	coreParameter.loadGameConfigs = false;
 	coreParameter.renderScaleFactor = cmdLineOptions.resolutionScale.value_or(1);
 	coreParameter.renderWidth = 480 * coreParameter.renderScaleFactor;
 	coreParameter.renderHeight = 272 * coreParameter.renderScaleFactor;
@@ -680,9 +685,15 @@ int main(int argc, const char* argv[]) {
 
 	std::vector<std::string> failedTests;
 	std::vector<std::string> passedTests;
+	std::vector<std::string> missingTests;
 
 	for (size_t i = 0; i < testFilenames.size(); ++i) {
 		coreParameter.fileToStart = Path(testFilenames[i]);
+		if (!File::Exists(coreParameter.fileToStart)) {
+			fprintf(stderr, "File not found: %s\n", coreParameter.fileToStart.c_str());
+			missingTests.push_back(testFilenames[i]);
+			continue;
+		}
 		if (testOptions.compare)
 			printf("%s:\n", coreParameter.fileToStart.c_str());
 		bool passed = RunAutoTest(headlessHost, coreParameter, testOptions);
@@ -714,9 +725,8 @@ int main(int argc, const char* argv[]) {
 	}
 
 	if (testOptions.compare) {
-		printf("%d tests passed, %d tests failed.\n", (int)passedTests.size(), (int)failedTests.size());
-		if (!failedTests.empty())
-		{
+		printf("%d tests passed, %d tests failed, %d tests missing.\n", (int)passedTests.size(), (int)failedTests.size(), (int)missingTests.size());
+		if (!failedTests.empty()) {
 			printf("Failed tests:\n");
 			for (size_t i = 0; i < failedTests.size(); ++i) {
 				printf("  %s\n", failedTests[i].c_str());

headless/README.md

@@ -1,6 +1,6 @@
 # PPSSPPHeadless
 
-Non-interactive, headless build of PPSSPP. It boots a PSP executable, PRX, or GE frame dump (`.ppdmp`) without a GUI, outputs emulated debug text to the console, optionally captures and compares screenshots, and exits.
+Non-interactive, headless build of PPSSPP. It boots a PSP executable, PRX, or GE frame dump (`.ppdmp`) without a GUI, outputs emulated debug text to the console, optionally captures and compares text output or screenshots, and exits.
 
 Primarily intended for:
 - Automated regression testing (via [pspautotests](https://github.com/hrydgard/pspautotests/))
@@ -120,7 +120,8 @@ When the MSE exceeds `--max-mse`, the following files are saved in the working d
 The `--compare` flag also compares emulated debug output (`printf` / `sceIoWrite` to the emulator channel) against a `.expected` text file:
 
 - For `.prx` files: same path with `.expected` extension.
-- The comparison is line-based with a diff algorithm that highlights insertions/deletions.
+- The comparison is line-based with a diff algorithm that highlights mismatches and insertions/deletions, although the support for the
+  latter is limited, it's not a full-on diff.
 
 If only a screenshot reference exists and no `.expected` file, the test passes as long as there is no unexpected text output.
 
@@ -157,13 +158,14 @@ Example workflow step:
 
 The following configuration is hardcoded for headless mode (config file is never saved):
 
+(For actual up-to-date hardcoded config, see the headless.cpp file).
+
 | Setting              | Value      | Notes                                    |
 |----------------------|------------|------------------------------------------|
 | Internal resolution  | 1× (480×272) | Fixed.                                 |
 | Hardware transform   | Enabled    |                                          |
 | Vertex decoder JIT   | Enabled    |                                          |
 | Software renderer JIT| Enabled    |                                          |
-| Skip GPU readback    | No skip    | Full readback for accurate screenshots.  |
 | Ignore bad mem access| Enabled    |                                          |
 | Firmware version     | 6.60       |                                          |
 | PSP model            | Slim       |                                          |

test.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python
+
 # Automated script to run the pspautotests test suite in PPSSPP.
 
 import sys
@@ -64,6 +65,7 @@ class Command(object):
 
 # Test names are the C files without the .c extension.
 # These have worked and should keep working always - regression tests.
+# -g flag runs these.
 tests_good = [
   "cpu/cpu_alu/cpu_alu",
   "cpu/cpu_alu/cpu_branch",
@@ -281,8 +283,6 @@ tests_good = [
   "threads/mbx/poll/poll",
   "threads/mbx/priority/priority",
   "threads/mbx/receive/receive",
-  "threads/mbx/refer/refer",
-  "threads/mbx/send/send",
   "threads/msgpipe/msgpipe",
   "threads/msgpipe/cancel",
   "threads/msgpipe/create",
@@ -381,6 +381,8 @@ tests_good = [
   "video/psmfplayer/stop",
 ]
 
+# Broken tests
+# -b flag runs these.
 tests_next = [
 # These are the next tests up for fixing. These run by default.
   "cpu/fpu/fcr",
@@ -467,6 +469,10 @@ tests_next = [
   "threads/callbacks/cancel",
   "threads/callbacks/count",
   "threads/callbacks/notify",
+  # These two mbx tests only appeared to work because they papered over bugs 
+  "threads/mbx/refer/refer",
+  "threads/mbx/send/send",
+
   "threads/scheduling/dispatch",
   "threads/scheduling/scheduling",
   "threads/threads/create",