This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH] PR gold/20462: Fix bogus layout on ARM with linker script using PHDRS clause
- From: Cary Coutant <ccoutant at gmail dot com>
- To: Roland McGrath <mcgrathr at google dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Fri, 12 Aug 2016 09:54:04 -0700
- Subject: Re: [PATCH] PR gold/20462: Fix bogus layout on ARM with linker script using PHDRS clause
- Authentication-results: sourceware.org; auth=none
- References: <CAB=4xhqS30x5enyJLxtDVfv-EnbAeXHxDOB4jD5ThtAR2wxSag@mail.gmail.com>
> I'm not at all sure this fix is correct. It fixes both the minimal
> test case I put in the bug, and my real case (which uses the same
> linker script). It causes no regressions in 'make check-gold' in
> a trunk build with --target=arm-eabi.
> Here's what I figured out:
> * On the first pass, the layout comes out just fine.
> * ARM always tries relaxation (unless -r).
> * In Target_arm::do_relax it calls Target_arm::fix_exidx_coverage and
> decides that constituted some relaxation happening, so a second pass
> always happens when there are any .ARM.exidx sections.
> * In the second pass, Script_sections::expected_segment_count sees
> this->segments_created_ and short-circuits, so it returns 0 instead
> of this->phdrs_elements_->size().
> * Ergo, SIZEOF_HEADERS omits the phdrs in the second pass and
> addresses go backwards, making everything unhappy.
> Script_sections::expected_segment_count is the only thing that tests
> this->segments_created_. It's not really clear to me if this ought
> to be part of the state that Script_sections::release_segments (sole
> caller Layout::clean_up_after_relaxation) resets, as my change here
> does. An alternate fix would be to reverse the order of the first two
> checks in Script_sections::expected_segment_count so that it always
> returns this->phdrs_elements_->size() when this->saw_phdrs_clause().
> (I haven't tried that on my test cases.)
> If this is indeed a good fix, may I commit it to trunk and 2.27?
> If not, is my other suggestion right or is there a different fix?
Yes, I think this is the right fix. OK for trunk and 2.27 branch.