Bug 15662 - gold (powerpc) internal error in do_relax() at gold/output.h:436
Summary: gold (powerpc) internal error in do_relax() at gold/output.h:436
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Cary Coutant
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-21 17:32 UTC by Jing Yu
Modified: 2013-06-27 23:23 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
Project(s) to access:
ssh public key:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jing Yu 2013-06-21 17:32:50 UTC
I found a few link time failures, reporting

internal error in set_current_data_size_for_child, at /usr/local/home/jingyu/opensource/binutils_trunk/src/gold/output.h:436

I am using top of trunk gold.

I investigated the failure and found that the error happens when
set_current_data_size_for_child is called through
    this->rel_->add_relative(elfcpp::R_POWERPC_RELATIVE, this, off, to);
through
   Target_powerpc<size, big_endian>::do_relax().

I notice that the data size for the failing object has been changed a
few times during relaxation. Every time before it is updated,
this->is_data_size_valid_ will be set to false. And after data size is
updated, is_data_size_valid_ will be set to true. However, when the
data size is updated through add_relative, the flag
is_data_size_valid_ is not set to false beforehand, which triggers the
assertion failure. I think it is a bug.

To verify my thought, I made a patch on powerpc.cc, though I am not sure it is the proper way to fix. With the patch, the failing tests pass linking.

Index: powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.91
diff -r1.91 powerpc.cc
2631a2632
>       this->brlt_section_->reset_rel_data_size();
2638a2640
>       this->brlt_section_->finalize_rel_data_size();
3015a3018,3029
>   void
>   reset_rel_data_size()
>   {
>     this->rel_->reset_data_size();
>   }
>
>   void
>   finalize_rel_data_size()
>   {
>     this->rel_->finalize_data_size();
>   }
>

Is the patch the proper way to fix the bug? Thanks!
Comment 1 Cary Coutant 2013-06-21 18:37:37 UTC
> Index: powerpc.cc
> ===================================================================
> RCS file: /cvs/src/src/gold/powerpc.cc,v
> retrieving revision 1.91
> diff -r1.91 powerpc.cc
> 2631a2632
>>       this->brlt_section_->reset_rel_data_size();
> 2638a2640
>>       this->brlt_section_->finalize_rel_data_size();

This looks right to me, but Alan should confirm.

I'd suggest, instead of adding a second call at each point, replacing
the call to reset_data_size and finalize_data_size with calls to
reset_brlt_sizes and finalize_brlt_sizes, and have those functions
make both calls that are needed.

> 3015a3018,3029
>>   void
>>   reset_rel_data_size()
>>   {
>>     this->rel_->reset_data_size();
>>   }
>>
>>   void
>>   finalize_rel_data_size()
>>   {
>>     this->rel_->finalize_data_size();
>>   }

These member functions would then become:

  void
  reset_brlt_sizes()
  {
    this->reset_data_size();
    this->rel_->reset_data_size();
  }

  void
  finalize_brlt_sizes()
  {
    this->finalize_data_size();
    this->rel_->finalize_data_size();
  }

(By the way, in the future, please use 'diff -up' to generate your
patches -- it makes them much easier to read.)

-cary
Comment 2 Sourceware Commits 2013-06-27 23:20:36 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	ccoutant@sourceware.org	2013-06-27 23:20:35

Modified files:
	gold           : ChangeLog powerpc.cc 

Log message:
	PR gold/15662
	* powerpc.cc (Output_data_brlt_powerpc::reset_brlt_sizes): New
	function.
	(Output_data_brlt_powerpc::finalize_brlt_sizes): New function.
	(Target_powerpc::do_relax): Call the above.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/ChangeLog.diff?cvsroot=src&r1=1.1086&r2=1.1087
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/powerpc.cc.diff?cvsroot=src&r1=1.92&r2=1.93
Comment 3 Cary Coutant 2013-06-27 23:23:57 UTC
Fixed on trunk.