Make sure that all globals are internal, except for "start" functions.
The existing code, when run on a fuzzed example, generates a runtime
assertion. The reason for this is that the input defines "memmove" as
an external global. However, the code generator can generate calls to
"memmove" which assumes it is internal (see PNaCl ABI). As a result,
the assertion that checks that global names are unique (for memmove)
fails.
This code fixes the problem by checking that global names are
internal, unless they are one of the "start" functions,
or the function is an intrinsic. To allow for
non-PNaCl ABI input, a flag was added to allow functions to be
external. However, in such cases the external can't be one of
Subzero's runtime helper functions.
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4330
R=jpp@chromium.org, stichnot@chromium.org
Review URL: https://codereview.chromium.org/1387963002 .
diff --git a/src/IceClFlags.cpp b/src/IceClFlags.cpp
index a27fb3f..5ed9ea0 100644
--- a/src/IceClFlags.cpp
+++ b/src/IceClFlags.cpp
@@ -34,6 +34,12 @@
cl::desc("Allow error recovery when reading PNaCl bitcode."),
cl::init(false));
+cl::opt<bool> AllowExternDefinedSymbols(
+ "allow-externally-defined-symbols",
+ cl::desc("Allow global symbols to be externally defined (other than _start "
+ "and __pnacl_pso_root)."),
+ cl::init(false));
+
cl::opt<bool> AllowIacaMarks(
"allow-iaca-marks",
cl::desc("Allow IACA (Intel Architecture Code Analyzer) marks to be "
@@ -360,6 +366,7 @@
void ClFlags::resetClFlags(ClFlags &OutFlags) {
// bool fields
OutFlags.AllowErrorRecovery = false;
+ OutFlags.AllowExternDefinedSymbols = false;
OutFlags.AllowIacaMarks = false;
OutFlags.AllowUninitializedGlobals = false;
OutFlags.DataSections = false;
@@ -420,6 +427,8 @@
}
OutFlags.setAllowErrorRecovery(::AllowErrorRecovery);
+ OutFlags.setAllowExternDefinedSymbols(::AllowExternDefinedSymbols ||
+ ::DisableInternal);
OutFlags.setAllowIacaMarks(::AllowIacaMarks);
OutFlags.setAllowUninitializedGlobals(::AllowUninitializedGlobals);
OutFlags.setDataSections(::DataSections);
diff --git a/src/IceClFlags.h b/src/IceClFlags.h
index 87e16cd..0aba214 100644
--- a/src/IceClFlags.h
+++ b/src/IceClFlags.h
@@ -39,6 +39,13 @@
bool getAllowErrorRecovery() const { return AllowErrorRecovery; }
void setAllowErrorRecovery(bool NewValue) { AllowErrorRecovery = NewValue; }
+ bool getAllowExternDefinedSymbols() const {
+ return AllowExternDefinedSymbols;
+ }
+ void setAllowExternDefinedSymbols(bool NewValue) {
+ AllowExternDefinedSymbols = NewValue;
+ }
+
bool getAllowIacaMarks() const { return AllowIacaMarks; }
void setAllowIacaMarks(bool NewValue) { AllowIacaMarks = NewValue; }
@@ -238,6 +245,7 @@
private:
bool AllowErrorRecovery;
+ bool AllowExternDefinedSymbols;
bool AllowIacaMarks;
bool AllowUninitializedGlobals;
bool DataSections;
diff --git a/src/IceConverter.cpp b/src/IceConverter.cpp
index a4d4f53..349fde5 100644
--- a/src/IceConverter.cpp
+++ b/src/IceConverter.cpp
@@ -867,6 +867,13 @@
FunctionDeclaration *IceFunc = FunctionDeclaration::create(
Ctx, Signature, Func.getCallingConv(), Func.getLinkage(), Func.empty());
IceFunc->setName(Func.getName());
+ if (!IceFunc->verifyLinkageCorrect(Ctx)) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Function " << IceFunc->getName()
+ << " has incorrect linkage: " << IceFunc->getLinkageName();
+ report_fatal_error(StrBuf.str());
+ }
GlobalDeclarationMap[&Func] = IceFunc;
}
// Install global variable declarations.
@@ -879,6 +886,13 @@
Var->setAlignment(GV->getAlignment());
Var->setIsConstant(GV->isConstant());
Var->setLinkage(GV->getLinkage());
+ if (!Var->verifyLinkageCorrect(Ctx)) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Global " << Var->getName()
+ << " has incorrect linkage: " << Var->getLinkageName();
+ report_fatal_error(StrBuf.str());
+ }
GlobalDeclarationMap[GV] = Var;
}
}
diff --git a/src/IceELFSection.cpp b/src/IceELFSection.cpp
index 3e33c99..3768150 100644
--- a/src/IceELFSection.cpp
+++ b/src/IceELFSection.cpp
@@ -116,7 +116,12 @@
NewSymbol.Section = NullSection;
NewSymbol.Number = ELFSym::UnknownNumber;
bool Unique = GlobalSymbols.insert(std::make_pair(Name, NewSymbol)).second;
- assert(Unique);
+ if (!Unique) {
+ std::string Buffer;
+ llvm::raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Symbol external and defined: " << Name;
+ llvm::report_fatal_error(StrBuf.str());
+ }
(void)Unique;
}
diff --git a/src/IceGlobalInits.h b/src/IceGlobalInits.h
index 8f51db2..82e5cde 100644
--- a/src/IceGlobalInits.h
+++ b/src/IceGlobalInits.h
@@ -87,11 +87,29 @@
return getSuppressMangling() ? Name : Ctx->mangleName(Name);
}
+ /// Returns textual name of linkage.
+ const char *getLinkageName() const {
+ return isInternal() ? "internal" : "external";
+ }
+
protected:
GlobalDeclaration(GlobalDeclarationKind Kind,
llvm::GlobalValue::LinkageTypes Linkage)
: Kind(Kind), Linkage(Linkage) {}
+ /// Returns true if linkage is defined correctly for the global declaration,
+ /// based on default rules.
+ bool verifyLinkageDefault(const GlobalContext *Ctx) const {
+ switch (Linkage) {
+ default:
+ return false;
+ case llvm::GlobalValue::InternalLinkage:
+ return true;
+ case llvm::GlobalValue::ExternalLinkage:
+ return Ctx->getFlags().getAllowExternDefinedSymbols();
+ }
+ }
+
const GlobalDeclarationKind Kind;
IceString Name;
llvm::GlobalValue::LinkageTypes Linkage;
@@ -124,6 +142,13 @@
void dump(GlobalContext *Ctx, Ostream &Stream) const final;
bool getSuppressMangling() const final { return isExternal() && IsProto; }
+ /// Returns true if linkage is correct for the function declaration.
+ bool verifyLinkageCorrect(const GlobalContext *Ctx) const {
+ if (isPNaClABIExternalName() || isIntrinsicName(Ctx))
+ return Linkage == llvm::GlobalValue::ExternalLinkage;
+ return verifyLinkageDefault(Ctx);
+ }
+
private:
const Ice::FuncSigType Signature;
llvm::CallingConv::ID CallingConv;
@@ -134,6 +159,19 @@
llvm::GlobalValue::LinkageTypes Linkage, bool IsProto)
: GlobalDeclaration(FunctionDeclarationKind, Linkage),
Signature(Signature), CallingConv(CallingConv), IsProto(IsProto) {}
+
+ bool isPNaClABIExternalName() const {
+ const char *Name = getName().c_str();
+ return strcmp(Name, "_start") == 0 || strcmp(Name, "__pnacl_pso_root") == 0;
+ }
+
+ bool isIntrinsicName(const GlobalContext *Ctx) const {
+ if (!hasName())
+ return false;
+ bool BadIntrinsic;
+ return Ctx->getIntrinsicsInfo().find(getName(), BadIntrinsic) &&
+ !BadIntrinsic;
+ }
};
/// Models a global variable declaration, and its initializers.
@@ -309,6 +347,11 @@
/// initialization).
void dump(GlobalContext *Ctx, Ostream &Stream) const final;
+ /// Returns true if linkage is correct for the variable declaration.
+ bool verifyLinkageCorrect(const GlobalContext *Ctx) const {
+ return verifyLinkageDefault(Ctx);
+ }
+
static bool classof(const GlobalDeclaration *Addr) {
return Addr->getKind() == VariableDeclarationKind;
}
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index c1a5919..7124da0 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -475,7 +475,16 @@
// Converts function declarations into constant value IDs.
void createValueIDsForFunctions() {
+ Ice::GlobalContext *Ctx = getTranslator().getContext();
for (const Ice::FunctionDeclaration *Func : FunctionDeclarations) {
+ if (!Func->verifyLinkageCorrect(Ctx)) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Function " << Func->getName()
+ << " has incorrect linkage: " << Func->getLinkageName();
+ Error(StrBuf.str());
+ continue;
+ }
Ice::Constant *C = nullptr;
if (!isIRGenerationDisabled()) {
C = getConstantSym(Func->getName(), Func->getSuppressMangling(),
@@ -487,7 +496,15 @@
// Converts global variable declarations into constant value IDs.
void createValueIDsForGlobalVars() {
+ Ice::GlobalContext *Ctx = getTranslator().getContext();
for (const Ice::VariableDeclaration *Decl : *VariableDeclarations) {
+ if (!Decl->verifyLinkageCorrect(Ctx)) {
+ std::string Buffer;
+ raw_string_ostream StrBuf(Buffer);
+ StrBuf << "Global " << Decl->getName()
+ << " has incorrect linkage: " << Decl->getLinkageName();
+ Error(StrBuf.str());
+ }
Ice::Constant *C = nullptr;
if (!isIRGenerationDisabled()) {
C = getConstantSym(Decl->getName(), Decl->getSuppressMangling(),
