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
On 11/14/2018 05:11 PM, Joel Brobecker wrote:
> Hello,
>
>>>> 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.
>> It does look exactly the same, doesn't it? I'll see if we can just
>> re-use dwarf2loc's copy_bitwise. Thanks for the suggestion!
> How about the attached? I ran it through AdaCore's testsuite on
> all the platforms we support as well as the official testsuite on
> x86_64-linux. No regression.
>
> gdb/ChangeLog:
>
> * ada-lang.c (move_bits): Delete. Update all callers to use
> copy_bitwise instead.
> * dwarf2loc.c (copy_bitwise, bits_to_str::bits_to_str)
> (selftests::check_copy_bitwise, selftests::copy_bitwise_tests):
> Move from here to utils.c.
> (_initialize_dwarf2loc): Remove call to register copy_bitwise
> selftests.
> * utils.h (copy_bitwise): Add declaration.
> * utils.c (copy_bitwise, bits_to_str::bits_to_str)
> (selftests::check_copy_bitwise, selftests::copy_bitwise_tests):
> Moved here from dwarf2loc.c.
> (_initialize_utils): Register copy_bitwise selftests.
>
> Thank you!
> -- Joel
>
>
Great, thanks!
Nit, since the function is now public, I'd consider moving the unit
tests to under gdb/unittests/ instead, like, to a new
copy_bitwise-selftests.c file. (I'm mildly thinking that'd be a better
filename than utils-selftest.c because the function may well
move again in the future. Notice how gdb_realpath's unit tests
were left behind in gdb/utils.c even though gdb_realpath moved to
common/pathstuff.c.)
If you do that, you can drop the
'#if GDB_SELF_TEST' around the tests, since files in that
directory are not compiled if unit tests are disabled.
Regardless, LGTM.
Thanks,
Pedro Alves