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

[Pedro Alves <palves@redhat.com>]

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?)

Thanks,
Pedro Alves