From aefd1e3e0dbe4e77f8d7dbbfa04e15c2db615394 Mon Sep 17 00:00:00 2001 From: Jay Kwak <82421531+jkwak-work@users.noreply.github.com> Date: Thu, 31 Jul 2025 14:32:02 -0700 Subject: Handle debug-layer messages in a separate channel (#7988) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Handle debug-layer messages in a separate channel The Problem (Issue #7343) The issue was that Vulkan Validation Layer error messages were being mixed into regular test output, causing potential false positives or negatives. When using -enable-debug-layers true, validation messages would appear in the same output stream as test results, potentially matching //CHECK: patterns incorrectly. Example Problem: - Test expects: //CHECK: 1 - Validation layer prints: VALIDATION ERROR: 1 invalid buffer binding - Test incorrectly matches the "1" in the error message instead of the actual output Slang Test Communication Architecture Execution Modes Slang has 3 different execution modes controlled by SpawnType: enum class SpawnType { UseSharedLibrary, // In-process execution UseTestServer, // Out-of-process via persistent server UseFullyIsolatedTestServer, // Out-of-process via isolated server UseExe // Direct executable spawn } 1. In-Process Mode (UseSharedLibrary) ┌─────────────────────────────────────────┐ │ slang-test │ │ ┌─────────────┐ ┌─────────────────┐ │ │ │render-test │ │gfx-unit-test │ │ │ │unit-test │ │slangc library │ │ │ └─────────────┘ └─────────────────┘ │ │ │ │ │ │ └───► StdWriters ◄──┘ │ │ (shared) │ └─────────────────────────────────────────┘ - Communication: Direct function calls, shared memory - Debug callbacks: Single callback instance in StdWriters - Used when: Default mode for most tests 2. Out-of-Process Mode (UseTestServer) ┌──────────────────┐ JSON-RPC ┌──────────────────┐ │ slang-test │◄──over pipes────┤ test-server.exe │ │ │ │ │ │ ┌─────────────┐ │ │ ┌─────────────┐ │ │ │StdWriters │ │ │ │render-test │ │ │ │+debug │ │ │ │gfx-unit-test│ │ │ │callback │ │ │ │+debug │ │ │ └─────────────┘ │ │ │callback │ │ └──────────────────┘ │ └─────────────┘ │ └──────────────────┘ - Communication: JSON-RPC over stdin/stdout pipes - Debug callbacks: Separate instances in each process - Used when: CI/CD, multi-threaded testing, crash isolation 3. Direct Executable Mode (UseExe) ┌──────────────────┐ pipes ┌──────────────────┐ │ slang-test │◄───────────────┤ slangc.exe │ │ │ │ other tools │ └──────────────────┘ └──────────────────┘ - Communication: Standard process pipes (stdout/stderr) - Debug callbacks: None (external executables) - Used when: Testing external tools Communication Mechanisms Deep Dive JSON-RPC Protocol Over Pipes The test-server.exe communicates with slang-test using JSON-RPC over stdin/stdout pipes: // Parent process (slang-test) creates child with pipes Process* testServerProcess = /* spawn test-server.exe */; // JSONRPCConnection wraps the pipe communication JSONRPCConnection connection; connection.initWithStdStreams(); // Uses stdin/stdout pipes // Send RPC call TestServerProtocol::ExecutionResult result; connection.sendCall("executeTool", &args, &result); Key Point: The pipes carry structured JSON messages, not raw stdout/stderr. This is what enables clean separation of different data channels. Protocol Structure Your changes extend the ExecutionResult protocol: struct ExecutionResult { String stdOut; // Regular program output String stdError; // Error messages String debugLayer; // NEW: Debug/validation messages int32_t result; int32_t returnCode; }; Your Debug Layer Solution The Challenge Memory pointers cannot cross process boundaries. A debugCallback pointer in the parent process is meaningless in the child process. The Solution: String-Based Serialization You solved this by using string capture and serialization: 1. Debug Callback Interface (slang-std-writers.h) class IDebugCallback { virtual void handleMessage( DebugMessageType type, DebugMessageSource source, const char* message) = 0; }; 2. String-Capturing Implementation (slang-support.h) class CoreDebugCallback : public Slang::IDebugCallback { StringBuilder m_buf; // Captures messages as strings void handleMessage(DebugMessageType type, DebugMessageSource source, const char* message) { if (type == DebugMessageType::Error) { m_buf << message << '\n'; // Serialize to string } } String getString() { return m_buf.toString(); } // Extract accumulated messages }; 3. Bridge Between RHI and Core (slang-support.h) class CoreToRHIDebugBridge : public rhi::IDebugCallback { Slang::IDebugCallback* m_coreCallback; void handleMessage(rhi::DebugMessageType type, rhi::DebugMessageSource source, const char* message) { // Convert RHI types to core types and forward m_coreCallback->handleMessage(convertType(type), convertSource(source), message); } }; Data Flow: Debug Messages End-to-End In-Process Mode Flow GPU Driver → RHI Debug Callback → Core Debug Callback → String Buffer → Test Output Out-of-Process Mode Flow Child Process: GPU Driver → RHI Debug Callback → Core Debug Callback → String Buffer ↓ Parent Process: JSON-RPC Serialization Test Output ← String Processing ← ExecutionResult.debugLayer ←┘ Step-by-Step Example 1. Test Execution Starts // In test-server process CoreDebugCallback debugCallback; CoreToRHIDebugBridge bridge; bridge.setCoreCallback(&debugCallback); // Set up graphics device with debug layers deviceDesc.debugCallback = &bridge; 2. Graphics API Call Triggers Validation Error // Inside Vulkan driver (external code) // Validation layer detects error and calls our callback bridge.handleMessage(RHI_ERROR, RHI_LAYER, "Invalid buffer binding"); 3. Message Capture // In CoreDebugCallback::handleMessage m_buf << "Invalid buffer binding\n"; // Stored in string buffer 4. Test Completion & Serialization // Back in test-server TestServerProtocol::ExecutionResult result; result.debugLayer = debugCallback.getString(); // "Invalid buffer binding\n" result.stdOut = "1"; // Regular test output // Send via JSON-RPC connection.sendResult(&result); 5. Parent Process Receives & Separates Output // In slang-test process String output = buildTestOutput(result); // Results in clean separation: // standard output = { // 1 // } // debug layer = { // Invalid buffer binding // } Why This Solution Works 1. Process Isolation: Each process has its own callback objects, no shared pointers 2. String Serialization: Debug messages converted to strings that can cross process boundaries 3. Protocol Extension: Uses existing JSON-RPC infrastructure, just adds new field 4. Clean Separation: Debug messages never mix with stdout/stderr 5. Backward Compatibility: Existing tests unaffected, debug layer field optional Key Benefits - Eliminates False Positives: Debug messages can't interfere with //CHECK: patterns - Better Debugging: Debug messages clearly separated and labeled - Robust Architecture: Works across all execution modes - Minimal Changes: Leverages existing communication infrastructure This elegant solution transforms a fundamental cross-process communication challenge into a simple string serialization problem, using the existing test server architecture to cleanly separate validation layer messages from test results. * Cover the missing slang-test execution path * format code (#82) Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com> --------- Co-authored-by: slangbot Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com> --- tools/gfx-unit-test/gfx-test-util.cpp | 29 +----------------- tools/render-test/render-test-main.cpp | 22 ++----------- tools/render-test/slang-support.h | 56 ++++++++++++++++++++++++++++++++++ tools/slang-test/slang-test-main.cpp | 40 ++++++++++++++++++++++++ tools/test-server/test-server-main.cpp | 11 +++++++ tools/unit-test/slang-unit-test.h | 6 ++++ 6 files changed, 117 insertions(+), 47 deletions(-) (limited to 'tools') diff --git a/tools/gfx-unit-test/gfx-test-util.cpp b/tools/gfx-unit-test/gfx-test-util.cpp index b5a39a4f0..2e3efe09f 100644 --- a/tools/gfx-unit-test/gfx-test-util.cpp +++ b/tools/gfx-unit-test/gfx-test-util.cpp @@ -15,33 +15,6 @@ using Slang::ComPtr; namespace gfx_test { -class DebugPrinter : public rhi::IDebugCallback -{ -public: - virtual SLANG_NO_THROW void SLANG_MCALL handleMessage( - rhi::DebugMessageType type, - rhi::DebugMessageSource source, - const char* message) override - { - static const char* kTypeStrings[] = {"INFO", "WARN", "ERROR"}; - static const char* kSourceStrings[] = {"Layer", "Driver", "Slang"}; - if (type == rhi::DebugMessageType::Error) - { - fprintf( - stderr, - "[%s] (%s) %s\n", - kTypeStrings[int(type)], - kSourceStrings[int(source)], - message); - fflush(stderr); - } - } - static DebugPrinter* getInstance() - { - static DebugPrinter instance; - return &instance; - } -}; void diagnoseIfNeeded(slang::IBlob* diagnosticsBlob) { @@ -278,7 +251,7 @@ Slang::ComPtr createTestingDevice( if (context->enableDebugLayers) { deviceDesc.enableValidation = context->enableDebugLayers; - deviceDesc.debugCallback = DebugPrinter::getInstance(); + deviceDesc.debugCallback = context->debugCallback; } D3D12DeviceExtendedDesc extDesc = {}; diff --git a/tools/render-test/render-test-main.cpp b/tools/render-test/render-test-main.cpp index 23035280c..af0415bd0 100644 --- a/tools/render-test/render-test-main.cpp +++ b/tools/render-test/render-test-main.cpp @@ -4,6 +4,7 @@ #include "../../source/core/slang-test-tool-util.h" #include "../source/core/slang-io.h" +#include "../source/core/slang-std-writers.h" #include "../source/core/slang-string-util.h" #include "core/slang-token-reader.h" #include "options.h" @@ -1322,23 +1323,6 @@ static void renderDocBeginFrame() {} static void renderDocEndFrame() {} #endif -class StdWritersDebugCallback : public rhi::IDebugCallback -{ -public: - Slang::StdWriters* writers; - virtual SLANG_NO_THROW void SLANG_MCALL handleMessage( - rhi::DebugMessageType type, - rhi::DebugMessageSource source, - const char* message) override - { - SLANG_UNUSED(source); - if (type == rhi::DebugMessageType::Error) - { - writers->getOut().print("%s\n", message); - } - } -}; - static SlangResult _innerMain( Slang::StdWriters* stdWriters, SlangSession* session, @@ -1456,8 +1440,8 @@ static SlangResult _innerMain( } } - StdWritersDebugCallback debugCallback; - debugCallback.writers = stdWriters; + renderer_test::CoreToRHIDebugBridge debugCallback; + debugCallback.setCoreCallback(stdWriters->getDebugCallback()); // Use the profile name set on options if set input.profile = options.profileName.getLength() ? options.profileName : input.profile; diff --git a/tools/render-test/slang-support.h b/tools/render-test/slang-support.h index 916166c3a..51a2c8de1 100644 --- a/tools/render-test/slang-support.h +++ b/tools/render-test/slang-support.h @@ -1,6 +1,7 @@ // slang-support.h #pragma once +#include "core/slang-std-writers.h" #include "options.h" #include "shader-input-layout.h" #include "slang.h" @@ -10,6 +11,61 @@ namespace renderer_test { +/// Bridge from core debug callback to RHI debug callback +/// This allows core callbacks to receive messages from RHI systems +/// TODO: We should replace rhi::IDebugCallback with Slang::IDebugCallback. +class CoreToRHIDebugBridge : public rhi::IDebugCallback +{ +public: + void setCoreCallback(Slang::IDebugCallback* coreCallback) { m_coreCallback = coreCallback; } + + virtual SLANG_NO_THROW void SLANG_MCALL handleMessage( + rhi::DebugMessageType type, + rhi::DebugMessageSource source, + const char* message) override + { + if (m_coreCallback) + { + // Convert RHI types to core types + Slang::DebugMessageType coreType = static_cast(type); + Slang::DebugMessageSource coreSource = static_cast(source); + m_coreCallback->handleMessage(coreType, coreSource, message); + } + } + +private: + Slang::IDebugCallback* m_coreCallback = nullptr; +}; + +/// Core debug callback that captures debug messages in a string buffer +class CoreDebugCallback : public Slang::IDebugCallback +{ +public: + virtual SLANG_NO_THROW void SLANG_MCALL handleMessage( + Slang::DebugMessageType type, + Slang::DebugMessageSource source, + const char* message) override + { + SLANG_UNUSED(source); + + // Only capture error messages + if (type == Slang::DebugMessageType::Error) + { + m_buf << message; + if (message[strlen(message) - 1] != '\n') + { + m_buf << '\n'; + } + } + } + + void clear() { m_buf.clear(); } + Slang::String getString() { return m_buf.toString(); } + +private: + Slang::StringBuilder m_buf; +}; + struct ShaderCompileRequest { struct SourceInfo diff --git a/tools/slang-test/slang-test-main.cpp b/tools/slang-test/slang-test-main.cpp index 54ec1a842..97c63c392 100644 --- a/tools/slang-test/slang-test-main.cpp +++ b/tools/slang-test/slang-test-main.cpp @@ -24,6 +24,7 @@ #include "../../source/compiler-core/slang-downstream-compiler.h" #include "../../source/compiler-core/slang-language-server-protocol.h" #include "../../source/compiler-core/slang-nvrtc-compiler.h" +#include "../render-test/slang-support.h" #include "directory-util.h" #include "options.h" #include "parse-diagnostic-util.h" @@ -844,6 +845,9 @@ Result spawnAndWaitSharedLibrary( { StringBuilder stdErrorString; StringBuilder stdOutString; + renderer_test::CoreDebugCallback coreDebugCallback; + renderer_test::CoreToRHIDebugBridge rhiDebugBridge; + rhiDebugBridge.setCoreCallback(&coreDebugCallback); // Say static so not released StringWriter stdError(&stdErrorString, WriterFlag::IsConsole | WriterFlag::IsStatic); @@ -854,6 +858,7 @@ Result spawnAndWaitSharedLibrary( StdWriters stdWriters; stdWriters.setWriter(SLANG_WRITER_CHANNEL_STD_ERROR, &stdError); stdWriters.setWriter(SLANG_WRITER_CHANNEL_STD_OUTPUT, &stdOut); + stdWriters.setDebugCallback(&coreDebugCallback); if (exeName == "slangc" || exeName == "slangi") { @@ -876,6 +881,7 @@ Result spawnAndWaitSharedLibrary( outRes.standardError = stdErrorString; outRes.standardOutput = stdOutString; + outRes.debugLayer = coreDebugCallback.getString(); outRes.resultCode = (int)TestToolUtil::getReturnCode(res); @@ -1013,6 +1019,7 @@ static Result _executeRPC( outRes.resultCode = exeRes.returnCode; outRes.standardError = exeRes.stdError; outRes.standardOutput = exeRes.stdOut; + outRes.debugLayer = exeRes.debugLayer; return SLANG_OK; } @@ -1530,6 +1537,7 @@ String getOutput(const ExecuteResult& exeRes) String standardOuptut = exeRes.standardOutput; String standardError = exeRes.standardError; + String debugLayer = exeRes.debugLayer; // We construct a single output string that captures the results StringBuilder actualOutputBuilder; @@ -1540,6 +1548,12 @@ String getOutput(const ExecuteResult& exeRes) actualOutputBuilder.append("}\nstandard output = {\n"); actualOutputBuilder.append(standardOuptut); actualOutputBuilder.append("}\n"); + if (debugLayer.getLength() > 0) + { + actualOutputBuilder.append("debug layer = {\n"); + actualOutputBuilder.append(debugLayer); + actualOutputBuilder.append("}\n"); + } return actualOutputBuilder.produceString(); } @@ -3262,6 +3276,7 @@ static TestResult _runHLSLComparisonTest( String standardOutput = exeRes.standardOutput; String standardError = exeRes.standardError; + String debugLayer = exeRes.debugLayer; // We construct a single output string that captures the results StringBuilder actualOutputBuilder; @@ -3272,6 +3287,12 @@ static TestResult _runHLSLComparisonTest( actualOutputBuilder.append("}\nstandard output = {\n"); actualOutputBuilder.append(standardOutput); actualOutputBuilder.append("}\n"); + if (debugLayer.getLength() > 0) + { + actualOutputBuilder.append("debug layer = {\n"); + actualOutputBuilder.append(debugLayer); + actualOutputBuilder.append("}\n"); + } String actualOutput = actualOutputBuilder.produceString(); @@ -3339,6 +3360,7 @@ TestResult doGLSLComparisonTestRun( String standardOuptut = exeRes.standardOutput; String standardError = exeRes.standardError; + String debugLayer = exeRes.debugLayer; // We construct a single output string that captures the results StringBuilder outputBuilder; @@ -3349,6 +3371,12 @@ TestResult doGLSLComparisonTestRun( outputBuilder.append("}\nstandard output = {\n"); outputBuilder.append(standardOuptut); outputBuilder.append("}\n"); + if (debugLayer.getLength() > 0) + { + outputBuilder.append("debug layer = {\n"); + outputBuilder.append(debugLayer); + outputBuilder.append("}\n"); + } String outputPath = outputStem + outputKind; String output = outputBuilder.produceString(); @@ -3748,6 +3776,7 @@ TestResult doRenderComparisonTestRun( String standardOutput = exeRes.standardOutput; String standardError = exeRes.standardError; + String debugLayer = exeRes.debugLayer; // We construct a single output string that captures the results StringBuilder outputBuilder; @@ -3758,6 +3787,12 @@ TestResult doRenderComparisonTestRun( outputBuilder.append("}\nstandard output = {\n"); outputBuilder.append(standardOutput); outputBuilder.append("}\n"); + if (debugLayer.getLength() > 0) + { + outputBuilder.append("debug layer = {\n"); + outputBuilder.append(debugLayer); + outputBuilder.append("}\n"); + } String outputPath = outputStem + outputKind; String output = outputBuilder.produceString(); @@ -4733,12 +4768,17 @@ static SlangResult runUnitTestModule( if (!testModule) return SLANG_FAIL; + renderer_test::CoreDebugCallback coreDebugCallback; + renderer_test::CoreToRHIDebugBridge rhiDebugBridge; + rhiDebugBridge.setCoreCallback(&coreDebugCallback); + UnitTestContext unitTestContext; unitTestContext.slangGlobalSession = context->getSession(); unitTestContext.workDirectory = ""; unitTestContext.enabledApis = context->options.enabledApis; unitTestContext.enableDebugLayers = context->options.enableDebugLayers; unitTestContext.executableDirectory = context->exeDirectoryPath.getBuffer(); + unitTestContext.debugCallback = &rhiDebugBridge; auto testCount = testModule->getTestCount(); diff --git a/tools/test-server/test-server-main.cpp b/tools/test-server/test-server-main.cpp index a2f7f8153..e98be6e5a 100644 --- a/tools/test-server/test-server-main.cpp +++ b/tools/test-server/test-server-main.cpp @@ -10,7 +10,10 @@ #include "../../source/core/slang-string.h" #include "../../source/core/slang-test-tool-util.h" #include "../../source/core/slang-writer.h" +#include "../render-test/slang-support.h" +#include "gfx-unit-test/gfx-test-util.h" #include "slang-com-helper.h" +#include "slang-rhi.h" #include "test-server-diagnostics.h" #include "unit-test/slang-unit-test.h" @@ -422,6 +425,9 @@ SlangResult TestServer::_executeUnitTest(const JSONRPCCall& call) } TestReporter testReporter; + renderer_test::CoreDebugCallback coreDebugCallback; + renderer_test::CoreToRHIDebugBridge rhiDebugCallback; + rhiDebugCallback.setCoreCallback(&coreDebugCallback); testModule->setTestReporter(&testReporter); @@ -438,6 +444,7 @@ SlangResult TestServer::_executeUnitTest(const JSONRPCCall& call) unitTestContext.enabledApis = RenderApiFlags(args.enabledApis); unitTestContext.executableDirectory = m_exeDirectory.getBuffer(); unitTestContext.enableDebugLayers = args.enableDebugLayers; + unitTestContext.debugCallback = &rhiDebugCallback; auto testCount = testModule->getTestCount(); SLANG_ASSERT(testIndex >= 0 && testIndex < testCount); @@ -455,6 +462,7 @@ SlangResult TestServer::_executeUnitTest(const JSONRPCCall& call) TestServerProtocol::ExecutionResult result; result.result = SLANG_OK; + result.debugLayer = coreDebugCallback.getString(); if (testReporter.m_failCount > 0) { @@ -508,6 +516,7 @@ SlangResult TestServer::_executeTool(const JSONRPCCall& call) StdWriters stdWriters; StringBuilder stdOut; StringBuilder stdError; + renderer_test::CoreDebugCallback debugCallback; // Make writer/s act as if they are the console. RefPtr stdOutWriter(new StringWriter(&stdOut, WriterFlag::IsConsole)); @@ -515,6 +524,7 @@ SlangResult TestServer::_executeTool(const JSONRPCCall& call) stdWriters.setWriter(SLANG_WRITER_CHANNEL_STD_ERROR, stdErrorWriter); stdWriters.setWriter(SLANG_WRITER_CHANNEL_STD_OUTPUT, stdOutWriter); + stdWriters.setDebugCallback(&debugCallback); // HACK, to make behavior the same as previously if (args.toolName == "slangc") @@ -529,6 +539,7 @@ SlangResult TestServer::_executeTool(const JSONRPCCall& call) result.result = funcRes; result.stdError = stdError; result.stdOut = stdOut; + result.debugLayer = debugCallback.getString(); result.returnCode = int32_t(TestToolUtil::getReturnCode(result.result)); return m_connection->sendResult(&result, id); diff --git a/tools/unit-test/slang-unit-test.h b/tools/unit-test/slang-unit-test.h index 49cb5d8d7..db921ded9 100644 --- a/tools/unit-test/slang-unit-test.h +++ b/tools/unit-test/slang-unit-test.h @@ -37,6 +37,11 @@ public: ITestReporter* getTestReporter(); +namespace rhi +{ +class IDebugCallback; +} + struct UnitTestContext { slang::IGlobalSession* slangGlobalSession; @@ -44,6 +49,7 @@ struct UnitTestContext const char* executableDirectory; Slang::RenderApiFlags enabledApis; bool enableDebugLayers; + rhi::IDebugCallback* debugCallback = nullptr; }; typedef void (*UnitTestFunc)(UnitTestContext*); -- cgit v1.2.3