Re: [RFAv2] Fix buffer overflow regression due to minsym malloc-ed instead of obstack-ed.

[Simon Marchi <simark@simark.ca>]

On 2019-03-25 3:54 p.m., Philippe Waroquiers wrote:
> On Mon, 2019-03-25 at 09:31 -0600, Tom Tromey wrote:
>> Philippe> +  int n_after_msymbol = minsym.objfile->per_bfd->minimal_symbol_count
>> Philippe> +    - (msymbol - minsym.objfile->per_bfd->msymbols.get ())
>> Philippe> +    - 1;
>>
>> What do you think of the appended instead?
>> The idea is to make the last element more explicit.
> Yes, that looks better, 2 minor comments below.

I just wanted to mention that I just hit this bug, and that Tom's patch fixes it for me.

>> @@ -1499,21 +1498,24 @@ minimal_symbol_upper_bound (struct bound_minimal_symbol minsym)
>>       other sections, to find the next symbol in this section with a
>>       different address.  */
>>  
>> +  struct minimal_symbol *last
>> +    = (minsym.objfile->per_bfd->msymbols.get ()
>> +       + minsym.objfile->per_bfd->minimal_symbol_count);
> Are the parenthesis needed here ?

It is mentioned here, search for "extra paren":

https://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting

It's just there to please people who use Emacs :).

> Also, I find the name 'last' a little bit confusing,
> as in the loop below, last is not handled.
> Maybe last could be the 'real' last i.e. as:
>    minsym.objfile->per_bfd->msymbols.get () +       
>     + minsym.objfile->per_bfd->minimal_symbol_count - 1;
> 
> and have the '< last' conditions below then be '<= last'.
> 
> That makes more clear for me that we handle the last
> element of the array.

This, or name the variable "past_the_end" or something like that.

Simon