mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-08 04:08:32 +09:00
[Fix] (MG_Util, MG_State): five latent frontend bugs the async work made load-bearing
- SpvcSession's move constructor and move assignment dropped the parsed metadata, so a moved-to session silently reported empty reflection. - ParseComputeLocalSize used std::stoull, whose std::out_of_range escaped glCompileShader on an oversized local_size literal; now std::from_chars saturating to UINT_MAX, pinned by a regression test that reproduced the escaping exception. - The compute local_size std::regex was rebuilt on every compile; hoisted. - LinkProgram dumped every shader's full source through MGLOG_D per link. - glslang::FinalizeProcess ran before the GL context tore down, leaving the context's live TShaders pointing at freed builtin symbol tables.
This commit is contained in:
+6
-1
@@ -37,7 +37,6 @@ namespace MobileGL {
|
||||
if (logLifecycle) {
|
||||
MGLOG_I("MobileGL closing...");
|
||||
}
|
||||
glslang::FinalizeProcess();
|
||||
// GL syncs die with their contexts, and every context is gone by the
|
||||
// time full teardown runs: drain the live-sync registry while the
|
||||
// backend function table can still release the backend handles (and
|
||||
@@ -49,6 +48,12 @@ namespace MobileGL {
|
||||
MG_State::pEGLContext.reset();
|
||||
MG_Impl::GLImpl::TextureImpl::pProxyTextureManager.reset();
|
||||
MG_Impl::GLImpl::FramebufferImpl::pDefaultFramebufferInfo.reset();
|
||||
// Must run AFTER pGLContext.reset(). FinalizeProcess -> ShFinalize deletes
|
||||
// glslang's process-wide pool allocator and every cached built-in symbol table,
|
||||
// while the TShader/TProgram objects owned by the shader and program objects
|
||||
// still reference levels adopted from those tables. Finalizing first left live
|
||||
// glslang objects pointing at freed memory for the rest of the teardown.
|
||||
glslang::FinalizeProcess();
|
||||
MG_Backend::gBackendFunctionsTable = {};
|
||||
g_isInitialized = false;
|
||||
if (logLifecycle) {
|
||||
|
||||
@@ -579,10 +579,11 @@ namespace MobileGL::MG_State::GLState {
|
||||
MGLOG_E("ProgramObject %u: Link failed - %s", m_externalIndex, m_infoLog.c_str());
|
||||
return;
|
||||
}
|
||||
// Deliberately no full-source dump here: a shaderpack stage runs to ~100 KB, and
|
||||
// one MGLOG line per shader per link is unreadable even single-threaded. Use the
|
||||
// transpiler dump paths when a specific source is actually needed.
|
||||
MGLOG_D("ProgramObject %u: shader[%zu] compiled shader ptr %p, src len %zu", m_externalIndex, i,
|
||||
shaders[i].get(), m_shaders[i]->GetShaderSource().length());
|
||||
MGLOG_D("ProgramObject %u: shader[%zu] source:\n%s", m_externalIndex, i,
|
||||
m_shaders[i]->GetShaderSource().c_str());
|
||||
}
|
||||
|
||||
// Merge the shaders' lexically extracted explicit uniform locations. The same
|
||||
|
||||
@@ -15,6 +15,8 @@
|
||||
#include <MG_Util/ShaderTranspiler/glslang/UniformTraverser.h>
|
||||
#include <MG_Backend/BackendObjects.h>
|
||||
|
||||
#include <charconv>
|
||||
|
||||
namespace {
|
||||
struct ComputeLocalSize {
|
||||
MobileGL::Uint x = 1;
|
||||
@@ -72,16 +74,31 @@ namespace {
|
||||
return result;
|
||||
}
|
||||
|
||||
// Hoisted out of ParseComputeLocalSize: constructing a std::regex costs far more than
|
||||
// running it over a small source, and it was being rebuilt on every compute compile. A
|
||||
// const regex carries no mutable state, so sharing one instance is safe.
|
||||
static const std::regex kComputeLocalSizePattern(R"(local_size_([xyz])\s*=\s*([0-9]+))");
|
||||
|
||||
static ComputeLocalSize ParseComputeLocalSize(const MobileGL::String& source) {
|
||||
ComputeLocalSize localSize;
|
||||
const MobileGL::String uncommentedSource = StripGlslComments(source);
|
||||
const std::regex localSizePattern(R"(local_size_([xyz])\s*=\s*([0-9]+))");
|
||||
|
||||
for (std::sregex_iterator it(uncommentedSource.begin(), uncommentedSource.end(), localSizePattern), end;
|
||||
for (std::sregex_iterator it(uncommentedSource.begin(), uncommentedSource.end(), kComputeLocalSizePattern),
|
||||
end;
|
||||
it != end; ++it) {
|
||||
const char axis = (*it)[1].str()[0];
|
||||
const auto value = static_cast<unsigned long long>(std::stoull((*it)[2].str()));
|
||||
const MobileGL::Uint clampedValue = value > UINT_MAX ? UINT_MAX : static_cast<MobileGL::Uint>(value);
|
||||
// The [0-9]+ capture is unbounded, so `local_size_x = 99999999999999999999999`
|
||||
// is a legal match. std::stoull would throw std::out_of_range on it and let the
|
||||
// exception escape glCompileShader; std::from_chars reports the overflow instead.
|
||||
// An overflowing literal saturates to UINT_MAX, which the device-limit check
|
||||
// below rejects anyway - the same verdict a non-overflowing huge value gets.
|
||||
const MobileGL::String digits = (*it)[2].str();
|
||||
unsigned long long value = 0;
|
||||
const std::from_chars_result parsed =
|
||||
std::from_chars(digits.data(), digits.data() + digits.size(), value);
|
||||
const MobileGL::Uint clampedValue = (parsed.ec != std::errc() || value > UINT_MAX)
|
||||
? UINT_MAX
|
||||
: static_cast<MobileGL::Uint>(value);
|
||||
|
||||
// TODO: Replace this literal layout scanner with parser/AST-backed validation so expressions and
|
||||
// specialization-id layouts are handled consistently with glslang.
|
||||
|
||||
@@ -347,6 +347,29 @@ void main() {
|
||||
EXPECT_EQ(GetError(), GL_NO_ERROR);
|
||||
}
|
||||
|
||||
TEST_F(ProgramTest, OutOfRangeComputeLocalSizeLiteralFailsCompileInsteadOfThrowing) {
|
||||
// The layout scanner's digit capture is unbounded, so a literal wider than 64 bits is a
|
||||
// legal match. It must saturate and be rejected through COMPILE_STATUS; if the integer
|
||||
// conversion throws instead, the exception escapes glCompileShader entirely.
|
||||
char infoLog[1024] = "";
|
||||
const char* csSrc = R"(#version 460 core
|
||||
layout(local_size_x = 99999999999999999999999) in;
|
||||
void main() {
|
||||
}
|
||||
)";
|
||||
|
||||
GLuint cs = CreateShader(GL_COMPUTE_SHADER);
|
||||
ShaderSource(cs, 1, &csSrc, nullptr);
|
||||
CompileShader(cs);
|
||||
|
||||
GLint csStatus = GL_TRUE;
|
||||
GetShaderiv(cs, GL_COMPILE_STATUS, &csStatus);
|
||||
EXPECT_EQ(csStatus, GL_FALSE);
|
||||
GetShaderInfoLog(cs, sizeof(infoLog), nullptr, infoLog);
|
||||
EXPECT_NE(String(infoLog).find("GL_MAX_COMPUTE_WORK_GROUP_SIZE"), String::npos) << infoLog;
|
||||
EXPECT_EQ(GetError(), GL_NO_ERROR);
|
||||
}
|
||||
|
||||
TEST_F(ProgramTest, DirectVulkanStorageBlockUsesShaderLayoutBinding) {
|
||||
char infoLog[1024] = "";
|
||||
const char* csSrc = R"(#version 460 core
|
||||
|
||||
@@ -138,6 +138,9 @@ namespace MobileGL {
|
||||
std::swap(this->resources, that.resources);
|
||||
std::swap(this->reflectModule, that.reflectModule);
|
||||
std::swap(this->reflectModuleValid, that.reflectModuleValid);
|
||||
// ParseMetaData() fills `metadata`, and GetMetadata() is read through the
|
||||
// moved-to session: leaving it behind silently returns an empty reflection.
|
||||
std::swap(this->metadata, that.metadata);
|
||||
}
|
||||
|
||||
SpvcSession& SpvcSession::operator=(SpvcSession&& that) {
|
||||
@@ -149,6 +152,7 @@ namespace MobileGL {
|
||||
std::swap(this->resources, that.resources);
|
||||
std::swap(this->reflectModule, that.reflectModule);
|
||||
std::swap(this->reflectModuleValid, that.reflectModuleValid);
|
||||
std::swap(this->metadata, that.metadata);
|
||||
return *this;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user