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

Unified Diff: src/PNaClTranslator.cpp

Issue 1347683003: Check that symbol names in symbol tables are unique. (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Patch Set: Fix nit. Created 5 years, 3 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 | tests_lit/parse_errs/Inputs/duplicate-fcn-name.tbc » ('j') | 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 d3c71fd4091cb737d4bbe45f4347fac86b0cfba6..476bdd25fbd9ca28a40227093187d77b81d4f107 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -27,6 +27,7 @@
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunused-parameter"
+#include "llvm/ADT/Hashing.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Bitcode/NaCl/NaClBitcodeDecoders.h"
#include "llvm/Bitcode/NaCl/NaClBitcodeDefs.h"
@@ -36,8 +37,19 @@
#include "llvm/Support/Format.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/raw_ostream.h"
+#include <unordered_set>
#pragma clang diagnostic pop
+// Define a hash function for SmallString's, so that it can be used in hash
+// tables.
+namespace std {
+template <unsigned InternalLen> struct hash<llvm::SmallString<InternalLen>> {
+ size_t operator()(const llvm::SmallString<InternalLen> &Key) const {
+ return llvm::hash_combine_range(Key.begin(), Key.end());
+ }
+};
+} // end of namespace std
+
namespace {
using namespace llvm;
@@ -1146,23 +1158,60 @@ public:
protected:
using StringType = SmallString<128>;
+ // Returns the name to identify the kind of symbol table this is
+ // in error messages.
+ virtual const char *getTableKind() const = 0;
+
// Associates Name with the value defined by the given Index.
virtual void setValueName(NaClBcIndexSize_t Index, StringType &Name) = 0;
// Associates Name with the value defined by the given Index;
virtual void setBbName(NaClBcIndexSize_t Index, StringType &Name) = 0;
+ // Reports that the assignment of Name to the value associated with
+ // index is not possible, for the given Context.
+ void reportUnableToAssign(const char *Context, NaClBcIndexSize_t Index,
+ StringType &Name);
+
private:
+ using NamesSetType = std::unordered_set<StringType>;
+ NamesSetType ValueNames;
+ NamesSetType BlockNames;
+
void ProcessRecord() override;
- void convertToString(StringType &ConvertedName) {
+ // Extracts out ConvertedName. Returns true if unique wrt to Names.
+ bool convertToString(NamesSetType &Names, StringType &ConvertedName) {
const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
for (size_t i = 1, e = Values.size(); i != e; ++i) {
ConvertedName += static_cast<char>(Values[i]);
}
+ auto Pair = Names.insert(ConvertedName);
+ return Pair.second;
}
+
+ void ReportDuplicateName(const char *NameCat, StringType &Name);
};
+void ValuesymtabParser::reportUnableToAssign(const char *Context,
+ NaClBcIndexSize_t Index,
+ StringType &Name) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << getTableKind() << " " << getBlockName() << ": " << Context
+ << " name '" << Name << "' can't be associated with index " << Index;
+ Error(StrBuf.str());
+}
+
+void ValuesymtabParser::ReportDuplicateName(const char *NameCat,
+ StringType &Name) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << getTableKind() << " " << getBlockName() << " defines duplicate "
+ << NameCat << " name: '" << Name << "'";
+ Error(StrBuf.str());
+}
+
void ValuesymtabParser::ProcessRecord() {
const NaClBitcodeRecord::RecordVector &Values = Record.GetValues();
StringType ConvertedName;
@@ -1171,16 +1220,20 @@ void ValuesymtabParser::ProcessRecord() {
// VST_ENTRY: [ValueId, namechar x N]
if (!isValidRecordSizeAtLeast(2, "value entry"))
return;
- convertToString(ConvertedName);
- setValueName(Values[0], ConvertedName);
+ if (convertToString(ValueNames, ConvertedName))
+ setValueName(Values[0], ConvertedName);
+ else
+ ReportDuplicateName("value", ConvertedName);
return;
}
case naclbitc::VST_CODE_BBENTRY: {
// VST_BBENTRY: [BbId, namechar x N]
if (!isValidRecordSizeAtLeast(2, "basic block entry"))
return;
- convertToString(ConvertedName);
- setBbName(Values[0], ConvertedName);
+ if (convertToString(BlockNames, ConvertedName))
+ setBbName(Values[0], ConvertedName);
+ else
+ ReportDuplicateName("block", ConvertedName);
return;
}
default:
@@ -2864,8 +2917,10 @@ private:
return reinterpret_cast<FunctionParser *>(GetEnclosingParser());
}
- void setValueName(NaClBcIndexSize_t Index, StringType &Name) override;
- void setBbName(NaClBcIndexSize_t Index, StringType &Name) override;
+ const char *getTableKind() const final { return "Function"; }
+
+ void setValueName(NaClBcIndexSize_t Index, StringType &Name) final;
+ void setBbName(NaClBcIndexSize_t Index, StringType &Name) final;
// Reports that the assignment of Name to the value associated with index is
// not possible, for the given Context.
@@ -2884,7 +2939,7 @@ void FunctionValuesymtabParser::setValueName(NaClBcIndexSize_t Index,
// Note: We check when Index is too small, so that we can error recover
// (FP->getOperand will create fatal error).
if (Index < getFunctionParser()->getNumGlobalIDs()) {
- reportUnableToAssign("instruction", Index, Name);
+ reportUnableToAssign("Global value", Index, Name);
return;
}
if (isIRGenerationDisabled())
@@ -2896,7 +2951,7 @@ void FunctionValuesymtabParser::setValueName(NaClBcIndexSize_t Index,
V->setName(getFunctionParser()->getFunc(), Nm);
}
} else {
- reportUnableToAssign("variable", Index, Name);
+ reportUnableToAssign("Local value", Index, Name);
}
}
@@ -2907,7 +2962,7 @@ void FunctionValuesymtabParser::setBbName(NaClBcIndexSize_t Index,
if (isIRGenerationDisabled())
return;
if (Index >= getFunctionParser()->getFunc()->getNumNodes()) {
- reportUnableToAssign("block", Index, Name);
+ reportUnableToAssign("Basic block", Index, Name);
return;
}
std::string Nm(Name.data(), Name.size());
@@ -2990,8 +3045,9 @@ public:
private:
Ice::TimerMarker Timer;
- void setValueName(NaClBcIndexSize_t Index, StringType &Name) override;
- void setBbName(NaClBcIndexSize_t Index, StringType &Name) override;
+ const char *getTableKind() const final { return "Module"; }
+ void setValueName(NaClBcIndexSize_t Index, StringType &Name) final;
+ void setBbName(NaClBcIndexSize_t Index, StringType &Name) final;
};
void ModuleValuesymtabParser::setValueName(NaClBcIndexSize_t Index,
@@ -3002,11 +3058,7 @@ void ModuleValuesymtabParser::setValueName(NaClBcIndexSize_t Index,
void ModuleValuesymtabParser::setBbName(NaClBcIndexSize_t Index,
StringType &Name) {
- std::string Buffer;
- raw_string_ostream StrBuf(Buffer);
- StrBuf << "Can't define basic block name at global level: '" << Name
- << "' -> " << Index;
- Error(StrBuf.str());
+ reportUnableToAssign("Basic block", Index, Name);
}
bool ModuleParser::ParseBlock(unsigned BlockID) {
« no previous file with comments | « no previous file | tests_lit/parse_errs/Inputs/duplicate-fcn-name.tbc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698