PATCH: PR gold/14675: No eh_frame info registered in exception_static_test
Cary Coutant
ccoutant@google.com
Wed Jan 7 18:50:00 GMT 2015
Thanks for looking at this, but I find this a bit much for what it
does -- it's pretty intrusive, and still doesn't even do the right
thing with the eh info from crt1.o (which is still placed before the
__EH_FRAME_BEGIN__ symbol, and will be ignored).
I really dislike special-cases based on filename. At one point in the
discussion, Roland suggested recognizing crtbeginT.o by the fact that
it has a relocation to the .eh_frame section symbol. I think it may be
even simpler than that, though -- given that the endcap in crtend.o
has a non-zero length, I think it's safe to put all zero-length
.eh_frame sections in front of the optimized data. That should have
the desired affect for crtbeginT.o. Let me give that a try.
-cary
On Wed, Jan 7, 2015 at 6:43 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 07, 2015 at 05:16:55AM -0800, H.J. Lu wrote:
>> On Mon, Dec 22, 2014 at 09:37:38AM -0800, Cary Coutant wrote:
>> > > Here is an idea. Does it look OK?
>> >
>> > I don't like the idea of making a file named "crtbegin" magic, and
>> > with this patch, any link *without* such a file will no longer
>> > optimize eh data.
>> >
>>
>> Here is a different approach. We always optimize eh data unless we
>> see crti followed by crtbeginT. If it is true, we start to optimize eh
>> data only when we see crtbeginT. It only leaves any eh data before
>> crtbeginT unoptimized. This patch changes the output eh data only when
>> there is crti followed by crtbeginT. OK for trunk and 2.25 branch?
>>
>> Thanks.
>>
>>
>
> Small update. Set has_crtbeginT_ to true only if it is false.
>
> H.J.
> --
> From 081ff0b8a64fa33a2fbd8b900f31228138400438 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Sat, 20 Dec 2014 05:45:51 -0800
> Subject: [PATCH] Treat .eh_frame section before crtbeginT as normal input
>
> Force the exception frame section from input files before the crtbeginT
> file to be handled as an ordinary input section if we aren't creating
> the exception frame header. If we don't do this, we won't correctly
> handle the special marker symbol in the exception frame section in the
> crtbeginT file.
>
> PR gold/14675
> * ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
> exception frame section from input files if it can't be
> optimized.
> (Eh_frame::add_ehframe_input_section<32, false>): Updated.
> (Eh_frame::add_ehframe_input_section<32, true>): Likewise.
> (Eh_frame::add_ehframe_input_section<64, false>): Likewise.
> (Eh_frame::add_ehframe_input_section<64, true>): Likewise.
> * ehframe.h (Eh_frame::add_ehframe_input_section): Add a
> bool parameter to indicate if the exception frame section
> can be optimized.
> * layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
> !has_crtbeginT.
> (Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
> Eh_frame::add_ehframe_input_section.
> (Layout::make_eh_frame_section): Set this->optimize_ehframe_
> to true when processing the crtbeginT file if it is on command
> line.
> (Layout::match_file_name (const char*, const char*)): New.
> (Layout::match_file_name(const Relobj*, const char*): Use it.
> * layout.h (Layout::Layout): Add has_crtbeginT.
> (Layout::match_file_name (const char*, const char*)): New.
> (Layout): Add an optimize_ehframe_ member.
> * main.cc (main): Pass command_line.has_crtbeginT() to layout.
> * options.cc: Include "layout.h".
> (Input_arguments::add_file): Set this->has_crtbeginT_ to true
> if there is a crtbeginT file and the last one is a crti file.
> * options.h (Input_arguments::Input_arguments): Initialize
> has_crtbeginT_ and last_is_crti_ to false.
> (Input_arguments::has_crtbeginT): New function.
> (Input_arguments::has_crtbeginT_): New bool member.
> (Input_arguments::last_is_crti_): Likewise.
> (Command_line::has_crtbeginT): New function.
> ---
> gold/ChangeLog | 36 ++++++++++++++++++++++++++++++++++++
> gold/ehframe.cc | 31 +++++++++++++++++++++----------
> gold/ehframe.h | 6 ++++--
> gold/layout.cc | 37 +++++++++++++++++++++++++++++--------
> gold/layout.h | 10 +++++++++-
> gold/main.cc | 3 ++-
> gold/options.cc | 14 ++++++++++++++
> gold/options.h | 17 ++++++++++++++++-
> 8 files changed, 131 insertions(+), 23 deletions(-)
>
> diff --git a/gold/ChangeLog b/gold/ChangeLog
> index 93529fe..d2ad829 100644
> --- a/gold/ChangeLog
> +++ b/gold/ChangeLog
> @@ -1,3 +1,39 @@
> +2015-01-07 H.J. Lu <hongjiu.lu@intel.com>
> +
> + PR gold/14675
> + * ehframe.cc (Eh_frame::add_ehframe_input_section): Force the
> + exception frame section from input files if it can't be
> + optimized.
> + (Eh_frame::add_ehframe_input_section<32, false>): Updated.
> + (Eh_frame::add_ehframe_input_section<32, true>): Likewise.
> + (Eh_frame::add_ehframe_input_section<64, false>): Likewise.
> + (Eh_frame::add_ehframe_input_section<64, true>): Likewise.
> + * ehframe.h (Eh_frame::add_ehframe_input_section): Add a
> + bool parameter to indicate if the exception frame section
> + can be optimized.
> + * layout.cc (Layout::Layout): Initialize optimize_ehframe_ to
> + !has_crtbeginT.
> + (Layout::layout_eh_frame): Pass this->optimize_ehframe_ to
> + Eh_frame::add_ehframe_input_section.
> + (Layout::make_eh_frame_section): Set this->optimize_ehframe_
> + to true when processing the crtbeginT file if it is on command
> + line.
> + (Layout::match_file_name (const char*, const char*)): New.
> + (Layout::match_file_name(const Relobj*, const char*): Use it.
> + * layout.h (Layout::Layout): Add has_crtbeginT.
> + (Layout::match_file_name (const char*, const char*)): New.
> + (Layout): Add an optimize_ehframe_ member.
> + * main.cc (main): Pass command_line.has_crtbeginT() to layout.
> + * options.cc: Include "layout.h".
> + (Input_arguments::add_file): Set this->has_crtbeginT_ to true
> + if there is a crtbeginT file and the last one is a crti file.
> + * options.h (Input_arguments::Input_arguments): Initialize
> + has_crtbeginT_ and last_is_crti_ to false.
> + (Input_arguments::has_crtbeginT): New function.
> + (Input_arguments::has_crtbeginT_): New bool member.
> + (Input_arguments::last_is_crti_): Likewise.
> + (Command_line::has_crtbeginT): New function.
> +
> 2015-01-06 H.J. Lu <hongjiu.lu@intel.com>
> Cary Coutant <ccoutant@google.com>
>
> diff --git a/gold/ehframe.cc b/gold/ehframe.cc
> index faea1a8..2fee0ff 100644
> --- a/gold/ehframe.cc
> +++ b/gold/ehframe.cc
> @@ -576,7 +576,8 @@ Eh_frame::add_ehframe_input_section(
> section_size_type symbol_names_size,
> unsigned int shndx,
> unsigned int reloc_shndx,
> - unsigned int reloc_type)
> + unsigned int reloc_type,
> + bool optimize_ehframe)
> {
> // Get the section contents.
> section_size_type contents_len;
> @@ -595,15 +596,21 @@ Eh_frame::add_ehframe_input_section(
> return false;
>
> New_cies new_cies;
> - if (!this->do_add_ehframe_input_section(object, symbols, symbols_size,
> + bool recognized_eh_frame_section
> + = this->do_add_ehframe_input_section(object, symbols, symbols_size,
> symbol_names, symbol_names_size,
> shndx, reloc_shndx,
> reloc_type, pcontents,
> - contents_len, &new_cies))
> + contents_len, &new_cies);
> + if (!recognized_eh_frame_section && this->eh_frame_hdr_ != NULL)
> + this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
> +
> + // If we don't unrecognize the exception frame section or it can't
> + // be optimized, then return false to force it to be handled as an
> + // ordinary input section.
> + if (!recognized_eh_frame_section
> + || (this->eh_frame_hdr_ == NULL && !optimize_ehframe))
> {
> - if (this->eh_frame_hdr_ != NULL)
> - this->eh_frame_hdr_->found_unrecognized_eh_frame_section();
> -
> for (New_cies::iterator p = new_cies.begin();
> p != new_cies.end();
> ++p)
> @@ -1224,7 +1231,8 @@ Eh_frame::add_ehframe_input_section<32, false>(
> section_size_type symbol_names_size,
> unsigned int shndx,
> unsigned int reloc_shndx,
> - unsigned int reloc_type);
> + unsigned int reloc_type,
> + bool optimize_ehframe);
> #endif
>
> #ifdef HAVE_TARGET_32_BIG
> @@ -1238,7 +1246,8 @@ Eh_frame::add_ehframe_input_section<32, true>(
> section_size_type symbol_names_size,
> unsigned int shndx,
> unsigned int reloc_shndx,
> - unsigned int reloc_type);
> + unsigned int reloc_type,
> + bool optimize_ehframe);
> #endif
>
> #ifdef HAVE_TARGET_64_LITTLE
> @@ -1252,7 +1261,8 @@ Eh_frame::add_ehframe_input_section<64, false>(
> section_size_type symbol_names_size,
> unsigned int shndx,
> unsigned int reloc_shndx,
> - unsigned int reloc_type);
> + unsigned int reloc_type,
> + bool optimize_ehframe);
> #endif
>
> #ifdef HAVE_TARGET_64_BIG
> @@ -1266,7 +1276,8 @@ Eh_frame::add_ehframe_input_section<64, true>(
> section_size_type symbol_names_size,
> unsigned int shndx,
> unsigned int reloc_shndx,
> - unsigned int reloc_type);
> + unsigned int reloc_type,
> + bool optimize_ehframe);
> #endif
>
> } // End namespace gold.
> diff --git a/gold/ehframe.h b/gold/ehframe.h
> index aa2bd31..73e0fd4 100644
> --- a/gold/ehframe.h
> +++ b/gold/ehframe.h
> @@ -371,7 +371,8 @@ class Eh_frame : public Output_section_data
> // is the relocation section if any (0 for none, -1U for multiple).
> // RELOC_TYPE is the type of the relocation section if any. This
> // returns whether the section was incorporated into the .eh_frame
> - // data.
> + // data. OPTIMIZE_EHFRAME is true if the exception frame section
> + // can be optimized.
> template<int size, bool big_endian>
> bool
> add_ehframe_input_section(Sized_relobj_file<size, big_endian>* object,
> @@ -380,7 +381,8 @@ class Eh_frame : public Output_section_data
> const unsigned char* symbol_names,
> section_size_type symbol_names_size,
> unsigned int shndx, unsigned int reloc_shndx,
> - unsigned int reloc_type);
> + unsigned int reloc_type,
> + bool optimize_ehframe);
>
> // Add a CIE and an FDE for a PLT section, to permit unwinding
> // through a PLT. The FDE data should start with 8 bytes of zero,
> diff --git a/gold/layout.cc b/gold/layout.cc
> index acc03b2..1ef4d18 100644
> --- a/gold/layout.cc
> +++ b/gold/layout.cc
> @@ -418,7 +418,8 @@ Layout_task_runner::run(Workqueue* workqueue, const Task* task)
>
> // Layout methods.
>
> -Layout::Layout(int number_of_input_files, Script_options* script_options)
> +Layout::Layout(int number_of_input_files, Script_options* script_options,
> + bool has_crtbeginT)
> : number_of_input_files_(number_of_input_files),
> script_options_(script_options),
> namepool_(),
> @@ -469,6 +470,7 @@ Layout::Layout(int number_of_input_files, Script_options* script_options)
> unique_segment_for_sections_specified_(false),
> incremental_inputs_(NULL),
> record_output_section_data_from_script_(false),
> + optimize_ehframe_(!has_crtbeginT),
> script_output_section_data_list_(),
> segment_states_(NULL),
> relaxation_debug_check_(NULL),
> @@ -1428,7 +1430,8 @@ Layout::layout_eh_frame(Sized_relobj_file<size, big_endian>* object,
> symbol_names_size,
> shndx,
> reloc_shndx,
> - reloc_type))
> + reloc_type,
> + this->optimize_ehframe_))
> {
> os->update_flags_for_input_section(shdr.get_sh_flags());
>
> @@ -1485,6 +1488,14 @@ Layout::make_eh_frame_section(const Relobj* object)
> if (os == NULL)
> return NULL;
>
> + // optimize_ehframe_ is false only if there is a crtbeginT file on
> + // command line. We can't optimize the exception frame section
> + // until we have seen the crtbeginT file.
> + if (!this->optimize_ehframe_
> + && object != NULL
> + && Layout::match_file_name(object, "crtbeginT"))
> + this->optimize_ehframe_ = true;
> +
> if (this->eh_frame_section_ == NULL)
> {
> this->eh_frame_section_ = os;
> @@ -5092,19 +5103,18 @@ Layout::output_section_name(const Relobj* relobj, const char* name,
> return name;
> }
>
> -// Return true if RELOBJ is an input file whose base name matches
> -// FILE_NAME. The base name must have an extension of ".o", and must
> -// be exactly FILE_NAME.o or FILE_NAME, one character, ".o". This is
> +// Return true if FILE is an input file whose base name matches
> +// MATCH. The base name must have an extension of ".o", and must
> +// be exactly MATCH.o or MATCH, one character, ".o". This is
> // to match crtbegin.o as well as crtbeginS.o without getting confused
> // by other possibilities. Overall matching the file name this way is
> // a dreadful hack, but the GNU linker does it in order to better
> // support gcc, and we need to be compatible.
>
> bool
> -Layout::match_file_name(const Relobj* relobj, const char* match)
> +Layout::match_file_name(const char* file, const char* match)
> {
> - const std::string& file_name(relobj->name());
> - const char* base_name = lbasename(file_name.c_str());
> + const char* base_name = lbasename(file);
> size_t match_len = strlen(match);
> if (strncmp(base_name, match, match_len) != 0)
> return false;
> @@ -5114,6 +5124,17 @@ Layout::match_file_name(const Relobj* relobj, const char* match)
> return memcmp(base_name + base_len - 2, ".o", 2) == 0;
> }
>
> +// Return true if FILE is an input file whose base name matches
> +// MATCH. The base name must have an extension of ".o", and must
> +// be exactly MATCH.o or MATCH, one character, ".o".
> +
> +bool
> +Layout::match_file_name(const Relobj* relobj, const char* match)
> +{
> + const std::string& file_name(relobj->name());
> + return Layout::match_file_name(file_name.c_str(), match);
> +}
> +
> // Check if a comdat group or .gnu.linkonce section with the given
> // NAME is selected for the link. If there is already a section,
> // *KEPT_SECTION is set to point to the existing section and the
> diff --git a/gold/layout.h b/gold/layout.h
> index aec0c53..bf4a44a 100644
> --- a/gold/layout.h
> +++ b/gold/layout.h
> @@ -488,7 +488,7 @@ enum Output_section_order
> class Layout
> {
> public:
> - Layout(int number_of_input_files, Script_options*);
> + Layout(int number_of_input_files, Script_options*, bool has_crtbeginT);
>
> ~Layout()
> {
> @@ -757,6 +757,12 @@ class Layout
> || strncmp(name, ".stab", sizeof(".stab") - 1) == 0);
> }
>
> + // Return true if FILE is an input file whose base name matches
> + // FILE_NAME. The base name must have an extension of ".o", and
> + // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
> + static bool
> + match_file_name(const char* file, const char* file_name);
> +
> // Return true if RELOBJ is an input file whose base name matches
> // FILE_NAME. The base name must have an extension of ".o", and
> // must be exactly FILE_NAME.o or FILE_NAME, one character, ".o".
> @@ -1420,6 +1426,8 @@ class Layout
> Incremental_inputs* incremental_inputs_;
> // Whether we record output section data created in script
> bool record_output_section_data_from_script_;
> + // Whether to optimize the exception frame section.
> + bool optimize_ehframe_;
> // List of output data that needs to be removed at relaxation clean up.
> Output_section_data_list script_output_section_data_list_;
> // Structure to save segment states before entering the relaxation loop.
> diff --git a/gold/main.cc b/gold/main.cc
> index c5e50d6..a3aea4b 100644
> --- a/gold/main.cc
> +++ b/gold/main.cc
> @@ -227,7 +227,8 @@ main(int argc, char** argv)
>
> // The layout object.
> Layout layout(command_line.number_of_input_files(),
> - &command_line.script_options());
> + &command_line.script_options(),
> + command_line.has_crtbeginT());
>
> if (layout.incremental_inputs() != NULL)
> layout.incremental_inputs()->report_command_line(argc, argv);
> diff --git a/gold/options.cc b/gold/options.cc
> index 7eb8f27..12c1aaa 100644
> --- a/gold/options.cc
> +++ b/gold/options.cc
> @@ -38,6 +38,7 @@
> #include "script.h"
> #include "target-select.h"
> #include "options.h"
> +#include "layout.h"
> #include "plugin.h"
>
> namespace gold
> @@ -1346,6 +1347,19 @@ Input_arguments::add_file(Input_file_argument& file)
> return this->input_argument_list_.back().lib()->add_file(file);
> }
> this->input_argument_list_.push_back(Input_argument(file));
> + if (!this->has_crtbeginT_)
> + {
> + // Set has_crtbeginT_ to true only if the last one is a crti file.
> + if (Layout::match_file_name(file.name(), "crti"))
> + this->last_is_crti_ = true;
> + else
> + {
> + if (this->last_is_crti_
> + && Layout::match_file_name(file.name(), "crtbeginT"))
> + this->has_crtbeginT_ = true;
> + this->last_is_crti_ = false;
> + }
> + }
> return this->input_argument_list_.back();
> }
>
> diff --git a/gold/options.h b/gold/options.h
> index 956a7f4..83bdd20 100644
> --- a/gold/options.h
> +++ b/gold/options.h
> @@ -1970,7 +1970,8 @@ class Input_arguments
> typedef Input_argument_list::const_iterator const_iterator;
>
> Input_arguments()
> - : input_argument_list_(), in_group_(false), in_lib_(false), file_count_(0)
> + : input_argument_list_(), in_group_(false), in_lib_(false),
> + file_count_(0), has_crtbeginT_(false), last_is_crti_(false)
> { }
>
> // Add a file.
> @@ -2030,11 +2031,20 @@ class Input_arguments
> number_of_input_files() const
> { return this->file_count_; }
>
> + // Return whether there is a crtbeginT file.
> + bool
> + has_crtbeginT() const
> + { return this->has_crtbeginT_; }
> +
> private:
> Input_argument_list input_argument_list_;
> bool in_group_;
> bool in_lib_;
> unsigned int file_count_;
> + // Whether there is a crtbeginT file.
> + bool has_crtbeginT_;
> + // Whether the last one is a crti file.
> + bool last_is_crti_;
> };
>
>
> @@ -2103,6 +2113,11 @@ class Command_line
> end() const
> { return this->inputs_.end(); }
>
> + // Whether there is a crtbeginT file.
> + bool
> + has_crtbeginT() const
> + { return this->inputs_.has_crtbeginT(); }
> +
> private:
> Command_line(const Command_line&);
> Command_line& operator=(const Command_line&);
> --
> 1.9.3
>
More information about the Binutils
mailing list