This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold] Error out when there is an access beyond the end of the merged section
- From: Cary Coutant <ccoutant at google dot com>
- To: Alexander Ivchenko <aivchenk at gmail dot com>
- Cc: binutils <binutils at sourceware dot org>
- Date: Tue, 5 Aug 2014 10:13:27 -0700
- Subject: Re: [gold] Error out when there is an access beyond the end of the merged section
- Authentication-results: sourceware.org; auth=none
- References: <CACysShj9wK3e1eC56TffaOQknD4A8XF3NVcyYUNAKEPOux2E8w at mail dot gmail dot com> <CAHACq4rvh5Uv_tXqwgNTrir5zBB_Vi+sO1tYhLYWkWsQdi+-Sg at mail dot gmail dot com> <CACysShgkDLZWzZZtZFwandLGztSTqNkZEniXcyQDWNxE4Cj2Vg at mail dot gmail dot com>
> I think you are right that it would be better to give an error from
> Merged_symbol_value::value_from_output_section, but I think that just
> replacing gold_assert is wrong:
>
> Here is how "found" is initialized:
> bool found = object->merge_map()->get_output_offset(NULL, input_shndx,
> input_offset,
> &output_offset,
> object);
>
> And Object_merge_map::get_output_offset can return false in three cases:
>
> 1)
> if (map == NULL
> || (merge_map != NULL && map->merge_map != merge_map))
> return false;
>
> 2)
> std::vector<Input_merge_entry>::const_iterator p =
> std::lower_bound(map->entries.begin(), map->entries.end(),
> entry, Input_merge_compare());
> if (p == map->entries.end() || p->input_offset > input_offset)
> {
> if (p == map->entries.begin())
> return false;
> --p;
> gold_assert(p->input_offset <= input_offset);
> }
>
> and
>
> 3)
> if (input_offset - p->input_offset
> >= static_cast<section_offset_type>(p->length))
> return false;
>
> AFAIU, for cases 1) and 2) the gold_assert in
> Merged_symbol_value::value_from_output_section should really be hit,
> and only 3) should give us an error with "access beyond end of merged
> section".
>
> So, I think, we should move gold_assert to 1), 2) in order to catch
> that situation.
I don't think that Object_merge_map::get_output_offset should assert
on these conditions -- there are other callers that may legitimately
expect a false return for either of these reasons. In the case of
Merged_symbol_value::value_from_output_section(), I think that we've
gone long enough without ever hitting that assert for either of
reasons (1) or (2) that it's safe to assume that a false return is due
to reason (3).
> There is an unfortunate necessity of const_cast, if we want to print
> out the name of the section.
> What do you think?
We really should be able to call section_name() without a const_cast.
I'll commit a patch to fix that shortly, then you can update this
patch.
Thanks, and sorry for the delay. I was on vacation, and am catching up.
-cary