This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix buffer overflow in ada-lang.c:move_bits
- From: Pedro Alves <palves at redhat dot com>
- To: Joel Brobecker <brobecker at adacore dot com>, Tom Tromey <tom at tromey dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 8 Nov 2018 19:12:47 +0000
- Subject: Re: [PATCH] Fix buffer overflow in ada-lang.c:move_bits
- References: <20181024162037.21024-1-tom@tromey.com> <20181101153517.GA2705@adacore.com> <082f66ea-223b-52f4-bc24-b274dbcf4f01@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