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: [PATCH] gold/arm: define $a/$d markers in .plt


> Target_arm<big_endian>::make_plt_entry(Symbol_table* symtab, Layout*
> layout,
> ? ? ? this->got_section(symtab, layout);
>
> ? ? ? this->plt_ = new Output_data_plt_arm<big_endian>(layout, this->got_plt_);
> - ? ? ?layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (elfcpp::SHF_ALLOC
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| elfcpp::SHF_EXECINSTR),
> +
> + ? ? ?Output_section* os = layout->add_output_section_data
> + ? ? ? (".plt", elfcpp::SHT_PROGBITS,
> + ? ? ? ?(elfcpp::SHF_ALLOC | elfcpp::SHF_EXECINSTR),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?this->plt_, ORDER_PLT, false);

We usually break long lines like this after the '=', and indent the
next line 4 spaces:

      Output_section* os =
          layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
                                          (elfcpp::SHF_ALLOC
                                           | elfcpp::SHF_EXECINSTR),
                                          this->plt_, ORDER_PLT, false);

> @@ -1919,6 +1945,8 @@ Symbol_table::do_define_in_output_data(
> ? ? ? ?this->force_local(sym);
> ? ? ? else if (version != NULL)
> ? ? ? ?sym->set_is_default();
> + ? ? ?if (binding == elfcpp::STB_LOCAL)
> + ? ? ? this->generated_locals_.push_back(sym);
> ? ? ? return sym;
> ? ? }

Given the change above this one, we will not have added any STB_LOCAL
symbols to the symbol table, so the force_local call is unnecessary in
that case. If the symbol is not STB_LOCAL, but
this->version_script_.symbol_is_local(name) is true, then we do still
need to call force_local. So perhaps this would work:

      if (binding == elfcpp::STB_LOCAL)
        this->generated_locals_.push_back(sym);
      else if (this->version_script_.symbol_is_local(name))
        this->force_local(sym);
      else if (version != NULL)
        sym->set_is_default();
      return sym;

For completeness, we should apply the same treatment to
do_define_in_output_segment and do_define_as_constant. It'd be OK with
me to defer that to a follow-up patch.

It looks good to me with those changes, but you still need Ian's
approval to commit.

-cary


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