Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(623)

Unified Diff: src/PNaClTranslator.cpp

Issue 1282523002: Fix processing of local variable indices in fuction blocks. (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Patch Set: Fix nits Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/PNaClTranslator.cpp
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index 1eb1767dbf195f0c9afb9a9ccdb218d13665b708..fe0285a0b1655816acbb9db240af3aff294c1f39 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -1306,12 +1306,6 @@ public:
return Context->getGlobalConstantByID(Index);
}
NaClBcIndexSize_t LocalIndex = Index - CachedNumGlobalValueIDs;
- if (LocalIndex >= LocalOperands.size()) {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Value index " << Index << " not defined!";
- Fatal(StrBuf.str());
- }
Ice::Operand *Op = LocalOperands[LocalIndex];
if (Op == nullptr) {
if (isIRGenerationDisabled())
@@ -1334,6 +1328,7 @@ public:
}
private:
+ typedef std::unordered_map<NaClBcIndexSize_t, Ice::Operand *> OperandMap;
typedef std::unordered_map<NaClBcIndexSize_t, Ice::CfgNode *> CfgNodeMap;
Ice::TimerMarker Timer;
@@ -1356,7 +1351,7 @@ private:
size_t CachedNumGlobalValueIDs;
// Holds operands local to the function block, based on indices
// defined in the bitcode file.
- std::vector<Ice::Operand *> LocalOperands;
+ OperandMap LocalOperands;
// Holds the index within LocalOperands corresponding to the next
// instruction that generates a value.
NaClBcIndexSize_t NextLocalInstIndex;
@@ -1392,6 +1387,8 @@ private:
bool verifyAndRenameBasicBlocks();
+ bool verifyAllForwardRefsDefined();
+
// Returns the Index-th basic block in the list of basic blocks.
// Assumes Index corresponds to a branch instruction. Hence, if
// the branch references the entry block, it also generates a
@@ -1467,34 +1464,26 @@ private:
assert(Op || isIRGenerationDisabled());
// Check if simple push works.
NaClBcIndexSize_t LocalIndex = Index - CachedNumGlobalValueIDs;
- if (LocalIndex == LocalOperands.size()) {
- LocalOperands.push_back(Op);
- return;
- }
-
- // Must be forward reference, expand vector to accommodate.
- if (LocalIndex >= LocalOperands.size())
- LocalOperands.resize(LocalIndex + 1);
// If element not defined, set it.
- Ice::Operand *OldOp = LocalOperands[LocalIndex];
- if (OldOp == nullptr) {
- LocalOperands[LocalIndex] = Op;
+ Ice::Operand *&IndexedOp = LocalOperands[LocalIndex];
+ if (IndexedOp == nullptr) {
+ IndexedOp = Op;
return;
}
- // See if forward reference matches.
- if (OldOp == Op)
+ // See if forward reference matchers.
+ if (IndexedOp == Op)
return;
// Error has occurred.
std::string Buffer;
raw_string_ostream StrBuf(Buffer);
StrBuf << "Multiple definitions for index " << Index << ": " << *Op
- << " and " << *OldOp;
+ << " and " << *IndexedOp;
Error(StrBuf.str());
// TODO(kschimpf) Remove error recovery once implementation complete.
- LocalOperands[LocalIndex] = Op;
+ IndexedOp = Op;
}
// Returns the relative operand (wrt to BaseIndex) referenced by
@@ -1998,6 +1987,26 @@ private:
}
};
+bool FunctionParser::verifyAllForwardRefsDefined() {
+ NaClBcIndexSize_t NumInstructions =
+ NextLocalInstIndex - CachedNumGlobalValueIDs;
+ if (NumInstructions == LocalOperands.size())
+ return true;
+ // Find undefined forward references and report.
+ std::vector<NaClBcIndexSize_t> UndefinedFwdRefs;
+ for (const OperandMap::value_type &Elmt : LocalOperands)
+ if (Elmt.first >= NextLocalInstIndex)
+ UndefinedFwdRefs.push_back(Elmt.first);
+ std::sort(UndefinedFwdRefs.begin(), UndefinedFwdRefs.end());
+ for (const NaClBcIndexSize_t Index : UndefinedFwdRefs) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Instruction forward reference not defined: " << Index;
+ Error(StrBuf.str());
+ }
+ return false;
+}
+
bool FunctionParser::verifyAndRenameBasicBlocks() {
const size_t NumFoundBbs = BbMap.size();
// Verify number of basic blocks found match amount specified in function.
@@ -2047,6 +2056,8 @@ void FunctionParser::ExitBlock() {
}
if (isIRGenerationDisabled())
return;
+ if (!verifyAllForwardRefsDefined())
+ return;
if (!verifyAndRenameBasicBlocks())
return;
// Before translating, check for blocks without instructions, and
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698