This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GOLD][PATCH][UPDATED] Added support for R_ARM_V4BX relocation (with interworking)


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)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]