Subzero: Control memory growth from local label fixups.
The ConstantRelocatable objects for pushing local labels are allocated from the Assembler arena, and are no longer pooled, which restricts the memory growth from sandboxing x86-64 calls.
Because the Assembler arena is destroyed while the fixups are still active, these fixups have to be fixed up by holding a pointer to the symbol rather than the constant.
On the 10MB test pexe, the overall growth by the end is ~20MB, instead of ~130MB as before.
This also partially fixes an existing bug with arm32/nonsfi/iasm, exposed by running cross tests and forcing iasm output.
BUG= none
R=jpp@chromium.org
Review URL: https://codereview.chromium.org/1773503003 .
diff --git a/pydir/crosstest_generator.py b/pydir/crosstest_generator.py
index 236c2d0..c634586 100755
--- a/pydir/crosstest_generator.py
+++ b/pydir/crosstest_generator.py
@@ -205,6 +205,8 @@
f.write(run_cmd + '\n')
f.write('echo Recreate a failure using ' + __file__ +
' --toolchain-root=' + args.toolchain_root +
+ (' --filetype=' + args.filetype
+ if args.filetype else '') +
' --include=' + ','.join(desc) + '\n')
f.write('# CHECK: Failures=0\n')
else:
diff --git a/src/IceAssemblerARM32.cpp b/src/IceAssemblerARM32.cpp
index 1117537..613fa77 100644
--- a/src/IceAssemblerARM32.cpp
+++ b/src/IceAssemblerARM32.cpp
@@ -593,11 +593,16 @@
IValueT Inst = Asm.load<IValueT>(position());
const bool IsMovw = kind() == llvm::ELF::R_ARM_MOVW_ABS_NC ||
kind() == llvm::ELF::R_ARM_MOVW_PREL_NC;
+ const IceString Symbol = symbol();
+ const bool NeedsPCRelSuffix =
+ (Asm.fixupIsPCRel(kind()) || Symbol == GlobalOffsetTable);
Str << "\t"
"mov" << (IsMovw ? "w" : "t") << "\t"
<< RegARM32::getRegName(RegNumT::fixme((Inst >> kRdShift) & 0xF))
- << ", #:" << (IsMovw ? "lower" : "upper") << "16:" << symbol(&Asm)
- << "\t@ .word " << llvm::format_hex_no_prefix(Inst, 8) << "\n";
+ << ", #:" << (IsMovw ? "lower" : "upper") << "16:" << Symbol
+ << (NeedsPCRelSuffix ? " - ." : "") << "\t@ .word "
+ // TODO(jpp): This is broken, it also needs to add a magic constant.
+ << llvm::format_hex_no_prefix(Inst, 8) << "\n";
return InstARM32::InstSize;
}
@@ -650,7 +655,7 @@
Ostream &Str = Ctx->getStrEmit();
IValueT Inst = Asm.load<IValueT>(position());
Str << "\t"
- "bl\t" << symbol(&Asm) << "\t@ .word "
+ "bl\t" << symbol() << "\t@ .word "
<< llvm::format_hex_no_prefix(Inst, 8) << "\n";
return InstARM32::InstSize;
}
diff --git a/src/IceAssemblerARM32.h b/src/IceAssemblerARM32.h
index 0e95eab..1cf07ad 100644
--- a/src/IceAssemblerARM32.h
+++ b/src/IceAssemblerARM32.h
@@ -160,8 +160,10 @@
}
bool fixupIsPCRel(FixupKind Kind) const override {
- (void)Kind;
- // TODO(kschimpf) Decide if we need this.
+ if (Kind == llvm::ELF::R_ARM_MOVW_PREL_NC)
+ return true;
+ if (Kind == llvm::ELF::R_ARM_MOVT_PREL)
+ return true;
return false;
}
diff --git a/src/IceELFObjectWriter.cpp b/src/IceELFObjectWriter.cpp
index fd1150e..d66b553 100644
--- a/src/IceELFObjectWriter.cpp
+++ b/src/IceELFObjectWriter.cpp
@@ -275,7 +275,7 @@
Fixup->emitOffset(Asm);
}
}
- RelSection->addRelocations(OffsetInSection, Asm->fixups());
+ RelSection->addRelocations(OffsetInSection, Asm->fixups(), SymTab);
}
Section->appendData(Str, Asm->getBufferView());
if (TimeThisFunction)
diff --git a/src/IceELFSection.cpp b/src/IceELFSection.cpp
index cf5c733..89c7369 100644
--- a/src/IceELFSection.cpp
+++ b/src/IceELFSection.cpp
@@ -67,11 +67,24 @@
// Relocation sections.
void ELFRelocationSection::addRelocations(RelocOffsetT BaseOff,
- const FixupRefList &FixupRefs) {
+ const FixupRefList &FixupRefs,
+ ELFSymbolTableSection *SymTab) {
for (const AssemblerFixup *FR : FixupRefs) {
Fixups.push_back(*FR);
AssemblerFixup &F = Fixups.back();
F.set_position(BaseOff + F.position());
+ assert(!F.valueIsSymbol());
+ if (!F.isNullSymbol()) {
+ // Do an early lookup in the symbol table. If the symbol is found,
+ // replace the Constant in the symbol with the ELFSym, and calculate the
+ // final value of the addend. As such, a local label allocated from the
+ // Assembler arena will be converted to a symbol before the Assembler
+ // arena goes away.
+ if (const ELFSym *Sym = SymTab->findSymbol(F.symbol())) {
+ F.set_addend(F.offset());
+ F.set_value(Sym);
+ }
+ }
}
}
diff --git a/src/IceELFSection.h b/src/IceELFSection.h
index 28fb6b0..c57b821 100644
--- a/src/IceELFSection.h
+++ b/src/IceELFSection.h
@@ -232,7 +232,8 @@
/// Track additional relocations which start out relative to offset 0, but
/// should be adjusted to be relative to BaseOff.
- void addRelocations(RelocOffsetT BaseOff, const FixupRefList &FixupRefs);
+ void addRelocations(RelocOffsetT BaseOff, const FixupRefList &FixupRefs,
+ ELFSymbolTableSection *SymTab);
/// Track a single additional relocation.
void addRelocation(const AssemblerFixup &Fixup) { Fixups.push_back(Fixup); }
@@ -353,9 +354,10 @@
const ELFSym *Symbol;
if (Fixup.isNullSymbol()) {
Symbol = SymTab->getNullSymbol();
+ } else if (Fixup.valueIsSymbol()) {
+ Symbol = Fixup.getSymbolValue();
} else {
- constexpr Assembler *Asm = nullptr;
- const IceString Name = Fixup.symbol(Asm);
+ const IceString Name = Fixup.symbol();
Symbol = SymTab->findSymbol(Name);
if (!Symbol)
llvm::report_fatal_error(Name + ": Missing symbol mentioned in reloc");
diff --git a/src/IceFixups.cpp b/src/IceFixups.cpp
index b4a5b2b..27d6eab 100644
--- a/src/IceFixups.cpp
+++ b/src/IceFixups.cpp
@@ -24,30 +24,26 @@
RelocOffsetT AssemblerFixup::offset() const {
if (isNullSymbol())
return addend_;
- if (const auto *CR = llvm::dyn_cast<ConstantRelocatable>(value_))
- return CR->getOffset() + addend_;
+ if (!ValueIsSymbol) {
+ if (const auto *CR = llvm::dyn_cast<ConstantRelocatable>(ConstValue))
+ return CR->getOffset() + addend_;
+ }
return addend_;
}
-IceString AssemblerFixup::symbol(const Assembler *Asm) const {
+IceString AssemblerFixup::symbol() const {
+ assert(!isNullSymbol());
+ assert(!ValueIsSymbol);
+ const Constant *C = ConstValue;
+ if (const auto *CR = llvm::dyn_cast<ConstantRelocatable>(C)) {
+ return CR->getName();
+ }
+ // NOTE: currently only float/doubles are put into constant pools. In the
+ // future we may put integers as well.
+ assert(llvm::isa<ConstantFloat>(C) || llvm::isa<ConstantDouble>(C));
std::string Buffer;
llvm::raw_string_ostream Str(Buffer);
- const Constant *C = value_;
- assert(!isNullSymbol());
- if (const auto *CR = llvm::dyn_cast<ConstantRelocatable>(C)) {
- Str << CR->getName();
- if (Asm && !Asm->fixupIsPCRel(kind()) &&
- GlobalContext::getFlags().getUseNonsfi() &&
- CR->getName() != GlobalOffsetTable) {
- // TODO(jpp): remove the special GOT test.
- Str << "@GOTOFF";
- }
- } else {
- // NOTE: currently only float/doubles are put into constant pools. In the
- // future we may put integers as well.
- assert(llvm::isa<ConstantFloat>(C) || llvm::isa<ConstantDouble>(C));
- C->emitPoolLabel(Str);
- }
+ C->emitPoolLabel(Str);
return Str.str();
}
@@ -61,8 +57,16 @@
if (isNullSymbol()) {
Str << "__Sz_AbsoluteZero";
} else {
- Symbol = symbol(&Asm);
+ Symbol = symbol();
Str << Symbol;
+ assert(!ValueIsSymbol);
+ if (const auto *CR = llvm::dyn_cast<ConstantRelocatable>(ConstValue)) {
+ if (!Asm.fixupIsPCRel(kind()) &&
+ GlobalContext::getFlags().getUseNonsfi() &&
+ CR->getName() != GlobalOffsetTable) {
+ Str << "@GOTOFF";
+ }
+ }
}
assert(Asm.load<RelocOffsetT>(position()) == 0);
diff --git a/src/IceFixups.h b/src/IceFixups.h
index 47ea02e..2232a67 100644
--- a/src/IceFixups.h
+++ b/src/IceFixups.h
@@ -23,6 +23,8 @@
/// This holds the specific target+container format's relocation number.
using FixupKind = uint32_t;
+struct ELFSym;
+
/// Assembler fixups are positions in generated code/data that hold relocation
/// information that needs to be processed before finalizing the code/data.
class AssemblerFixup {
@@ -32,27 +34,33 @@
AssemblerFixup() = default;
AssemblerFixup(const AssemblerFixup &) = default;
virtual ~AssemblerFixup() = default;
- intptr_t position() const {
- assert(position_was_set_);
- return position_;
- }
- void set_position(intptr_t Position) {
- position_ = Position;
- position_was_set_ = true;
- }
+ intptr_t position() const { return position_; }
+ void set_position(intptr_t Position) { position_ = Position; }
FixupKind kind() const { return kind_; }
void set_kind(FixupKind Kind) { kind_ = Kind; }
RelocOffsetT offset() const;
- IceString symbol(const Assembler *Asm) const;
+ IceString symbol() const;
static const Constant *NullSymbol;
- bool isNullSymbol() const { return value_ == NullSymbol; }
+ bool isNullSymbol() const { return ConstValue == NullSymbol; }
static constexpr AssemblerFixup *NoFixup = nullptr;
- void set_value(const Constant *Value) { value_ = Value; }
+ bool valueIsSymbol() const { return ValueIsSymbol; }
+ void set_value(const Constant *Value) {
+ ValueIsSymbol = false;
+ ConstValue = Value;
+ }
+ void set_value(const ELFSym *Value) {
+ ValueIsSymbol = true;
+ SymbolValue = Value;
+ }
+ const ELFSym *getSymbolValue() const {
+ assert(ValueIsSymbol);
+ return SymbolValue;
+ }
void set_addend(RelocOffsetT Addend) { addend_ = Addend; }
RelocOffsetT get_addend() const { return addend_; }
@@ -65,13 +73,19 @@
virtual void emitOffset(Assembler *Asm) const;
private:
- bool position_was_set_ = false;
intptr_t position_ = 0;
FixupKind kind_ = 0;
- const Constant *value_ = nullptr;
// An offset addend to the fixup offset (as returned by offset()), in case the
// assembler needs to adjust it.
RelocOffsetT addend_ = 0;
+
+ // Tagged union that holds either a Constant or ELFSym pointer, depending on
+ // the ValueIsSymbol tag.
+ bool ValueIsSymbol = false;
+ union {
+ const Constant *ConstValue;
+ const ELFSym *SymbolValue;
+ };
};
/// Extends a fixup to be textual. That is, it emits text instead of a sequence
diff --git a/src/IceOperand.h b/src/IceOperand.h
index 888e4df..bc48b74 100644
--- a/src/IceOperand.h
+++ b/src/IceOperand.h
@@ -270,8 +270,8 @@
RelocOffset &operator=(const RelocOffset &) = delete;
public:
- static RelocOffset *create(GlobalContext *Ctx) {
- return new (Ctx->allocate<RelocOffset>()) RelocOffset();
+ template <typename T> static RelocOffset *create(T *AllocOwner) {
+ return new (AllocOwner->template allocate<RelocOffset>()) RelocOffset();
}
static RelocOffset *create(GlobalContext *Ctx, RelocOffsetT Value) {
@@ -342,10 +342,12 @@
ConstantRelocatable &operator=(const ConstantRelocatable &) = delete;
public:
- static ConstantRelocatable *create(GlobalContext *Ctx, Type Ty,
+ template <typename T>
+ static ConstantRelocatable *create(T *AllocOwner, Type Ty,
const RelocatableTuple &Tuple) {
- return new (Ctx->allocate<ConstantRelocatable>()) ConstantRelocatable(
- Ty, Tuple.Offset, Tuple.OffsetExpr, Tuple.Name, Tuple.EmitString);
+ return new (AllocOwner->template allocate<ConstantRelocatable>())
+ ConstantRelocatable(Ty, Tuple.Offset, Tuple.OffsetExpr, Tuple.Name,
+ Tuple.EmitString);
}
RelocOffsetT getOffset() const {
diff --git a/src/IceTargetLoweringX8664.cpp b/src/IceTargetLoweringX8664.cpp
index 28304a9..5aaf2c9 100644
--- a/src/IceTargetLoweringX8664.cpp
+++ b/src/IceTargetLoweringX8664.cpp
@@ -623,13 +623,14 @@
//
// push .after_call
InstX86Label *ReturnAddress = InstX86Label::create(Func, this);
- auto *ReturnRelocOffset = RelocOffset::create(Ctx);
+ auto *ReturnRelocOffset = RelocOffset::create(Func->getAssembler());
ReturnAddress->setRelocOffset(ReturnRelocOffset);
constexpr RelocOffsetT NoFixedOffset = 0;
const IceString EmitString = ReturnAddress->getName(Func);
- auto *ReturnReloc = llvm::cast<ConstantRelocatable>(
- Ctx->getConstantSym(NoFixedOffset, {ReturnRelocOffset},
- Func->getFunctionName(), EmitString));
+ auto *ReturnReloc = ConstantRelocatable::create(
+ Func->getAssembler(), IceType_i32,
+ RelocatableTuple(NoFixedOffset, {ReturnRelocOffset},
+ Func->getFunctionName(), EmitString));
/* AutoBundle scoping */ {
std::unique_ptr<AutoBundle> Bundler;
if (CallTargetR == nullptr) {
