From patchwork Thu Nov 15 00:01:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 30148 Received: (qmail 52905 invoked by alias); 15 Nov 2018 00:02:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 52786 invoked by uid 89); 15 Nov 2018 00:02:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=UD:selftest.h, selftesth, selftest.h X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 15 Nov 2018 00:02:02 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id E9C6056127; Wed, 14 Nov 2018 19:02:00 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id kb5622L0JPo0; Wed, 14 Nov 2018 19:02:00 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 8806E560D6; Wed, 14 Nov 2018 19:02:00 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 2FA38849EA; Wed, 14 Nov 2018 16:01:58 -0800 (PST) Date: Wed, 14 Nov 2018 16:01:58 -0800 From: Joel Brobecker To: Pedro Alves Cc: Tom Tromey , gdb-patches@sourceware.org Subject: [RFA] Move copy_bitwise unittests to own unittest file (was: "Re: [PATCH] Fix buffer overflow in ada-lang.c:move_bits") Message-ID: <20181115000158.GC4336@adacore.com> References: <20181024162037.21024-1-tom@tromey.com> <20181101153517.GA2705@adacore.com> <082f66ea-223b-52f4-bc24-b274dbcf4f01@redhat.com> <609409e6-235a-4724-3ced-57c3ff42e299@redhat.com> <20181109171641.GD18647@adacore.com> <20181114171116.GA6074@adacore.com> <7628ab26-ad93-726c-f1d4-d5c60492115c@redhat.com> <20181114231724.GA4336@adacore.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181114231724.GA4336@adacore.com> User-Agent: Mutt/1.9.4 (2018-02-28) > > 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. > > I can do that. Since you said you're file reguardless, it's a little > easier for me to do it in two stages, so I'll go ahead and push this > one. I'll start on moving the unit tests again right after, and > will finish ASAP if it's not finished by end of today. Here it is: gdb/ChangeLog: * unittests/copy_bitwise-selftests.c: New file. * utils.c (selftests::bits_to_str, selftests::check_copy_bitwise) (selftests::copy_bitwise_tests): Delete, moving this code to unittests/copy_bitwise-selftests.c instead. (_initialize_utils): Do not register copy_bitwise tests. * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add unittests/copy_bitwise-selftests.c. Tested on x86_64-linux using the official testsuite, but also by verifying that "maintenance selftests" still runs the copy_bitwise tests. OK to push to master? Thank you! From b50e765dd1e317e09dee6082d6a9e6729c9f7542 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Wed, 14 Nov 2018 18:38:46 -0500 Subject: [PATCH] Move copy_bitwise unittests to own unittest file Now that copy_bitwise has been made public, and considering that its implementation could move to a different file again in the future, this patch moves its unittest to its own file in gdb/unittests. gdb/ChangeLog: * unittests/copy_bitwise-selftests.c: New file. * utils.c (selftests::bits_to_str, selftests::check_copy_bitwise) (selftests::copy_bitwise_tests): Delete, moving this code to unittests/copy_bitwise-selftests.c instead. (_initialize_utils): Do not register copy_bitwise tests. * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add unittests/copy_bitwise-selftests.c. Tested on x86_64-linux using the official testsuite, but also by verifying that "maintenance selftests" still runs the copy_bitwise tests. --- gdb/Makefile.in | 1 + gdb/unittests/copy_bitwise-selftests.c | 159 +++++++++++++++++++++++++++++++++ gdb/utils.c | 136 ---------------------------- 3 files changed, 160 insertions(+), 136 deletions(-) create mode 100644 gdb/unittests/copy_bitwise-selftests.c diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 6f74911..8bd78a6 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -408,6 +408,7 @@ SUBDIR_UNITTESTS_SRCS = \ unittests/array-view-selftests.c \ unittests/cli-utils-selftests.c \ unittests/common-utils-selftests.c \ + unittests/copy_bitwise-selftests.c \ unittests/environ-selftests.c \ unittests/format_pieces-selftests.c \ unittests/function-view-selftests.c \ diff --git a/gdb/unittests/copy_bitwise-selftests.c b/gdb/unittests/copy_bitwise-selftests.c new file mode 100644 index 0000000..af6e5b9 --- /dev/null +++ b/gdb/unittests/copy_bitwise-selftests.c @@ -0,0 +1,159 @@ +/* Self tests of the copy_bitwise routine for GDB, the GNU debugger. + + Copyright (C) 2018 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include "defs.h" +#include "selftest.h" +#include "utils.h" + +namespace selftests { + +/* Helper function for the unit test of copy_bitwise. Convert NBITS bits + out of BITS, starting at OFFS, to the respective '0'/'1'-string. MSB0 + specifies whether to assume big endian bit numbering. Store the + resulting (not null-terminated) string at STR. */ + +static void +bits_to_str (char *str, const gdb_byte *bits, ULONGEST offs, + ULONGEST nbits, int msb0) +{ + unsigned int j; + size_t i; + + for (i = offs / 8, j = offs % 8; nbits; i++, j = 0) + { + unsigned int ch = bits[i]; + for (; j < 8 && nbits; j++, nbits--) + *str++ = (ch & (msb0 ? (1 << (7 - j)) : (1 << j))) ? '1' : '0'; + } +} + +/* Check one invocation of copy_bitwise with the given parameters. */ + +static void +check_copy_bitwise (const gdb_byte *dest, unsigned int dest_offset, + const gdb_byte *source, unsigned int source_offset, + unsigned int nbits, int msb0) +{ + size_t len = align_up (dest_offset + nbits, 8); + char *expected = (char *) alloca (len + 1); + char *actual = (char *) alloca (len + 1); + gdb_byte *buf = (gdb_byte *) alloca (len / 8); + + /* Compose a '0'/'1'-string that represents the expected result of + copy_bitwise below: + Bits from [0, DEST_OFFSET) are filled from DEST. + Bits from [DEST_OFFSET, DEST_OFFSET + NBITS) are filled from SOURCE. + Bits from [DEST_OFFSET + NBITS, LEN) are filled from DEST. + + E.g., with: + dest_offset: 4 + nbits: 2 + len: 8 + dest: 00000000 + source: 11111111 + + We should end up with: + buf: 00001100 + DDDDSSDD (D=dest, S=source) + */ + bits_to_str (expected, dest, 0, len, msb0); + bits_to_str (expected + dest_offset, source, source_offset, nbits, msb0); + + /* Fill BUF with data from DEST, apply copy_bitwise, and convert the + result to a '0'/'1'-string. */ + memcpy (buf, dest, len / 8); + copy_bitwise (buf, dest_offset, source, source_offset, nbits, msb0); + bits_to_str (actual, buf, 0, len, msb0); + + /* Compare the resulting strings. */ + expected[len] = actual[len] = '\0'; + if (strcmp (expected, actual) != 0) + error (_("copy_bitwise %s != %s (%u+%u -> %u)"), + expected, actual, source_offset, nbits, dest_offset); +} + +/* Unit test for copy_bitwise. */ + +static void +copy_bitwise_tests (void) +{ + /* Data to be used as both source and destination buffers. The two + arrays below represent the lsb0- and msb0- encoded versions of the + following bit string, respectively: + 00000000 00011111 11111111 01001000 10100101 11110010 + This pattern is chosen such that it contains: + - constant 0- and 1- chunks of more than a full byte; + - 0/1- and 1/0 transitions on all bit positions within a byte; + - several sufficiently asymmetric bytes. + */ + static const gdb_byte data_lsb0[] = { + 0x00, 0xf8, 0xff, 0x12, 0xa5, 0x4f + }; + static const gdb_byte data_msb0[] = { + 0x00, 0x1f, 0xff, 0x48, 0xa5, 0xf2 + }; + + constexpr size_t data_nbits = 8 * sizeof (data_lsb0); + constexpr unsigned max_nbits = 24; + + /* Try all combinations of: + lsb0/msb0 bit order (using the respective data array) + X [0, MAX_NBITS] copy bit width + X feasible source offsets for the given copy bit width + X feasible destination offsets + */ + for (int msb0 = 0; msb0 < 2; msb0++) + { + const gdb_byte *data = msb0 ? data_msb0 : data_lsb0; + + for (unsigned int nbits = 1; nbits <= max_nbits; nbits++) + { + const unsigned int max_offset = data_nbits - nbits; + + for (unsigned source_offset = 0; + source_offset <= max_offset; + source_offset++) + { + for (unsigned dest_offset = 0; + dest_offset <= max_offset; + dest_offset++) + { + check_copy_bitwise (data + dest_offset / 8, + dest_offset % 8, + data + source_offset / 8, + source_offset % 8, + nbits, msb0); + } + } + } + + /* Special cases: copy all, copy nothing. */ + check_copy_bitwise (data_lsb0, 0, data_msb0, 0, data_nbits, msb0); + check_copy_bitwise (data_msb0, 0, data_lsb0, 0, data_nbits, msb0); + check_copy_bitwise (data, data_nbits - 7, data, 9, 0, msb0); + } +} + +} /* namespace selftests */ + +void +_initialize_copy_bitwise_utils_selftests () +{ + selftests::register_test ("copy_bitwise", selftests::copy_bitwise_tests); +} diff --git a/gdb/utils.c b/gdb/utils.c index c088d8b..0577e64 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -3312,141 +3312,6 @@ copy_bitwise (gdb_byte *dest, ULONGEST dest_offset, } } -#if GDB_SELF_TEST - -namespace selftests { - -/* Helper function for the unit test of copy_bitwise. Convert NBITS bits - out of BITS, starting at OFFS, to the respective '0'/'1'-string. MSB0 - specifies whether to assume big endian bit numbering. Store the - resulting (not null-terminated) string at STR. */ - -static void -bits_to_str (char *str, const gdb_byte *bits, ULONGEST offs, - ULONGEST nbits, int msb0) -{ - unsigned int j; - size_t i; - - for (i = offs / 8, j = offs % 8; nbits; i++, j = 0) - { - unsigned int ch = bits[i]; - for (; j < 8 && nbits; j++, nbits--) - *str++ = (ch & (msb0 ? (1 << (7 - j)) : (1 << j))) ? '1' : '0'; - } -} - -/* Check one invocation of copy_bitwise with the given parameters. */ - -static void -check_copy_bitwise (const gdb_byte *dest, unsigned int dest_offset, - const gdb_byte *source, unsigned int source_offset, - unsigned int nbits, int msb0) -{ - size_t len = align_up (dest_offset + nbits, 8); - char *expected = (char *) alloca (len + 1); - char *actual = (char *) alloca (len + 1); - gdb_byte *buf = (gdb_byte *) alloca (len / 8); - - /* Compose a '0'/'1'-string that represents the expected result of - copy_bitwise below: - Bits from [0, DEST_OFFSET) are filled from DEST. - Bits from [DEST_OFFSET, DEST_OFFSET + NBITS) are filled from SOURCE. - Bits from [DEST_OFFSET + NBITS, LEN) are filled from DEST. - - E.g., with: - dest_offset: 4 - nbits: 2 - len: 8 - dest: 00000000 - source: 11111111 - - We should end up with: - buf: 00001100 - DDDDSSDD (D=dest, S=source) - */ - bits_to_str (expected, dest, 0, len, msb0); - bits_to_str (expected + dest_offset, source, source_offset, nbits, msb0); - - /* Fill BUF with data from DEST, apply copy_bitwise, and convert the - result to a '0'/'1'-string. */ - memcpy (buf, dest, len / 8); - copy_bitwise (buf, dest_offset, source, source_offset, nbits, msb0); - bits_to_str (actual, buf, 0, len, msb0); - - /* Compare the resulting strings. */ - expected[len] = actual[len] = '\0'; - if (strcmp (expected, actual) != 0) - error (_("copy_bitwise %s != %s (%u+%u -> %u)"), - expected, actual, source_offset, nbits, dest_offset); -} - -/* Unit test for copy_bitwise. */ - -static void -copy_bitwise_tests (void) -{ - /* Data to be used as both source and destination buffers. The two - arrays below represent the lsb0- and msb0- encoded versions of the - following bit string, respectively: - 00000000 00011111 11111111 01001000 10100101 11110010 - This pattern is chosen such that it contains: - - constant 0- and 1- chunks of more than a full byte; - - 0/1- and 1/0 transitions on all bit positions within a byte; - - several sufficiently asymmetric bytes. - */ - static const gdb_byte data_lsb0[] = { - 0x00, 0xf8, 0xff, 0x12, 0xa5, 0x4f - }; - static const gdb_byte data_msb0[] = { - 0x00, 0x1f, 0xff, 0x48, 0xa5, 0xf2 - }; - - constexpr size_t data_nbits = 8 * sizeof (data_lsb0); - constexpr unsigned max_nbits = 24; - - /* Try all combinations of: - lsb0/msb0 bit order (using the respective data array) - X [0, MAX_NBITS] copy bit width - X feasible source offsets for the given copy bit width - X feasible destination offsets - */ - for (int msb0 = 0; msb0 < 2; msb0++) - { - const gdb_byte *data = msb0 ? data_msb0 : data_lsb0; - - for (unsigned int nbits = 1; nbits <= max_nbits; nbits++) - { - const unsigned int max_offset = data_nbits - nbits; - - for (unsigned source_offset = 0; - source_offset <= max_offset; - source_offset++) - { - for (unsigned dest_offset = 0; - dest_offset <= max_offset; - dest_offset++) - { - check_copy_bitwise (data + dest_offset / 8, - dest_offset % 8, - data + source_offset / 8, - source_offset % 8, - nbits, msb0); - } - } - } - - /* Special cases: copy all, copy nothing. */ - check_copy_bitwise (data_lsb0, 0, data_msb0, 0, data_nbits, msb0); - check_copy_bitwise (data_msb0, 0, data_lsb0, 0, data_nbits, msb0); - check_copy_bitwise (data, data_nbits - 7, data, 9, 0, msb0); - } -} - -} /* namespace selftests */ - -#endif /* GDB_SELF_TEST */ - void _initialize_utils (void) { @@ -3456,6 +3321,5 @@ _initialize_utils (void) #if GDB_SELF_TEST selftests::register_test ("gdb_realpath", gdb_realpath_tests); - selftests::register_test ("copy_bitwise", selftests::copy_bitwise_tests); #endif } -- 2.1.4