Assemble calls to constant addresses.
Finally address this TODO in the assembler. This will help translate
non-IRT using programs (no ABI stability). The default scons testing
mode is non-IRT, so this helps with that. I haven't actually tested
this against scons yet, but I'm filling in the tests based on how
LLVM translates the same bitcode.
The filetype=asm is adjusted to omit the "*" and the "$".
The filetype=obj is adjusted to check for fixups with NullSymbols,
and also fill the assembler buffer at the instruction's immediate
field w/ the right constant.
The filetype=iasm is still TODO (hits an new assert in the Fixup's emit() function).
Reverts 7ad1bed99d058199a3ba246a5383458518596fbc:
"Allow stubbing of called constant addresses using command line argument."
since this is now handled (except for iasm).
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4080
R=stichnot@chromium.org
Review URL: https://codereview.chromium.org/1017373002
diff --git a/src/IceClFlags.h b/src/IceClFlags.h
index 3ddbbf8..6fcbbbb 100644
--- a/src/IceClFlags.h
+++ b/src/IceClFlags.h
@@ -30,8 +30,8 @@
DecorateAsm(false), DisableInternal(false), DisableIRGeneration(false),
DisableTranslation(false), DumpStats(false), FunctionSections(false),
GenerateUnitTestMessages(false), PhiEdgeSplit(false),
- StubConstantCalls(false), SubzeroTimingEnabled(false),
- TimeEachFunction(false), UseSandboxing(false),
+ SubzeroTimingEnabled(false), TimeEachFunction(false),
+ UseSandboxing(false),
// FileType field
OutFileType(FT_Iasm),
// IceString fields.
@@ -87,11 +87,6 @@
bool getPhiEdgeSplit() const { return PhiEdgeSplit; }
void setPhiEdgeSplit(bool NewValue) { PhiEdgeSplit = NewValue; }
- bool getStubConstantCalls() const {
- return !ALLOW_MINIMAL_BUILD && StubConstantCalls;
- }
- void setStubConstantCalls(bool NewValue) { StubConstantCalls = NewValue; }
-
bool getSubzeroTimingEnabled() const { return SubzeroTimingEnabled; }
void setSubzeroTimingEnabled(bool NewValue) {
SubzeroTimingEnabled = NewValue;
@@ -154,7 +149,6 @@
bool FunctionSections;
bool GenerateUnitTestMessages;
bool PhiEdgeSplit;
- bool StubConstantCalls;
bool SubzeroTimingEnabled;
bool TimeEachFunction;
bool UseSandboxing;
diff --git a/src/IceELFObjectWriter.cpp b/src/IceELFObjectWriter.cpp
index 230848c..073163a 100644
--- a/src/IceELFObjectWriter.cpp
+++ b/src/IceELFObjectWriter.cpp
@@ -84,10 +84,7 @@
"Elf_Sym sizes cannot be derived from sizeof");
SymTab = createSection<ELFSymbolTableSection>(SymTabName, SHT_SYMTAB, 0,
SymTabAlign, SymTabEntSize);
- // The first entry in the symbol table should be a NULL entry.
- const IceString NullSymName("");
- SymTab->createDefinedSym(NullSymName, STT_NOTYPE, STB_LOCAL, NullSection, 0,
- 0);
+ SymTab->createNullSymbol(NullSection);
const IceString StrTabName(".strtab");
StrTab =
diff --git a/src/IceELFSection.cpp b/src/IceELFSection.cpp
index f2e129d..4ca2de4 100644
--- a/src/IceELFSection.cpp
+++ b/src/IceELFSection.cpp
@@ -80,6 +80,15 @@
// Symbol tables.
+void ELFSymbolTableSection::createNullSymbol(ELFSection *NullSection) {
+ // The first entry in the symbol table should be a NULL entry,
+ // so make sure the map is still empty.
+ assert(LocalSymbols.empty());
+ const IceString NullSymName("");
+ createDefinedSym(NullSymName, STT_NOTYPE, STB_LOCAL, NullSection, 0, 0);
+ NullSymbol = findSymbol(NullSymName);
+}
+
void ELFSymbolTableSection::createDefinedSym(const IceString &Name,
uint8_t Type, uint8_t Binding,
ELFSection *Section,
@@ -105,7 +114,9 @@
NewSymbol.Sym.setBindingAndType(STB_GLOBAL, STT_NOTYPE);
NewSymbol.Section = NullSection;
NewSymbol.Number = ELFSym::UnknownNumber;
- GlobalSymbols.insert(std::make_pair(Name, NewSymbol));
+ bool Unique = GlobalSymbols.insert(std::make_pair(Name, NewSymbol)).second;
+ assert(Unique);
+ (void)Unique;
}
const ELFSym *ELFSymbolTableSection::findSymbol(const IceString &Name) const {
diff --git a/src/IceELFSection.h b/src/IceELFSection.h
index 0bfccca..c2ef9bf 100644
--- a/src/IceELFSection.h
+++ b/src/IceELFSection.h
@@ -163,7 +163,11 @@
ELFSymbolTableSection &operator=(const ELFSymbolTableSection &) = delete;
public:
- using ELFSection::ELFSection;
+ ELFSymbolTableSection(const IceString &Name, Elf64_Word ShType,
+ Elf64_Xword ShFlags, Elf64_Xword ShAddralign,
+ Elf64_Xword ShEntsize)
+ : ELFSection(Name, ShType, ShFlags, ShAddralign, ShEntsize),
+ NullSymbol(nullptr) {}
// Create initial entry for a symbol when it is defined.
// Each entry should only be defined once.
@@ -179,6 +183,9 @@
const ELFSym *findSymbol(const IceString &Name) const;
+ void createNullSymbol(ELFSection *NullSection);
+ const ELFSym *getNullSymbol() const { return NullSymbol; }
+
size_t getSectionDataSize() const {
return (LocalSymbols.size() + GlobalSymbols.size()) * Header.sh_entsize;
}
@@ -198,6 +205,7 @@
template <bool IsELF64>
void writeSymbolMap(ELFStreamer &Str, const SymMap &Map);
+ const ELFSym *NullSymbol;
// Keep Local and Global symbols separate, since the sh_info needs to
// know the index of the last LOCAL.
SymMap LocalSymbols;
@@ -211,7 +219,11 @@
ELFRelocationSection &operator=(const ELFRelocationSection &) = delete;
public:
- using ELFSection::ELFSection;
+ ELFRelocationSection(const IceString &Name, Elf64_Word ShType,
+ Elf64_Xword ShFlags, Elf64_Xword ShAddralign,
+ Elf64_Xword ShEntsize)
+ : ELFSection(Name, ShType, ShFlags, ShAddralign, ShEntsize),
+ RelatedSection(nullptr) {}
const ELFSection *getRelatedSection() const { return RelatedSection; }
void setRelatedSection(const ELFSection *Section) {
@@ -340,7 +352,11 @@
void ELFRelocationSection::writeData(const GlobalContext &Ctx, ELFStreamer &Str,
const ELFSymbolTableSection *SymTab) {
for (const AssemblerFixup &Fixup : Fixups) {
- const ELFSym *Symbol = SymTab->findSymbol(Fixup.symbol(&Ctx));
+ const ELFSym *Symbol;
+ if (Fixup.isNullSymbol())
+ Symbol = SymTab->getNullSymbol();
+ else
+ Symbol = SymTab->findSymbol(Fixup.symbol(&Ctx));
if (!Symbol)
llvm::report_fatal_error("Missing symbol mentioned in reloc");
diff --git a/src/IceFixups.cpp b/src/IceFixups.cpp
index 52bb0dd..05559c5 100644
--- a/src/IceFixups.cpp
+++ b/src/IceFixups.cpp
@@ -17,7 +17,11 @@
namespace Ice {
+const Constant *AssemblerFixup::NullSymbol = nullptr;
+
RelocOffsetT AssemblerFixup::offset() const {
+ if (isNullSymbol())
+ return 0;
if (const auto CR = llvm::dyn_cast<ConstantRelocatable>(value_))
return CR->getOffset();
return 0;
@@ -27,6 +31,7 @@
std::string Buffer;
llvm::raw_string_ostream Str(Buffer);
const Constant *C = value_;
+ assert(!isNullSymbol());
if (const auto CR = llvm::dyn_cast<ConstantRelocatable>(C)) {
if (CR->getSuppressMangling())
Str << CR->getName();
@@ -43,6 +48,9 @@
void AssemblerFixup::emit(GlobalContext *Ctx) const {
Ostream &Str = Ctx->getStrEmit();
+ // TODO(jvoung): Not yet clear how to emit a relocation against
+ // the null symbol.
+ assert(!isNullSymbol());
Str << symbol(Ctx);
RelocOffsetT Offset = offset();
if (Offset)
diff --git a/src/IceFixups.h b/src/IceFixups.h
index bbe7d4a..1bc45f3 100644
--- a/src/IceFixups.h
+++ b/src/IceFixups.h
@@ -38,6 +38,10 @@
RelocOffsetT offset() const;
IceString symbol(const GlobalContext *Ctx) const;
+
+ static const Constant *NullSymbol;
+ bool isNullSymbol() const { return value_ == NullSymbol; }
+
void set_value(const Constant *Value) { value_ = Value; }
void emit(GlobalContext *Ctx) const;
diff --git a/src/IceInstX8632.cpp b/src/IceInstX8632.cpp
index 776a870..2464567 100644
--- a/src/IceInstX8632.cpp
+++ b/src/IceInstX8632.cpp
@@ -489,11 +489,10 @@
} else if (const auto Imm = llvm::dyn_cast<ConstantInteger32>(Target)) {
// NaCl trampoline calls refer to an address within the sandbox directly.
// This is usually only needed for non-IRT builds and otherwise not
- // very portable or stable. For this, we would use the 0xE8 opcode
- // (relative version of call) and there should be a PC32 reloc too.
- // The PC32 reloc will have symbol index 0, and the absolute address
- // would be encoded as an offset relative to the next instruction.
- // TODO(jvoung): Do we need to support this?
+ // very portable or stable. Usually this is only done for "calls"
+ // and not jumps.
+ // TODO(jvoung): Support this when there is a lowering that
+ // actually triggers this case.
(void)Imm;
llvm::report_fatal_error("Unexpected jmp to absolute address");
} else {
@@ -515,10 +514,7 @@
Ostream &Str = Func->getContext()->getStrEmit();
assert(getSrcSize() == 1);
Str << "\tcall\t";
- if (const auto CallTarget =
- llvm::dyn_cast<ConstantRelocatable>(getCallTarget())) {
- // TODO(stichnot): All constant targets should suppress the '$',
- // not just relocatables.
+ if (const auto CallTarget = llvm::dyn_cast<Constant>(getCallTarget())) {
CallTarget->emitWithoutDollar(Func->getContext());
} else {
Str << "*";
@@ -544,15 +540,7 @@
assert(CR->getOffset() == 0 && "We only support calling a function");
Asm->call(CR);
} else if (const auto Imm = llvm::dyn_cast<ConstantInteger32>(Target)) {
- // NaCl trampoline calls refer to an address within the sandbox directly.
- // This is usually only needed for non-IRT builds and otherwise not
- // very portable or stable. For this, we would use the 0xE8 opcode
- // (relative version of call) and there should be a PC32 reloc too.
- // The PC32 reloc will have symbol index 0, and the absolute address
- // would be encoded as an offset relative to the next instruction.
- // TODO(jvoung): Do we need to support this?
- (void)Imm;
- llvm_unreachable("Unexpected call to absolute address");
+ Asm->call(x86::Immediate(Imm->getValue()));
} else {
llvm_unreachable("Unexpected operand type");
}
diff --git a/src/IceOperand.h b/src/IceOperand.h
index 5630c5c..3bbd02c 100644
--- a/src/IceOperand.h
+++ b/src/IceOperand.h
@@ -108,6 +108,7 @@
}
void emit(const Cfg *Func) const override { emit(Func->getContext()); }
virtual void emit(GlobalContext *Ctx) const = 0;
+ virtual void emitWithoutDollar(GlobalContext *Ctx) const = 0;
static bool classof(const Operand *Operand) {
OperandKind Kind = Operand->getKind();
@@ -149,6 +150,7 @@
// The target needs to implement this for each ConstantPrimitive
// specialization.
void emit(GlobalContext *Ctx) const override;
+ void emitWithoutDollar(GlobalContext *Ctx) const override;
using Constant::dump;
void dump(const Cfg *, Ostream &Str) const override {
if (ALLOW_DUMP)
@@ -234,7 +236,7 @@
using Constant::emit;
using Constant::dump;
void emit(GlobalContext *Ctx) const override;
- void emitWithoutDollar(GlobalContext *Ctx) const;
+ void emitWithoutDollar(GlobalContext *Ctx) const override;
void dump(const Cfg *Func, Ostream &Str) const override;
static bool classof(const Operand *Operand) {
@@ -273,6 +275,7 @@
using Constant::dump;
// The target needs to implement this.
void emit(GlobalContext *Ctx) const override;
+ void emitWithoutDollar(GlobalContext *Ctx) const override;
void dump(const Cfg *, Ostream &Str) const override {
if (ALLOW_DUMP)
Str << "undef";
diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index 42f513b..4f619d6 100644
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -4599,17 +4599,34 @@
}
}
+template <>
+void ConstantInteger32::emitWithoutDollar(GlobalContext *Ctx) const {
+ if (!ALLOW_DUMP)
+ return;
+ Ostream &Str = Ctx->getStrEmit();
+ Str << (int32_t)getValue();
+}
+
template <> void ConstantInteger32::emit(GlobalContext *Ctx) const {
if (!ALLOW_DUMP)
return;
Ostream &Str = Ctx->getStrEmit();
- Str << "$" << (int32_t)getValue();
+ Str << "$";
+ emitWithoutDollar(Ctx);
+}
+
+template <> void ConstantInteger64::emitWithoutDollar(GlobalContext *) const {
+ llvm_unreachable("Not expecting to emitWithoutDollar 64-bit integers");
}
template <> void ConstantInteger64::emit(GlobalContext *) const {
llvm_unreachable("Not expecting to emit 64-bit integers");
}
+template <> void ConstantFloat::emitWithoutDollar(GlobalContext *) const {
+ llvm_unreachable("Not expecting to emitWithoutDollar floats");
+}
+
template <> void ConstantFloat::emit(GlobalContext *Ctx) const {
if (!ALLOW_DUMP)
return;
@@ -4617,6 +4634,10 @@
emitPoolLabel(Str);
}
+template <> void ConstantDouble::emitWithoutDollar(GlobalContext *) const {
+ llvm_unreachable("Not expecting to emitWithoutDollar doubles");
+}
+
template <> void ConstantDouble::emit(GlobalContext *Ctx) const {
if (!ALLOW_DUMP)
return;
@@ -4624,6 +4645,10 @@
emitPoolLabel(Str);
}
+void ConstantUndef::emitWithoutDollar(GlobalContext *) const {
+ llvm_unreachable("Not expecting to emitWithoutDollar undef");
+}
+
void ConstantUndef::emit(GlobalContext *) const {
llvm_unreachable("undef value encountered by emitter.");
}
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index f02d0cb..799ff94 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -167,7 +167,7 @@
: NaClBitcodeParser(Cursor), Translator(Translator),
ErrorStatus(ErrorStatus), NumErrors(0), NextDefiningFunctionID(0),
VariableDeclarations(new Ice::VariableDeclarationList()),
- BlockParser(nullptr), StubbedConstCallValue(nullptr) {}
+ BlockParser(nullptr) {}
~TopLevelParser() override {}
@@ -276,24 +276,6 @@
createValueIDsForGlobalVars();
}
- /// Returns a defined function reference to be used in place of
- /// called constant addresses. Returns the corresponding operand
- /// to replace the calling address with. Reports an error if
- /// a stub could not be found, returning the CallValue.
- Ice::Operand *getStubbedConstCallValue(Ice::Operand *CallValue) {
- if (StubbedConstCallValue)
- return StubbedConstCallValue;
- for (unsigned i = 0; i < getNumFunctionIDs(); ++i) {
- Ice::FunctionDeclaration *Func = getFunctionByID(i);
- if (!Func->isProto()) {
- StubbedConstCallValue = getGlobalConstantByID(i);
- return StubbedConstCallValue;
- }
- }
- Error("Unable to find function definition to stub constant calls with");
- return CallValue;
- }
-
/// Returns the number of function declarations in the bitcode file.
unsigned getNumFunctionIDs() const { return FunctionDeclarationList.size(); }
@@ -383,8 +365,6 @@
// The block parser currently being applied. Used for error
// reporting.
BlockParserBaseClass *BlockParser;
- // Value to use to stub constant calls.
- Ice::Operand *StubbedConstCallValue;
bool ParseBlock(unsigned BlockID) override;
@@ -510,8 +490,8 @@
raw_ostream &OldErrStream = setErrStream(Context->getStrDump());
NaClBitcodeParser::ErrorAt(Level, Bit, Message);
setErrStream(OldErrStream);
- if (Level >= naclbitc::Error
- && !Translator.getFlags().getAllowErrorRecovery())
+ if (Level >= naclbitc::Error &&
+ !Translator.getFlags().getAllowErrorRecovery())
Fatal();
return true;
}
@@ -677,8 +657,8 @@
}
// Generates an error Message with the bit address prefixed to it.
-bool BlockParserBaseClass::ErrorAt(
- naclbitc::ErrorLevel Level, uint64_t Bit, const std::string &Message) {
+bool BlockParserBaseClass::ErrorAt(naclbitc::ErrorLevel Level, uint64_t Bit,
+ const std::string &Message) {
std::string Buffer;
raw_string_ostream StrBuf(Buffer);
// Note: If dump routines have been turned off, the error messages
@@ -2507,10 +2487,6 @@
return;
}
} else {
- if (getFlags().getStubConstantCalls() &&
- llvm::isa<Ice::ConstantInteger32>(Callee)) {
- Callee = Context->getStubbedConstCallValue(Callee);
- }
ReturnType = Context->getSimpleTypeByID(Values[2]);
}
diff --git a/src/assembler_ia32.cpp b/src/assembler_ia32.cpp
index a57d791..216cea6 100644
--- a/src/assembler_ia32.cpp
+++ b/src/assembler_ia32.cpp
@@ -112,6 +112,17 @@
(void)call_start;
}
+void AssemblerX86::call(const Immediate &abs_address) {
+ AssemblerBuffer::EnsureCapacity ensured(&buffer_);
+ intptr_t call_start = buffer_.GetPosition();
+ EmitUint8(0xE8);
+ EmitFixup(
+ this->createFixup(llvm::ELF::R_386_PC32, AssemblerFixup::NullSymbol));
+ EmitInt32(abs_address.value() - 4);
+ assert((buffer_.GetPosition() - call_start) == kCallExternalLabelSize);
+ (void)call_start;
+}
+
void AssemblerX86::pushl(GPRRegister reg) {
AssemblerBuffer::EnsureCapacity ensured(&buffer_);
EmitUint8(0x50 + reg);
diff --git a/src/assembler_ia32.h b/src/assembler_ia32.h
index a6a0bb4..1781b47 100644
--- a/src/assembler_ia32.h
+++ b/src/assembler_ia32.h
@@ -475,6 +475,7 @@
void call(GPRRegister reg);
void call(const Address &address);
void call(const ConstantRelocatable *label);
+ void call(const Immediate &abs_address);
static const intptr_t kCallExternalLabelSize = 5;
diff --git a/src/main.cpp b/src/main.cpp
index 22ab0a1..bd96142 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -176,12 +176,6 @@
cl::desc("Allow error recovery when reading PNaCl bitcode."),
cl::init(false));
-// TODO(kschimpf) Remove once the emitter handles these cases.
-static cl::opt<bool>
- StubConstantCalls("stub-const-calls",
- cl::desc("Stub indirect calls to constants."),
- cl::init(false));
-
static cl::opt<bool> LLVMVerboseErrors(
"verbose-llvm-parse-errors",
cl::desc("Print out more descriptive PNaCl bitcode parse errors when "
@@ -311,7 +305,6 @@
Flags.setFunctionSections(FunctionSections);
Flags.setNumTranslationThreads(NumThreads);
Flags.setPhiEdgeSplit(EnablePhiEdgeSplit);
- Flags.setStubConstantCalls(StubConstantCalls);
Flags.setSubzeroTimingEnabled(SubzeroTimingEnabled);
Flags.setTimeEachFunction(TimeEachFunction);
Flags.setTimingFocusOn(TimingFocusOn);
diff --git a/tests_lit/llvm2ice_tests/callindirect.pnacl.ll b/tests_lit/llvm2ice_tests/callindirect.pnacl.ll
index dfa605b..b77ae53 100644
--- a/tests_lit/llvm2ice_tests/callindirect.pnacl.ll
+++ b/tests_lit/llvm2ice_tests/callindirect.pnacl.ll
@@ -4,6 +4,9 @@
; RUN: %p2i --filetype=obj --disassemble -i %s --args -O2 \
; RUN: | FileCheck %s
+; RUN: %if --need=allow_dump --command %p2i --filetype=asm --assemble \
+; RUN: --disassemble -i %s --args -O2 \
+; RUN: | %if --need=allow_dump --command FileCheck %s
; RUN: %p2i --filetype=obj --disassemble -i %s --args -Om1 \
; RUN: | FileCheck --check-prefix=OPTM1 %s
@@ -62,14 +65,23 @@
; OPTM1: call [[TARGET]]
; Calling an absolute address is used for non-IRT PNaCl pexes to directly
-; access syscall trampolines. Do we need to support this?
-; define internal void @CallIndirectConst() {
-; entry:
-; %__1 = inttoptr i32 66496 to void ()*
-; call void %__1()
-; call void %__1()
-; call void %__1()
-; call void %__1()
-; call void %__1()
-; ret void
-; }
+; access syscall trampolines. This is not really an indirect call, but
+; there is a cast from int to pointer first.
+define internal void @CallConst() {
+entry:
+ %__1 = inttoptr i32 66496 to void ()*
+ call void %__1()
+ call void %__1()
+ call void %__1()
+ ret void
+}
+
+; CHECK-LABEL: CallConst
+; CHECK: e8 bc 03 01 00 call {{[0-9a-f]+}} {{.*}} R_386_PC32 *ABS*
+; CHECK: e8 bc 03 01 00 call {{[0-9a-f]+}} {{.*}} R_386_PC32 *ABS*
+; CHECK: e8 bc 03 01 00 call {{[0-9a-f]+}} {{.*}} R_386_PC32 *ABS*
+;
+; OPTM1-LABEL: CallConst
+; OPTM1: e8 bc 03 01 00 call {{[0-9a-f]+}} {{.*}} R_386_PC32 *ABS*
+; OPTM1: e8 bc 03 01 00 call {{[0-9a-f]+}} {{.*}} R_386_PC32 *ABS*
+; OPTM1: e8 bc 03 01 00 call {{[0-9a-f]+}} {{.*}} R_386_PC32 *ABS*
