This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 9/9] Add unit test to gdbarch methods register_to_value and value_to_register
On 2017-04-13 03:26, Yao Qi wrote:
This patch adds one unit test for gdbarch methods register_to_value and
value_to_register. The test pass different combinations of {regnu,
type}
to gdbarch_register_to_value and gdbarch_value_to_register. In order
to do the test, add a new function create_new_frame to create a fake
frame. It can be improved after we converted frame_info to class.
This test is useful with ASAN. Suppose I incorrectly modified
the size of buffer as below,
@@ -1228,7 +1228,7 @@ ia64_register_to_value (struct frame_info
*frame, int regnum,
int *optimizedp, int *unavailablep)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
- gdb_byte in[MAX_REGISTER_SIZE];
+ gdb_byte in[1];
/* Convert to TYPE. */
if (!get_frame_register_bytes (frame, regnum, 0,
build GDB with "-fsanitize=address" and run unittest.exp, asan can
detect
such error,
=================================================================
==6003==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7fff60a36520 at pc 0xbaa15d bp 0x7fff60a36250 sp 0x7fff60a36248
WRITE of size 16 at 0x7fff60a36520 thread T0
#0 0xbaa15c in frame_register_unwind(frame_info*, int, int*, int*,
lval_type*, unsigned long*, int*, unsigned char*) gdb/frame.c:1119
#1 0xbaa43b in frame_register(frame_info*, int, int*, int*,
lval_type*, unsigned long*, int*, unsigned char*) gdb/frame.c:1147
#2 0xbab998 in get_frame_register_bytes(frame_info*, int, unsigned
long, int, unsigned char*, int*, int*) gdb/frame.c:1424
#3 0x6ff39e in ia64_register_to_value gdb/ia64-tdep.c:1236
#4 0xbc9276 in gdbarch_register_to_value(gdbarch*, frame_info*,
int, type*, unsigned char*, int*, int*) gdb/gdbarch.c:2565
#5 0xbd8e6b in register_to_value_test
gdb/git/gdb/gdbarch-selftests.c:83
#6 0xd4e628 in tests_with_arch gdb/selftest-arch.c:75
#7 0xd4d02c in run_self_tests() gdb/selftest.c:48
#8 0xc8cc3d in maintenance_selftest gdb/maint.c:949
gdb:
2017-04-11 Yao Qi <yao.qi@linaro.org>
* gdbarch-selftests.c: New file.
Some unit tests are in the unittests directory. Do we have a rule for
what goes there versus in gdb/ directly?
* frame.c [GDB_SELF_TESTS] (create_new_frame): New function.
* frame.h [GDB_SELF_TESTS] (create_new_frame): Declare.
---
gdb/Makefile.in | 2 +
gdb/findvar.c | 1 -
gdb/frame.c | 18 +++++++
gdb/frame.h | 4 ++
gdb/gdbarch-selftests.c | 123
++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 147 insertions(+), 1 deletion(-)
create mode 100644 gdb/gdbarch-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 23e4bed..a926d9a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1087,6 +1087,7 @@ SFILES = \
gdb_obstack.c \
gdb_usleep.c \
gdbarch.c \
+ gdbarch-selftests.c \
gdbtypes.c \
gnu-v2-abi.c \
gnu-v3-abi.c \
@@ -1697,6 +1698,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
gdb_usleep.o \
gdb_vecs.o \
gdbarch.o \
+ gdbarch-selftests.o \
gdbtypes.o \
gnu-v2-abi.o \
gnu-v3-abi.o \
diff --git a/gdb/findvar.c b/gdb/findvar.c
index ed4d5c1..84f4f47 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -1004,4 +1004,3 @@ address_from_register (int regnum, struct
frame_info *frame)
return result;
}
-
Spurious change.
diff --git a/gdb/frame.c b/gdb/frame.c
index d4fc456..0dc8726 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1704,6 +1704,24 @@ select_frame (struct frame_info *fi)
}
}
+#if GDB_SELF_TEST
+struct frame_info *
+create_new_frame (struct gdbarch *gdbarch)
+{
+ struct frame_info *this_frame = XCNEW (struct frame_info);
+ struct regcache *regcache = regcache_xmalloc (gdbarch, NULL);
+
+ sentinel_frame = create_sentinel_frame (NULL, regcache);
+ sentinel_frame->prev = this_frame;
+ sentinel_frame->prev_p = 1;;
+ this_frame->prev_arch.p = 1;
+ this_frame->prev_arch.arch = gdbarch;
+ this_frame->next = sentinel_frame;
+
+ return this_frame;
+}
+#endif
+
/* Create an arbitrary (i.e. address specified by user) or innermost
frame.
Always returns a non-NULL value. */
diff --git a/gdb/frame.h b/gdb/frame.h
index 1d0644f..78727b2 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -833,6 +833,10 @@ extern struct frame_info
*deprecated_safe_get_selected_frame (void);
extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR
pc);
+#if GDB_SELF_TEST
+extern struct frame_info *create_new_frame (struct gdbarch *gdbarch);
The name create_new_frame suggests that it could be used to create a
frame used in the standard GDB context. Maybe create_test_frame would
be more appropriate?
Even if this function is only used in tests, a comment explaining what
it does would be nice, especially since it does more than creating a new
frame (it sets up some global state).
+#endif
+
/* Return true if the frame unwinder for frame FI is UNWINDER; false
otherwise. */
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
new file mode 100644
index 0000000..df084ce
--- /dev/null
+++ b/gdb/gdbarch-selftests.c
@@ -0,0 +1,123 @@
+/* Self tests for gdbarch for GDB, the GNU debugger.
+
+ Copyright (C) 2017 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
<http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#if GDB_SELF_TEST
+#include "selftest.h"
+#include "selftest-arch.h"
+#include "inferior.h"
+
+namespace selftests {
+
+/* Test gdbarch methods register_to_value and value_to_register. */
+
+static void
+register_to_value_test (struct gdbarch *gdbarch)
+{
+ const struct builtin_type *builtin = builtin_type (gdbarch);
+ struct type *types[] =
+ {
+ builtin->builtin_void, builtin->builtin_char,
+ builtin->builtin_short, builtin->builtin_int,
+ builtin->builtin_long, builtin->builtin_signed_char,
Can you put all of them on their own line?
+ builtin->builtin_unsigned_short,
+ builtin->builtin_unsigned_int,
+ builtin->builtin_unsigned_long,
+ builtin->builtin_float,
+ builtin->builtin_double,
+ builtin->builtin_long_double,
+ builtin->builtin_complex,
+ builtin->builtin_double_complex,
+ builtin->builtin_string,
+ builtin->builtin_bool,
+ builtin->builtin_long_long,
+ builtin->builtin_unsigned_long_long,
+ builtin->builtin_int8,
+ builtin->builtin_uint8,
+ builtin->builtin_int16,
+ builtin->builtin_uint16,
+ builtin->builtin_int32,
+ builtin->builtin_uint32,
+ builtin->builtin_int64,
+ builtin->builtin_uint64,
+ builtin->builtin_int128,
+ builtin->builtin_uint128,
+ builtin->builtin_char16,
+ builtin->builtin_char32,
+ };
+
+ current_inferior()->gdbarch = gdbarch;
+
+ struct frame_info *frame = create_new_frame (gdbarch);
If we need to reuse this in other tests, it
+ const int num_regs = (gdbarch_num_regs (gdbarch)
+ + gdbarch_num_pseudo_regs (gdbarch));
+
+ /* Test gdbarch methods register_to_value and value_to_register with
+ different combinations of register numbers and types. */
+ for (const auto &type : types)
+ {
+ for (auto regnum = 0; regnum < num_regs; regnum++)
+ {
+ if (gdbarch_convert_register_p (gdbarch, regnum, type))
+ {
+ std::vector<gdb_byte> buf (TYPE_LENGTH (type));
+ int optim, unavail, ok;
+
+ ok = gdbarch_register_to_value (gdbarch, frame, regnum, type,
+ buf.data (), &optim, &unavail);
+
+ SELF_CHECK (ok);
+ SELF_CHECK (!optim);
+ SELF_CHECK (!unavail);
+
+ bool saw_no_process_error = false;
+ TRY
+ {
+ gdbarch_value_to_register (gdbarch, frame, regnum, type,
+ buf.data ());
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (strcmp (ex.message,
+ "You can't do that without a process to debug.")
+ == 0)
+ saw_no_process_error = true;
+ }
+
+ /* GDB wants to write registers to target, so it is expected
+ to see no process exception. */
+ SELF_CHECK (saw_no_process_error);
When you subclass regcache to have a "mock" regcache, do you think we'll
be able to test the success case of this?
+ }
+ }
+ }
+}
+
+} // namespace selftests
+#endif /* GDB_SELF_TEST */
+
+/* Suppress warning from -Wmissing-prototypes. */
+extern initialize_file_ftype _initialize_gdbarch_selftests;
I don't think we still need this anymore.
+
+void
+_initialize_gdbarch_selftests (void)
+{
+#if GDB_SELF_TEST
+ register_self_test_foreach_arch (selftests::register_to_value_test);
+#endif
+}
Thanks,
Simon