Re: [PATCH] Fix buffer overflow in ada-lang.c:move_bits

[Pedro Alves <palves@redhat.com>]

On 11/08/2018 07:11 PM, Pedro Alves wrote:
> On 11/01/2018 03:35 PM, Joel Brobecker wrote:
>> Hi Tom,
>>
>>> -fsanitize=address showed that ada-lang.c:move_bits can run off the
>>> end of the source buffer.  I believe this patch fixes the problem, by
>>> arranging not to read from the source buffer once there are sufficient
>>> bits in the accumulator.
>>>
>>> gdb/ChangeLog
>>> 2018-10-23  Tom Tromey  <tom@tromey.com>
>>>
>>> 	* ada-lang.c (move_bits): Don't run off the end of the source
>>> 	buffer.
>>
>> Thanks for the patch!
>>
>> This is a part of the code that always forces me to think twice
>> (or ten times), each time I try to touch it. I should really start
>> adding comments to this code that detail what we are trying to do
>> as we do it.
>>
>> I tested your change through our testsuite on the various baremetal
>> targets we have, and noticed that it causes regressions on ppc and arm
>> targets. It's hopefully something small, but just being back from
>> a holiday, I'm a bit tied up at work; I'll put that issue on my TODO
>> list to look at further.
> 
> I was going to suggest that this would benefit from unit tests in
> the style of dwarf2read.c:copy_bitwise's, but, actually, isn't this
> exactly the same as copy_bitwise?  Can we get rid of ada-lang.c:move_bits?
> (And maybe move copy_bitwise elsewhere?)

I meant to say dwarf2loc.c instead of dwarf2read.c.

Thanks,
Pedro Alves