Skip to content

S
swiftshader
Commit 52863b13 by Karl Schimpf
Options

Check that symbol names in symbol tables are unique.

Makes sure that names within a symbol table are unique. Also cleans up error reporting to be more consistent between symbol tables. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4301 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1347683003 .
parent 9d25e620
Showing with 134 additions and 17 deletions
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) {
......
tests_lit/parse_errs/Inputs/duplicate-fcn-name.tbc 0 → 100644
65535,8,2;
1,1;
65535,17,2;
1,2;
2;
21,0,0;
65534;
8,1,0,0,0;
8,1,0,0,0;
65535,19,2;
5,0;
65534;
65535,14,2;
1,0,102;
1,1,102;
65534;
65535,12,2;
1,1;
10;
65534;
65535,12,2;
1,1;
10;
65534;
65534;
tests_lit/parse_errs/duplicate-fcn-name.test 0 → 100644
; Test if we detect duplicate names in a symbol table.
; REQUIRES: no_minimal_build
; RUN: not %pnacl_sz -bitcode-as-text %p/Inputs/duplicate-fcn-name.tbc \
; RUN: -bitcode-format=pnacl -notranslate -no-ir-gen -build-on-read 2>&1 \
; RUN: | FileCheck %s
; CHECK: Module valuesymtab defines duplicate value name: 'f'
; RUN: pnacl-bcfuzz -bitcode-as-text %p/Inputs/duplicate-fcn-name.tbc -output - \
; RUN: | pnacl-bcdis -no-records | FileCheck -check-prefix=ASM %s
; ASM: module { // BlockID = 8
; ASM: version 1;
; ASM: types { // BlockID = 17
; ASM: count 2;
; ASM: @t0 = void;
; ASM: @t1 = void ();
; ASM: }
; ASM: define external void @f0();
; ASM: define external void @f1();
; ASM: globals { // BlockID = 19
; ASM: count 0;
; ASM: }
; ASM: valuesymtab { // BlockID = 14
; ASM: @f0 : "f";
; ASM: @f1 : "f";
; ASM: }
; ASM: function void @f0() { // BlockID = 12
; ASM: blocks 1;
; ASM: %b0:
; ASM: ret void;
; ASM: }
; ASM: function void @f1() { // BlockID = 12
; ASM: blocks 1;
; ASM: %b0:
; ASM: ret void;
; ASM: }
; ASM: }
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