Commit 3f48ecb3 by Ben Clayton

GLES: Fix OOB access of labelBlock.

This was being incorrectly sized by the number of functions, not the highest label in use. While investigating this, I've realized that the sanity checks to ensure there are no dead functions was never going to fire as the function list was built from the list of called functions. Instead I've changed the function scanning pass to look for labels starting a LABEL-RET pair. Bug: b/125183107 Change-Id: Ic921097ed42a96b52f1ab7c9590c02fb3552b565 Reviewed-on: https://swiftshader-review.googlesource.com/c/25168Tested-by: 's avatarBen Clayton <bclayton@google.com> Kokoro-Presubmit: kokoro <noreply+kokoro@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent f56c4506
...@@ -37,7 +37,7 @@ namespace sw ...@@ -37,7 +37,7 @@ namespace sw
ifFalseBlock.resize(limits.ifs); ifFalseBlock.resize(limits.ifs);
loopRepTestBlock.resize(limits.loops); loopRepTestBlock.resize(limits.loops);
loopRepEndBlock.resize(limits.loops); loopRepEndBlock.resize(limits.loops);
labelBlock.resize(limits.functions); labelBlock.resize(limits.maxLabel + 1);
isConditionalIf.resize(limits.ifs); isConditionalIf.resize(limits.ifs);
loopDepth = -1; loopDepth = -1;
......
...@@ -1957,32 +1957,47 @@ namespace sw ...@@ -1957,32 +1957,47 @@ namespace sw
bool reachable; // Is this function reachable? bool reachable; // Is this function reachable?
}; };
// Begin by doing a pass over the instructions to identify all the
// labels that denote the start of a function.
std::unordered_map<FunctionID, FunctionInfo> functions; std::unordered_map<FunctionID, FunctionInfo> functions;
for(auto &inst : instruction) uint32_t maxLabel = 0; // Highest label found
{
if (inst->isCall())
{
FunctionID id = inst->dst.label;
ASSERT(id != MAIN_ID); // If this fires, we're going to have to represent main with something else.
functions[id] = FunctionInfo();
}
}
// Add a definition for the main entry point. // Add a definition for the main entry point.
// This starts at the beginning of the instructions and does not have // This starts at the beginning of the instructions and does not have
// its own label. // its own label.
functions[MAIN_ID] = FunctionInfo(); functions[MAIN_ID] = FunctionInfo();
functions[MAIN_ID].reachable = true; functions[MAIN_ID].reachable = true;
// Begin by doing a pass over the instructions to identify all the
// functions. These start with a label and end with a ret. Note that
// functions can have labels within them.
FunctionID currentFunc = MAIN_ID; FunctionID currentFunc = MAIN_ID;
for(auto &inst : instruction)
{
switch (inst->opcode)
{
case OPCODE_LABEL:
if (currentFunc == INVALID_ID)
{
// Start of a function.
FunctionID id = inst->dst.label;
ASSERT(id != MAIN_ID); // If this fires, we're going to have to represent main with something else.
functions[id] = FunctionInfo();
}
break;
case OPCODE_RET:
currentFunc = INVALID_ID;
break;
default:
break;
}
}
// Limits for the currently analyzed function. // Limits for the currently analyzed function.
FunctionLimits currentLimits; FunctionLimits currentLimits;
// Now loop over the instructions gathering the limits of each of the // Now loop over the instructions gathering the limits of each of the
// functions. // functions.
currentFunc = MAIN_ID;
for(size_t i = 0; i < instruction.size(); i++) for(size_t i = 0; i < instruction.size(); i++)
{ {
const auto& inst = instruction[i]; const auto& inst = instruction[i];
...@@ -1990,15 +2005,14 @@ namespace sw ...@@ -1990,15 +2005,14 @@ namespace sw
{ {
case OPCODE_LABEL: case OPCODE_LABEL:
{ {
FunctionID id = inst->dst.label; maxLabel = std::max(maxLabel, inst->dst.label);
auto it = functions.find(id); if (currentFunc == INVALID_ID)
if (it == functions.end())
{ {
// Label does not start a new function. // Start of a function.
continue; FunctionID id = inst->dst.label;
ASSERT(functions.find(id) != functions.end()); // Sanity check
currentFunc = id;
} }
ASSERT(currentFunc == INVALID_ID); // Functions overlap
currentFunc = id;
break; break;
} }
case OPCODE_CALL: case OPCODE_CALL:
...@@ -2006,6 +2020,7 @@ namespace sw ...@@ -2006,6 +2020,7 @@ namespace sw
{ {
ASSERT(currentFunc != INVALID_ID); ASSERT(currentFunc != INVALID_ID);
FunctionID id = inst->dst.label; FunctionID id = inst->dst.label;
ASSERT(functions.find(id) != functions.end());
functions[currentFunc].calls.emplace(id); functions[currentFunc].calls.emplace(id);
functions[id].reachable = true; functions[id].reachable = true;
break; break;
...@@ -2067,6 +2082,9 @@ namespace sw ...@@ -2067,6 +2082,9 @@ namespace sw
// stripped in earlier stages). Unreachable functions may be code // stripped in earlier stages). Unreachable functions may be code
// generated, but their own limits are not considered below, potentially // generated, but their own limits are not considered below, potentially
// causing OOB indexing in later stages. // causing OOB indexing in later stages.
// If we ever find cases where there are unreachable functions, we can
// replace this assert with NO-OPing or stripping out the dead
// functions.
for (auto it : functions) { ASSERT(it.second.reachable); } for (auto it : functions) { ASSERT(it.second.reachable); }
// We have now gathered all the information about each of the functions // We have now gathered all the information about each of the functions
...@@ -2097,6 +2115,6 @@ namespace sw ...@@ -2097,6 +2115,6 @@ namespace sw
}; };
limits = traverse(MAIN_ID); limits = traverse(MAIN_ID);
limits.functions = (uint32_t)functions.size(); limits.maxLabel = maxLabel;
} }
} }
...@@ -561,7 +561,7 @@ namespace sw ...@@ -561,7 +561,7 @@ namespace sw
uint32_t loops = 0; // maximum nested loop and reps. uint32_t loops = 0; // maximum nested loop and reps.
uint32_t ifs = 0; // maximum nested if statements. uint32_t ifs = 0; // maximum nested if statements.
uint32_t stack = 0; // maximum call depth. uint32_t stack = 0; // maximum call depth.
uint32_t functions = 0; // total number of functions. uint32_t maxLabel = 0; // highest label in use.
}; };
Shader(); Shader();
......
...@@ -36,7 +36,7 @@ namespace sw ...@@ -36,7 +36,7 @@ namespace sw
ifFalseBlock.resize(limits.ifs); ifFalseBlock.resize(limits.ifs);
loopRepTestBlock.resize(limits.loops); loopRepTestBlock.resize(limits.loops);
loopRepEndBlock.resize(limits.loops); loopRepEndBlock.resize(limits.loops);
labelBlock.resize(limits.functions); labelBlock.resize(limits.maxLabel + 1);
isConditionalIf.resize(limits.ifs); isConditionalIf.resize(limits.ifs);
loopDepth = -1; loopDepth = -1;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment