Re: [gold] Error out when there is an access beyond the end of the merged section

[Cary Coutant <ccoutant at google 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