Re: [PATCH][GOLD] Add ARM relocation property table.

[Ian Lance Taylor <iant at google dot com>]

"Doug Kwan (éæå)" <dougkwan@google.com> writes:

> 2010-02-02  Doug Kwan  <dougkwan@google.com>
>
>         * Makefile.am (HFILES): Add arm-reloc-property.h.
>         (DEFFILES): New.
>         (TARGETSOURCES): Add arm-reloc-property.cc
>         (ALL_TARGETOBJS): Add arm-reloc-property.$(OBJEXT)
>         (libgold_a_SOURCES): $(DEFFILES)
>         (arm-reloc-property.$(OBJEXT)): New dependency.
>         * Makefile.in: Regenerate.
>         * arm-reloc-property.cc: New file.
>         * arm-reloc-property.h: New file.
>         * arm-reloc.def: New file.
>         * arm.cc: Update comments.
>         (arm-reloc-property.h): New included header.
>         (arm_reloc_property_table): New global variable.


> +# Use an explicit dependency for the arm-reloc.def.
> +arm-reloc-property.$(OBJEXT): arm-reloc.def

This should not be necessary, as it brought in via #include and it is
not generated.  Check gold/.deps/arm-reloc-property.Po in your build
directory--arm-reloc.def should be in there.


> +Arm_reloc_property::Tree_node*
> +Arm_reloc_property::Tree_node::make_tree(const std::string& s)
> +{
> +  std::stack<size_t> size_stack;
> +  Tree_node_vector node_stack;
> +
> +  // strtok needs a non-cosnt string pointer.

s/cosnt/const/

> +// Constructor.  This processing informations in arm-reloc.def to
> +// initialize the table.
> +
> +Arm_reloc_property_table::Arm_reloc_property_table()
> +{
> +  // These appers in arm-reloc.def.  Do not rename them.
> +  Parse_expression A("A"), GOT_ORG("GOT_ORG"), NONE("NONE"), P("P"),
> +		   Pa("Pa"), S("S"), T("T"), TLS("TLS"), tp("tp");
> +  const bool Y(true), N(false);
> +
> +  for (unsigned int i = 0; i < Property_table_size; ++i)
> +    this->table_[i] = NULL;
> +
> +#undef RD
> +#define RD(name, type, deprecated, class, operation, is_implemented, \
> +	   group_index, checks_oveflow) \
> +  do \
> +    { \
> +      unsigned int code = elfcpp::R_ARM_##name; \
> +      gold_assert(code < Property_table_size); \
> +      this->table_[elfcpp::R_ARM_##name] = \

s/elfcpp::R_ARM_##name/code/

> +	new Arm_reloc_property(elfcpp::R_ARM_##name, "R_ARM_" #name, \
> +			       Arm_reloc_property::RT_##type, deprecated, \
> +			       Arm_reloc_property::RC_##class, \
> +			       (operation).s_expression(), is_implemented, \
> +			       group_index, checks_oveflow); \
> +    } \
> +  while(0);
> +
> +#include "arm-reloc.def"
> +#undef RD
> +}

I don't like the fact that we will pay in linker startup time for
every linker which includes the ARM target, even if we are not
linking ARM.  This is problematic because by default the linker
includes support for all targets.  Can we arrange for this to only be
constructed when the ARM target is being used?


> +    // Construct an internal node.  A node owns all its children and is
> +    // responsible for releasing them at its own destruction.
> +    Tree_node(Tree_node_vector::const_iterator begin,
> +	      Tree_node_vector::const_iterator end)
> +      : is_leaf_(false), name_(), children_()
> +    {
> +      for(Tree_node_vector::const_iterator p = begin; p != end; ++p)
> +	this->children_.push_back(*p);
> +    }

s/for(/for (/

> +
> +    ~Tree_node()
> +    {
> +      for(size_t i = 0; i <this->children_.size(); ++i)
> +	delete this->children_[i];
> +    }

Do we ever actually destroy a Tree_node structure?  If not, there is
no reason to have a destructor.

If you keep this, s/for(/for (/ and s/<this/< this/


> +  ~Arm_reloc_property_table()
> +  {
> +    for (size_t code = 0; code < Property_table_size; ++code)
> +      delete this->table_[code];
> +  }

Do we ever actually destroy an Arm_reloc_property_table?


This is a cute idea.  Can you think of a way to make it a bit more
general so that it is easier for other targets to use it?

You need to arrange to add arm-reloc-property.$(OBJEXT) to targetobjs
in gold/configure.ac when using --enable-target=arm.  Look at how we
set ${targ_obj}.\$(OBJEXT), where targ_obj is set in configure.tgt.

Ian