This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [GOLD][PATCH][UPDATED] Added support for R_ARM_V4BX relocation (with interworking)
- From: =?big5?b?RG91ZyBLd2FuICjD9q62vHcp?= <dougkwan at google dot com>
- To: Viktor Kutuzov <vkutuzov at accesssoftek dot com>, Ian Lance Taylor <iant at google dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 19 Jan 2010 15:44:29 -0800
- Subject: Re: [GOLD][PATCH][UPDATED] Added support for R_ARM_V4BX relocation (with interworking)
- References: <6AE1604EE3EC5F4296C096518C6B77EE01884D438D@mail.accesssoftek.com>
Please see comments below.
+
+ protected:
+ // Arm V4BX stubs are created via a stub factory. So these are protected.
+ Arm_v4bx_stub(const Stub_template* stub_template, const uint32_t reg)
+ : Stub(stub_template), reg_(reg)
+ { gold_assert(reg < 0xf); }
No need to use gold_assert as make_arm_v4bx already has an assertion.
+
+ friend class Stub_factory;
+
+ // Return the relocation target address of the i-th relocation in the
+ // stub.
+ Arm_address
+ do_reloc_target(size_t)
+ { return NULL; }
^^^ Arm_address is a scalar type. Please do a "return 0;" or better
"gold_unreachable();" since V4BX stub does not contain any relocation.
+ private:
+ // A template to implement do_write.
+ template<bool big_endian>
+ void inline
+ do_fixed_endian_v4bx_write(unsigned char* view, section_size_type)
+ {
+ const Insn_template* insns = this->stub_template()->insns();
+ elfcpp::Swap<32, big_endian>::writeval(view, insns[0].data() +
+ (this->reg_ << 16));
I find the indentation a bit strange. If Ian is okay with it, it is fine.
+
+ view += insns[0].size();
+ elfcpp::Swap<32, big_endian>::writeval(view, insns[1].data() + this->reg_);
+ view += insns[1].size();
+ elfcpp::Swap<32, big_endian>::writeval(view, insns[2].data() + this->reg_);
+ }
Most, if not all uses of elfcpp::Swap::writeval in gold use the correct
pointer type. Perhaps you need to add an reinterpret_cast? Ian?
@@ -818,7 +885,11 @@
// Whether this stub table is empty.
bool
empty() const
- { return this->reloc_stubs_.empty() && this->cortex_a8_stubs_.empty(); }
+ {
+ return this->reloc_stubs_.empty()
+ && this->cortex_a8_stubs_.empty()
+ && this->arm_v4bx_stubs_.empty();
+ }
Add a pair of parentheses so that the expression indents nicely with emacs :^)
return (this->reloc ...
&& this->..
&& this->..);
// Return the current data size.
off_t
@@ -845,6 +916,16 @@
this->cortex_a8_stubs_.insert(value);
}
+ // Add an ARM V4BX relocation stub. A register index will be retrieved
+ // from the stub.
+ void
+ add_arm_v4bx_stub(Arm_v4bx_stub* stub)
+ {
+ gold_assert(stub);
+ if (this->find_arm_v4bx_stub(stub->reg()) == NULL)
+ this->arm_v4bx_stubs_[stub->reg()] = stub;
+ }
+
If we add a stub for a register that already has a stub, we are doing something
wrong. Please change this to
{
gold_assert(stub != NULL && this->arm_vb4x_stubs_[stub->reg()] == NULL);
this->arm_v4bx_stubs_[stub->reg()] = stub;
}
// Remove all Cortex-A8 stubs.
void
remove_all_cortex_a8_stubs();
@@ -857,6 +938,15 @@
return (p != this->reloc_stubs_.end()) ? p->second : NULL;
}
+ // Look up an arm v4bx relocation stub using the register index.
+ // Return NULL if there is none.
+ Arm_v4bx_stub*
+ find_arm_v4bx_stub(const uint32_t reg) const
+ {
+ gold_assert(reg < 0xf);
+ return this->arm_v4bx_stubs_[reg];
+ }
+
// Relocate stubs in this stub table.
void
relocate_stubs(const Relocate_info<32, big_endian>*,
@@ -1073,7 +1167,8 @@
Arm_relobj(const std::string& name, Input_file* input_file, off_t offset,
const typename elfcpp::Ehdr<32, big_endian>& ehdr)
: Sized_relobj<32, big_endian>(name, input_file, offset, ehdr),
- stub_tables_(), local_symbol_is_thumb_function_(),
+ stub_tables_(),
+ local_symbol_is_thumb_function_(),
attributes_section_data_(NULL), mapping_symbols_info_(),
section_has_cortex_a8_workaround_(NULL)
{ }
There is no need for this unless you really don't like the formatting :)
@@ -2745,6 +2850,51 @@
elfcpp::Swap<16, big_endian>::writeval(wv + 1, val & 0xffff);
return This::STATUS_OKAY;
}
+
+ // R_ARM_V4BX
+ static inline typename This::Status
+ v4bx(const Relocate_info<32, big_endian>* relinfo,
+ unsigned char *view,
+ const Arm_relobj<big_endian>* object,
+ const Arm_address address,
+ const bool is_interworking)
+ {
+
+ typedef typename elfcpp::Swap<32, big_endian>::Valtype Valtype;
+ Valtype* wv = reinterpret_cast<Valtype*>(view);
+ Valtype val = elfcpp::Swap<32, big_endian>::readval(wv);
+
+ // Ensure that we have a BX instruction.
+ gold_assert((val & 0x0ffffff0) == 0x012fff10);
+ const uint32_t reg = (val & 0xf);
+ if (is_interworking && reg != 0xf)
+ {
+ Stub_table<big_endian>* stub_table =
+ object->stub_table(relinfo->data_shndx);
+
+ if (stub_table == NULL)
+ return This::STATUS_BAD_RELOC;
Change this to gold_assert. This should not happend.
+
+ Arm_v4bx_stub* stub = stub_table->find_arm_v4bx_stub(reg);
+ gold_assert(stub != NULL);
+
+ int32_t veneer_address =
+ stub_table->address() + stub->offset() - 8 - address;
+ gold_assert((veneer_address <= ARM_MAX_FWD_BRANCH_OFFSET)
+ && (veneer_address >= ARM_MAX_BWD_BRANCH_OFFSET));
+ // Replace with a branch to veneer (B <addr>)
+ val = (val & 0xf0000000) | 0x0a000000
+ | ((veneer_address >> 2) & 0x00ffffff);
+ }
+ else
+ {
+ // Preserve Rm (lowest four bits) and the condition code
+ // (highest four bits). Other bits encode MOV PC,Rm.
+ val = (val & 0xf000000f) | 0x01a0f000;
+ }
+ elfcpp::Swap<32, big_endian>::writeval(wv, val);
+ return This::STATUS_OKAY;
+ }
};
// Relocate ARM long branches. This handles relocation types
@@ -3676,6 +3826,15 @@
Insn_template::arm_rel_insn(0xea000000, -8) // b dest
};
+ // Stub used to provide an interworking for R_ARM_V4BX relocation
+ // (bx r[n] instruction).
+ static const Insn_template elf32_arm_stub_v4_veneer_bx[] =
+ {
+ Insn_template::arm_insn(0xe3100001), // tst r<n>, #1
+ Insn_template::arm_insn(0x01a0f000), // moveq pc, r<n>
+ Insn_template::arm_insn(0xe12fff10) // bx r<n>
+ };
+
// Fill in the stub template look-up table. Stub templates are constructed
// per instance of Stub_factory for fast look-up without locking
// in a thread-enabled environment.
@@ -3770,6 +3929,16 @@
++p)
this->relocate_stub(p->second, relinfo, arm_target, output_section, view,
address, view_size);
+
+ // Relocate all ARM V4BX stubs.
+ for (Arm_v4bx_stub_list::iterator p = this->arm_v4bx_stubs_.begin();
+ p != this->arm_v4bx_stubs_.end();
+ ++p)
+ {
+ if (*p != NULL)
+ this->relocate_stub(*p, relinfo, arm_target, output_section, view,
+ address, view_size);
+ }
}
// Write out the stubs to file.
@@ -3811,6 +3980,22 @@
big_endian);
}
+ // Write ARM V4BX relocation stubs.
+ for (Arm_v4bx_stub_list::const_iterator p = this->arm_v4bx_stubs_.begin();
+ p != this->arm_v4bx_stubs_.end();
+ ++p)
+ {
+ if (*p == NULL)
+ continue;
+
+ Arm_address address = this->address() + (*p)->offset();
+ gold_assert(address
+ == align_address(address,
+ (*p)->stub_template()->alignment()));
+ (*p)->write(oview + (*p)->offset(), (*p)->stub_template()->size(),
+ big_endian);
+ }
+
of->write_output_view(this->offset(), oview_size, oview);
}
@@ -3847,6 +4032,19 @@
+ stub_template->size());
}
+ for (Arm_v4bx_stub_list::const_iterator p = this->arm_v4bx_stubs_.begin();
+ p != this->arm_v4bx_stubs_.end();
+ ++p)
+ {
+ if (*p == NULL)
+ continue;
+
+ const Stub_template* stub_template = (*p)->stub_template();
+ addralign = std::max(addralign, stub_template->alignment());
+ size = (align_address(size, stub_template->alignment())
+ + stub_template->size());
+ }
+
// Check if either data size or alignment changed in this pass.
// Update prev_data_size_ and prev_addralign_. These will be used
// as the current data size and address alignment for the next pass.
@@ -3898,6 +4096,20 @@
arm_relobj->mark_section_for_cortex_a8_workaround(stub->shndx());
}
+ for (Arm_v4bx_stub_list::const_iterator p = this->arm_v4bx_stubs_.begin();
+ p != this->arm_v4bx_stubs_.end();
+ ++p)
+ {
+ if (*p == NULL)
+ continue;
+
+ const Stub_template* stub_template = (*p)->stub_template();
+ uint64_t stub_addralign = stub_template->alignment();
+ off = align_address(off, stub_addralign);
+ (*p)->set_offset(off);
+ off += stub_template->size();
+ }
+
gold_assert(off <= this->prev_data_size_);
}
@@ -5603,6 +5817,13 @@
|| cpu_arch_profile_attr->int_value() == 0));
}
+ // Check if we can use V4BX interworking.
+ // The V4BX interworking stub contains BX instruction,
+ // which is not specified for some profiles.
+ if (this->fix_v4bx() == 2 && !this->may_use_blx())
+ gold_error("unable to provide V4BX reloc interworking fix up. "
+ "The target profile does not support BX instruction.");
+
You need to use the _(...) macro here to enclose the error message. Please
look at other use of gold_error.
// Fill in some more dynamic tags.
const Reloc_section* rel_plt = (this->plt_ == NULL
? NULL
@@ -6090,6 +6311,15 @@
address, thumb_bit);
break;
+ case elfcpp::R_ARM_V4BX:
+ if (target->fix_v4bx() == 1
+ || !target->may_use_blx()
+ || (target->fix_v4bx() == 2 && target->may_use_blx()))
+ reloc_status =
+ Arm_relocate_functions::v4bx(relinfo, view, object, address,
+ (target->fix_v4bx() == 2));
+ break;
This is different from what ld is doing. It is sufficient to just check
target fix_v4bx() > 0. If someone tries to use --fix-v4bx-interworking
for a target not supporting BLX, there is already an error.
case elfcpp::R_ARM_TARGET1:
// This should have been mapped to another type already.
// Fall through.
@@ -7382,6 +7613,28 @@
const Arm_relobj<big_endian>* arm_relobj =
Arm_relobj<big_endian>::as_arm_relobj(relinfo->object);
+ if (r_type == elfcpp::R_ARM_V4BX)
+ {
+ const uint32_t reg = (addend & 0xf);
+ if (this->fix_v4bx() == 2 && reg < 0xf)
+ {
+ // Try looking up an existing stub from a stub table.
+ Stub_table<big_endian>* stub_table =
+ arm_relobj->stub_table(relinfo->data_shndx);
If we cannot find the stub table, something is wrong in gold. Change this to
gold_assert(stub_table != NULL);
if (stub_table->find_arm_v4bx_stub(reg) == NULL)
...
+
+ if (stub_table && stub_table->find_arm_v4bx_stub(reg) == NULL)
+ {
+ // create a new stub and add it to stub table.
+ Arm_v4bx_stub* stub =
+ this->stub_factory().make_arm_v4bx_stub(reg);
+ gold_assert(stub != NULL);
+ stub_table->add_arm_v4bx_stub(stub);
+ }
+ }
+
+ return;
+ }
+
bool target_is_thumb;
Symbol_value<32> symval;
if (gsym != NULL)