[Gold] .ehframe problem with --sort-section=name. PR gold/17005

[Gold] .ehframe problem with --sort-section=name. PR gold/17005

[Alexander Ivchenko]

Hi,

The attached patch fixes PR17005 by providing the correct fde offsets
to eh_frame_hdr and by
fixing the assumption that in Eh_frame::set_final_data_size
output_section is always equals to 0 in the beginning.

Is it ok?

diff --git a/gold/ChangeLog b/gold/ChangeLog
index a9dd87b..fcfaf07 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,14 @@
+2014-06-24  Alexander Ivchenko  <alexander.ivchenko@intel.com>
+
+ PR gold/17005
+ * ehframe.cc (Fde::write): When recording fde offsets, keep
+ in mind that offsets must be from the beginning of the eh_frame
+ section, not relative to the current oview.
+ (Eh_frame::set_final_data_size): Fix the assumtion that output_offset
+ is always equals to zero.
+ * ehframe.h (Eh_frame_hdr::get_ehframe_section): New method.
+
+
 2014-05-27  H.J. Lu  <hongjiu.lu@intel.com>

  PR gold/16945
diff --git a/gold/ehframe.cc b/gold/ehframe.cc
index 699073d..5414a35 100644
--- a/gold/ehframe.cc
+++ b/gold/ehframe.cc
@@ -389,7 +389,13 @@ Fde::write(unsigned char* oview,
section_offset_type offset,

   // Tell the exception frame header about this FDE.
   if (eh_frame_hdr != NULL)
-    eh_frame_hdr->record_fde(offset, fde_encoding);
+    {
+      section_offset_type offset_from_start_of_section = 0;
+      if (eh_frame_hdr->get_ehframe_section()->is_address_valid())
+ offset_from_start_of_section = address -
eh_frame_hdr->get_ehframe_section()->address();
+
+      eh_frame_hdr->record_fde(offset + offset_from_start_of_section,
fde_encoding);
+    }

   return offset + aligned_full_length;
 }
@@ -1093,7 +1099,12 @@ Eh_frame::set_final_data_size()
       return;
     }

+  // If the output section has file offset, we need to subtract it at this
+  // point from the offset of the currect input section.
   section_offset_type output_offset = 0;
+  if (this->output_section()->is_offset_valid()
+      && this->is_offset_valid())
+    output_offset = this->offset() - this->output_section()->offset();

   for (Unmergeable_cie_offsets::iterator p =
  this->unmergeable_cie_offsets_.begin();
@@ -1112,6 +1123,10 @@ Eh_frame::set_final_data_size()

   this->mappings_are_done_ = true;
   this->final_data_size_ = output_offset;
+  if (this->output_section()->is_offset_valid()
+      && this->is_offset_valid())
+    this->final_data_size_ -= this->output_section()->offset();
+

   gold_assert((output_offset & (this->addralign() - 1)) == 0);
   this->set_data_size(output_offset);
diff --git a/gold/ehframe.h b/gold/ehframe.h
index 42ed7f6..ffb9a4a 100644
--- a/gold/ehframe.h
+++ b/gold/ehframe.h
@@ -63,6 +63,13 @@ class Eh_frame_hdr : public Output_section_data
       this->fde_offsets_.push_back(std::make_pair(fde_offset, fde_encoding));
   }

+  // Return the .eh_frame section.
+ const Output_section*
+ get_ehframe_section()
+  {
+    return eh_frame_section_;
+  }
+
  protected:
   // Set the final data size.
   void


thanks,
Alexander

Re: [Gold] .ehframe problem with --sort-section=name. PR gold/17005

[Cary Coutant]

> The attached patch fixes PR17005 by providing the correct fde offsets
> to eh_frame_hdr and by
> fixing the assumption that in Eh_frame::set_final_data_size
> output_section is always equals to 0 in the beginning.
>
> Is it ok?

The problem actually goes a bit deeper than this. The .eh_frame
contribution from crtendS.o should not be sorted to the beginning --
it's an end sentinel for the .eh_frame section, and needs to stay at
the end. The problem is that when sorting input sections by name, the
computed contents from optimizing the .eh_frame input sections is
added to the output section as an Output_section_data with no name,
and gets sorted to the end. I'm working on an alternate fix that will
preserve the proper order when sorting.

