gl_shader_decompiler: Guard out of bound geometry shader input reads

Geometry shaders follow a pattern that results in out of bound reads.
This pattern is:
- VSETP to predicate
- Use that predicate to conditionally set a register a big number
- Use the register to access geometry shaders
At the time of writing this commit I don't know what's the intent of
this number. Some drivers argue about these out of bound reads. To avoid
this issue, input reads are guarded limiting reads to the highest
posible vertex input of the current topology (e.g. points to 1 and
triangles to 3).
This commit is contained in:
ReinUsesLisp 2018-11-10 02:41:33 -03:00
parent 29f082775b
commit 8d4bb10d44
4 changed files with 24 additions and 15 deletions

View file

@ -121,12 +121,16 @@ GLint CachedShader::GetUniformLocation(const GLShader::SamplerEntry& sampler) {
} }
GLuint CachedShader::LazyGeometryProgram(OGLProgram& target_program, GLuint CachedShader::LazyGeometryProgram(OGLProgram& target_program,
const std::string& glsl_topology, const std::string& glsl_topology, u32 max_vertices,
const std::string& debug_name) { const std::string& debug_name) {
if (target_program.handle != 0) { if (target_program.handle != 0) {
return target_program.handle; return target_program.handle;
} }
const std::string source{geometry_programs.code + "layout (" + glsl_topology + ") in;\n"}; std::string source = "#version 430 core\n";
source += "layout (" + glsl_topology + ") in;\n";
source += "#define MAX_VERTEX_INPUT " + std::to_string(max_vertices) + '\n';
source += geometry_programs.code;
OGLShader shader; OGLShader shader;
shader.Create(source.c_str(), GL_GEOMETRY_SHADER); shader.Create(source.c_str(), GL_GEOMETRY_SHADER);
target_program.Create(true, shader.handle); target_program.Create(true, shader.handle);

View file

@ -46,22 +46,23 @@ public:
} }
switch (primitive_mode) { switch (primitive_mode) {
case GL_POINTS: case GL_POINTS:
return LazyGeometryProgram(geometry_programs.points, "points", "ShaderPoints"); return LazyGeometryProgram(geometry_programs.points, "points", 1, "ShaderPoints");
case GL_LINES: case GL_LINES:
case GL_LINE_STRIP: case GL_LINE_STRIP:
return LazyGeometryProgram(geometry_programs.lines, "lines", "ShaderLines"); return LazyGeometryProgram(geometry_programs.lines, "lines", 2, "ShaderLines");
case GL_LINES_ADJACENCY: case GL_LINES_ADJACENCY:
case GL_LINE_STRIP_ADJACENCY: case GL_LINE_STRIP_ADJACENCY:
return LazyGeometryProgram(geometry_programs.lines_adjacency, "lines_adjacency", return LazyGeometryProgram(geometry_programs.lines_adjacency, "lines_adjacency", 4,
"ShaderLinesAdjacency"); "ShaderLinesAdjacency");
case GL_TRIANGLES: case GL_TRIANGLES:
case GL_TRIANGLE_STRIP: case GL_TRIANGLE_STRIP:
case GL_TRIANGLE_FAN: case GL_TRIANGLE_FAN:
return LazyGeometryProgram(geometry_programs.triangles, "triangles", "ShaderTriangles"); return LazyGeometryProgram(geometry_programs.triangles, "triangles", 3,
"ShaderTriangles");
case GL_TRIANGLES_ADJACENCY: case GL_TRIANGLES_ADJACENCY:
case GL_TRIANGLE_STRIP_ADJACENCY: case GL_TRIANGLE_STRIP_ADJACENCY:
return LazyGeometryProgram(geometry_programs.triangles_adjacency, "triangles_adjacency", return LazyGeometryProgram(geometry_programs.triangles_adjacency, "triangles_adjacency",
"ShaderLines"); 6, "ShaderTrianglesAdjacency");
default: default:
UNREACHABLE_MSG("Unknown primitive mode."); UNREACHABLE_MSG("Unknown primitive mode.");
} }
@ -76,7 +77,7 @@ public:
private: private:
/// Generates a geometry shader or returns one that already exists. /// Generates a geometry shader or returns one that already exists.
GLuint LazyGeometryProgram(OGLProgram& target_program, const std::string& glsl_topology, GLuint LazyGeometryProgram(OGLProgram& target_program, const std::string& glsl_topology,
const std::string& debug_name); u32 max_vertices, const std::string& debug_name);
VAddr addr; VAddr addr;
Maxwell::ShaderProgram program_type; Maxwell::ShaderProgram program_type;

View file

@ -494,10 +494,10 @@ public:
// instruction for now. // instruction for now.
if (stage == Maxwell3D::Regs::ShaderStage::Geometry) { if (stage == Maxwell3D::Regs::ShaderStage::Geometry) {
// TODO(Rodrigo): nouveau sets some attributes after setting emitting a geometry // TODO(Rodrigo): nouveau sets some attributes after setting emitting a geometry
// shader. These instructions use a dirty register as buffer index. To avoid some // shader. These instructions use a dirty register as buffer index, to avoid some
// drivers from complaining for the out of boundary writes, guard them. // drivers from complaining about out of boundary writes, guard them.
const std::string buf_index{"min(" + GetRegisterAsInteger(buf_reg) + ", " + const std::string buf_index{"((" + GetRegisterAsInteger(buf_reg) + ") % " +
std::to_string(MAX_GEOMETRY_BUFFERS - 1) + ')'}; std::to_string(MAX_GEOMETRY_BUFFERS) + ')'};
shader.AddLine("amem[" + buf_index + "][" + shader.AddLine("amem[" + buf_index + "][" +
std::to_string(static_cast<u32>(attribute)) + ']' + std::to_string(static_cast<u32>(attribute)) + ']' +
GetSwizzle(elem) + " = " + src + ';'); GetSwizzle(elem) + " = " + src + ';');
@ -811,7 +811,11 @@ private:
std::optional<Register> vertex = {}) { std::optional<Register> vertex = {}) {
auto GeometryPass = [&](const std::string& name) { auto GeometryPass = [&](const std::string& name) {
if (stage == Maxwell3D::Regs::ShaderStage::Geometry && vertex) { if (stage == Maxwell3D::Regs::ShaderStage::Geometry && vertex) {
return "gs_" + name + '[' + GetRegisterAsInteger(*vertex, 0, false) + ']'; // TODO(Rodrigo): Guard geometry inputs against out of bound reads. Some games set
// an 0x80000000 index for those and the shader fails to build. Find out why this
// happens and what's its intent.
return "gs_" + name + '[' + GetRegisterAsInteger(*vertex, 0, false) +
" % MAX_VERTEX_INPUT]";
} }
return name; return name;
}; };

View file

@ -82,8 +82,8 @@ void main() {
} }
ProgramResult GenerateGeometryShader(const ShaderSetup& setup) { ProgramResult GenerateGeometryShader(const ShaderSetup& setup) {
std::string out = "#version 430 core\n"; // Version is intentionally skipped in shader generation, it's added by the lazy compilation.
out += "#extension GL_ARB_separate_shader_objects : enable\n\n"; std::string out = "#extension GL_ARB_separate_shader_objects : enable\n\n";
out += Decompiler::GetCommonDeclarations(); out += Decompiler::GetCommonDeclarations();
out += "bool exec_geometry();\n"; out += "bool exec_geometry();\n";