Still, when there is an unoptimized section that comes, say, from
crt1.o or crtbeginS.o, it may be that the computed contents are placed
at a non-zero offset relative to the start of the output section, and
it is necessary to take that into account when adding the FDEs. See
also PR 14675 and the following discussion:

https://sourceware.org/ml/binutils/2014-03/msg00310.html

...continuing into April...

https://sourceware.org/ml/binutils/2014-04/msg00006.html

-cary

Re: [Gold] .ehframe problem with --sort-section=name. PR gold/17005

[Alexander Ivchenko]

I missed those discussions, thanks for pointing out. I'm looking
forward to your fix then :)
Please, also note, that PR17005 is present in 2.24.

thanks,
Alexander

2014-07-02 2:08 GMT+04:00 Cary Coutant <ccoutant@google.com>:
>> The attached patch fixes PR17005 by providing the correct fde offsets
>> to eh_frame_hdr and by
>> fixing the assumption that in Eh_frame::set_final_data_size
>> output_section is always equals to 0 in the beginning.
>>
>> Is it ok?
>
> The problem actually goes a bit deeper than this. The .eh_frame
> contribution from crtendS.o should not be sorted to the beginning --
> it's an end sentinel for the .eh_frame section, and needs to stay at
> the end. The problem is that when sorting input sections by name, the
> computed contents from optimizing the .eh_frame input sections is
> added to the output section as an Output_section_data with no name,
> and gets sorted to the end. I'm working on an alternate fix that will
> preserve the proper order when sorting.
>
> Still, when there is an unoptimized section that comes, say, from
> crt1.o or crtbeginS.o, it may be that the computed contents are placed
> at a non-zero offset relative to the start of the output section, and
> it is necessary to take that into account when adding the FDEs. See
> also PR 14675 and the following discussion:
>
> https://sourceware.org/ml/binutils/2014-03/msg00310.html
>
> ...continuing into April...
>
> https://sourceware.org/ml/binutils/2014-04/msg00006.html
>
> -cary

Re: [Gold] .ehframe problem with --sort-section=name. PR gold/17005

[Cary Coutant]

I've committed a fix for this issue:

   https://sourceware.org/ml/binutils/2014-09/msg00011.html

Thanks for your investigation and proposed patches!

-cary


On Wed, Jul 2, 2014 at 6:18 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
> I missed those discussions, thanks for pointing out. I'm looking
> forward to your fix then :)
> Please, also note, that PR17005 is present in 2.24.
>
> thanks,
> Alexander
>
> 2014-07-02 2:08 GMT+04:00 Cary Coutant <ccoutant@google.com>:
>>> The attached patch fixes PR17005 by providing the correct fde offsets
>>> to eh_frame_hdr and by
>>> fixing the assumption that in Eh_frame::set_final_data_size
>>> output_section is always equals to 0 in the beginning.
>>>
>>> Is it ok?
>>
>> The problem actually goes a bit deeper than this. The .eh_frame
>> contribution from crtendS.o should not be sorted to the beginning --
>> it's an end sentinel for the .eh_frame section, and needs to stay at
>> the end. The problem is that when sorting input sections by name, the
>> computed contents from optimizing the .eh_frame input sections is
>> added to the output section as an Output_section_data with no name,
>> and gets sorted to the end. I'm working on an alternate fix that will
>> preserve the proper order when sorting.
>>
>> Still, when there is an unoptimized section that comes, say, from
>> crt1.o or crtbeginS.o, it may be that the computed contents are placed
>> at a non-zero offset relative to the start of the output section, and
>> it is necessary to take that into account when adding the FDEs. See
>> also PR 14675 and the following discussion:
>>
>> https://sourceware.org/ml/binutils/2014-03/msg00310.html
>>
>> ...continuing into April...
>>
>> https://sourceware.org/ml/binutils/2014-04/msg00006.html
>>
>> -cary