[PATCH v2 10/10] Add --enable-ubsan

[PATCH v2 10/10] Add --enable-ubsan

[Tom Tromey]

This adds --enable-ubsan to gdb's configure.  By default it is enabled
in development mode, and disabled otherwise.  This passes both
-fsanitize=undefined and -fno-sanitize-recover=undefined to
compilations, so that undefined behavior violations will be sure to
cause test failures.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* acinclude.m4: Include sanitize.m4.
	* configure: Rebuild.
	* configure.ac: Call AM_GDB_UBSAN.
	* sanitize.m4: New file.
---
 gdb/ChangeLog    |   7 ++++
 gdb/acinclude.m4 |   3 ++
 gdb/configure    | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/configure.ac |   1 +
 gdb/sanitize.m4  |  46 ++++++++++++++++++++++++
 5 files changed, 162 insertions(+)
 create mode 100644 gdb/sanitize.m4

diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
index 3c2d01015bf..52ba3f9ed6e 100644
--- a/gdb/acinclude.m4
+++ b/gdb/acinclude.m4
@@ -15,6 +15,9 @@ sinclude(transform.m4)
 # This gets AM_GDB_WARNINGS.
 sinclude(warning.m4)
 
+# AM_GDB_UBSAN
+sinclude(sanitize.m4)
+
 dnl gdb/configure.in uses BFD_NEED_DECLARATION, so get its definition.
 sinclude(../bfd/bfd.m4)
 
diff --git a/gdb/configure b/gdb/configure
index d207c2baf1b..51fef3266e4 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -886,6 +886,7 @@ with_system_gdbinit
 enable_werror
 enable_build_warnings
 enable_gdb_build_warnings
+enable_ubsan
 with_lzma
 with_liblzma_prefix
 with_tcl
@@ -1558,6 +1559,7 @@ Optional Features:
   --enable-gdb-build-warnings
                           enable GDB specific build-time compiler warnings if
                           gcc is used
+  --enable-ubsan          enable undefined behavior sanitizer (auto/yes/no)
   --enable-sim            link gdb with simulator
   --enable-multi-ice      build the multi-ice-gdb-server
   --enable-gdbserver      automatically build gdbserver (yes/no/auto, default
@@ -2451,6 +2453,52 @@ $as_echo "$ac_res" >&6; }
   eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
 
 } # ac_fn_c_check_decl
+
+# ac_fn_cxx_try_link LINENO
+# -------------------------
+# Try to link conftest.$ac_ext, and return whether this succeeded.
+ac_fn_cxx_try_link ()
+{
+  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
+  rm -f conftest.$ac_objext conftest$ac_exeext
+  if { { ac_try="$ac_link"
+case "(($ac_try" in
+  *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
+  *) ac_try_echo=$ac_try;;
+esac
+eval ac_try_echo="\"\$as_me:${as_lineno-$LINENO}: $ac_try_echo\""
+$as_echo "$ac_try_echo"; } >&5
+  (eval "$ac_link") 2>conftest.err
+  ac_status=$?
+  if test -s conftest.err; then
+    grep -v '^ *+' conftest.err >conftest.er1
+    cat conftest.er1 >&5
+    mv -f conftest.er1 conftest.err
+  fi
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; } && {
+	 test -z "$ac_cxx_werror_flag" ||
+	 test ! -s conftest.err
+       } && test -s conftest$ac_exeext && {
+	 test "$cross_compiling" = yes ||
+	 test -x conftest$ac_exeext
+       }; then :
+  ac_retval=0
+else
+  $as_echo "$as_me: failed program was:" >&5
+sed 's/^/| /' conftest.$ac_ext >&5
+
+	ac_retval=1
+fi
+  # Delete the IPA/IPO (Inter Procedural Analysis/Optimization) information
+  # created by the PGI compiler (conftest_ipa8_conftest.oo), as it would
+  # interfere with the next link command; also delete a directory that is
+  # left behind by Apple's compiler.  We do this before executing the actions.
+  rm -rf conftest.dSYM conftest_ipa8_conftest.oo
+  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
+  as_fn_set_status $ac_retval
+
+} # ac_fn_cxx_try_link
 cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
@@ -15555,6 +15603,63 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 
 
+# Check whether --enable-ubsan was given.
+if test "${enable_ubsan+set}" = set; then :
+  enableval=$enable_ubsan;
+else
+  enable_ubsan=auto
+fi
+
+if test "x$enable_ubsan" = xauto; then
+  if $development; then
+    enable_ubsan=yes
+  fi
+fi
+ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+if test "x$enable_ubsan" = xyes; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether -fsanitize=undefined is accepted" >&5
+$as_echo_n "checking whether -fsanitize=undefined is accepted... " >&6; }
+  saved_CXXFLAGS="$CXXFLAGS"
+  CXXFLAGS="$CXXFLAGS -fsanitize=undefined -fno-sanitize-recover=undefined"
+        cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+int
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_link "$LINENO"; then :
+  enable_ubsan=yes
+else
+  enable_ubsan=no
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext conftest.$ac_ext
+  CXXFLAGS="$saved_CXXFLAGS"
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $enable_ubsan" >&5
+$as_echo "$enable_ubsan" >&6; }
+  if test "x$enable_ubsan" = xyes; then
+    WARN_CFLAGS="$WARN_CFLAGS -fsanitize=undefined -fno-sanitize-recover=undefined"
+    CONFIG_LDFLAGS="$CONFIG_LDFLAGS -fsanitize=undefined"
+  fi
+fi
+ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+
+
 # In the Cygwin environment, we need some additional flags.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for cygwin" >&5
 $as_echo_n "checking for cygwin... " >&6; }
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 13bc5f9a8f2..a87a5ff43ac 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1860,6 +1860,7 @@ GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
     [])
 
 AM_GDB_WARNINGS
+AM_GDB_UBSAN
 
 # In the Cygwin environment, we need some additional flags.
 AC_CACHE_CHECK([for cygwin], gdb_cv_os_cygwin,
diff --git a/gdb/sanitize.m4 b/gdb/sanitize.m4
new file mode 100644
index 00000000000..3e2207b466c
--- /dev/null
+++ b/gdb/sanitize.m4
@@ -0,0 +1,46 @@
+dnl Autoconf configure script for GDB, the GNU debugger.
+dnl Copyright (C) 2018 Free Software Foundation, Inc.
+dnl
+dnl This file is part of GDB.
+dnl
+dnl This program is free software; you can redistribute it and/or modify
+dnl it under the terms of the GNU General Public License as published by
+dnl the Free Software Foundation; either version 3 of the License, or
+dnl (at your option) any later version.
+dnl
+dnl This program is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+dnl GNU General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU General Public License
+dnl along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+AC_DEFUN([AM_GDB_UBSAN],[
+AC_ARG_ENABLE(ubsan,
+  AS_HELP_STRING([--enable-ubsan],
+                 [enable undefined behavior sanitizer (auto/yes/no)]),
+  [],enable_ubsan=auto)
+if test "x$enable_ubsan" = xauto; then
+  if $development; then
+    enable_ubsan=yes
+  fi
+fi
+AC_LANG_PUSH([C++])
+if test "x$enable_ubsan" = xyes; then
+  AC_MSG_CHECKING(whether -fsanitize=undefined is accepted)
+  saved_CXXFLAGS="$CXXFLAGS"
+  CXXFLAGS="$CXXFLAGS -fsanitize=undefined -fno-sanitize-recover=undefined"
+  dnl A link check is required because it is possible to install gcc
+  dnl without libubsan, leading to link failures when compiling with
+  dnl -fsanitize=undefined.
+  AC_TRY_LINK([],[],enable_ubsan=yes,enable_ubsan=no)
+  CXXFLAGS="$saved_CXXFLAGS"
+  AC_MSG_RESULT($enable_ubsan)
+  if test "x$enable_ubsan" = xyes; then
+    WARN_CFLAGS="$WARN_CFLAGS -fsanitize=undefined -fno-sanitize-recover=undefined"
+    CONFIG_LDFLAGS="$CONFIG_LDFLAGS -fsanitize=undefined"
+  fi
+fi
+AC_LANG_POP([C++])
+])
-- 
2.13.6

[PATCH v2 09/10] Avoid undefined behavior in expression dumping

[Tom Tromey]

-fsanitize=undefined pointed out undefined behavior in
dump_raw_expression like:

    runtime error: load of value 2887952, which is not a valid value for type 'exp_opcode'

dump_raw_expression will try to print the opcode for each element of
the expression, even when it is not valid.  To allow this, but have it
avoid undefined behavior, this patch sets the underlying type of enum
exp_opcode, and arranges for op_name to handle invalid opcodes more
nicely.

Before this patch, debug-expr.exp shows:

Dump of expression @ 0x60f000007750, before conversion to prefix form:
	Language c, 8 elements, 16 bytes each.
	Index                Opcode         Hex Value  String Value
	    0               OP_TYPE  89  Y...............
   <unknown 3851920>  107820862850704  ..:..b..........
	    2               OP_TYPE  89  Y...............
	    3          OP_VAR_VALUE  40  (...............
	    4     <unknown 2807568>  107820861806352  ..*..b..........
	    5     <unknown 2806368>  107820861805152  `.*..b..........
	    6          OP_VAR_VALUE  40  (...............
	    7      UNOP_MEMVAL_TYPE  57  9...............

Afterward, the output is:

Dump of expression @ 0x4820f90, before conversion to prefix form:
	Language c, 8 elements, 16 bytes each.
	Index                Opcode         Hex Value  String Value
	    0               OP_TYPE  89  Y...............
	    1   unknown opcode: 176  75444400  .0..............
	    2               OP_TYPE  89  Y...............
	    3          OP_VAR_VALUE  40  (...............
	    4               OP_BOOL  74616912  P.r.............
	    5   unknown opcode: 128  74615680  ..r.............
	    6          OP_VAR_VALUE  40  (...............
	    7      UNOP_MEMVAL_TYPE  57  9...............

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* expression.h (enum exp_opcode): Use uint8_t as base type.
	* expprint.c (op_name): Handle invalid opcodes.
---
 gdb/ChangeLog    | 5 +++++
 gdb/expprint.c   | 7 +++++++
 gdb/expression.h | 2 +-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/gdb/expprint.c b/gdb/expprint.c
index d6ed41253ed..e87b3b709b9 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -687,6 +687,13 @@ static int dump_subexp_body (struct expression *exp, struct ui_file *, int);
 const char *
 op_name (struct expression *exp, enum exp_opcode opcode)
 {
+  if (opcode >= OP_UNUSED_LAST)
+    {
+      char *cell = get_print_cell ();
+      xsnprintf (cell, PRINT_CELL_SIZE, "unknown opcode: %u",
+		 unsigned (opcode));
+      return cell;
+    }
   return exp->language_defn->la_exp_desc->op_name (opcode);
 }
 
diff --git a/gdb/expression.h b/gdb/expression.h
index 9f26bb8d60b..db572efe2a3 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -39,7 +39,7 @@
    and skip that many.  Strings, like numbers, are indicated
    by the preceding opcode.  */
 
-enum exp_opcode
+enum exp_opcode : uint8_t
   {
 #define OP(name) name ,
 
-- 
2.13.6

[PATCH v2 02/10] Change dwarf2_frame_state_reg_info::reg to be std::vector

[Tom Tromey]

This changes dwarf2_frame_state_reg_info::reg to be a std::vector.
This avoids undefined behavior in the copy constructor when the
original object does not have any registers.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* dwarf2-frame.h (dwarf2_frame_state_reg_info)
	<~dwarf2_frame_state_reg_info>: Update.
	<dwarf2_frame_state_reg_info>: Update.
	<alloc_regs>: Add assertion.  Update.
	<reg>: Now a std::vector.
	<num_regs>: Remove.
	<swap>: Update.
	* dwarf2-frame.c (dwarf2_restore_rule, execute_cfa_program)
	(execute_cfa_program_test, dwarf2_frame_cache): Update.
---
 gdb/ChangeLog      | 12 ++++++++++++
 gdb/dwarf2-frame.c | 15 +++++++--------
 gdb/dwarf2-frame.h | 31 +++++++++----------------------
 3 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index f7dc820f4db..a594d223aa9 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -204,14 +204,13 @@ dwarf2_restore_rule (struct gdbarch *gdbarch, ULONGEST reg_num,
 {
   ULONGEST reg;
 
-  gdb_assert (fs->initial.reg);
   reg = dwarf2_frame_adjust_regnum (gdbarch, reg_num, eh_frame_p);
   fs->regs.alloc_regs (reg + 1);
 
   /* Check if this register was explicitly initialized in the
   CIE initial instructions.  If not, default the rule to
   UNSPECIFIED.  */
-  if (reg < fs->initial.num_regs)
+  if (reg < fs->initial.reg.size ())
     fs->regs.reg[reg] = fs->initial.reg[reg];
   else
     fs->regs.reg[reg].how = DWARF2_FRAME_REG_UNSPECIFIED;
@@ -602,7 +601,7 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
 	}
     }
 
-  if (fs->initial.reg == NULL)
+  if (fs->initial.reg.empty ())
     {
       /* Don't allow remember/restore between CIE and FDE programs.  */
       delete fs->regs.prev;
@@ -653,12 +652,12 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
   auto r1 = dwarf2_frame_adjust_regnum (gdbarch, 1, fde.eh_frame_p);
   auto r2 = dwarf2_frame_adjust_regnum (gdbarch, 2, fde.eh_frame_p);
 
-  SELF_CHECK (fs.regs.num_regs == (std::max (r1, r2) + 1));
+  SELF_CHECK (fs.regs.reg.size () == (std::max (r1, r2) + 1));
 
   SELF_CHECK (fs.regs.reg[r2].how == DWARF2_FRAME_REG_SAVED_OFFSET);
   SELF_CHECK (fs.regs.reg[r2].loc.offset == -4);
 
-  for (auto i = 0; i < fs.regs.num_regs; i++)
+  for (auto i = 0; i < fs.regs.reg.size (); i++)
     if (i != r2)
       SELF_CHECK (fs.regs.reg[i].how == DWARF2_FRAME_REG_UNSPECIFIED);
 
@@ -1097,7 +1096,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
   {
     int column;		/* CFI speak for "register number".  */
 
-    for (column = 0; column < fs.regs.num_regs; column++)
+    for (column = 0; column < fs.regs.reg.size (); column++)
       {
 	/* Use the GDB register number as the destination index.  */
 	int regnum = dwarf_reg_to_regnum (gdbarch, column);
@@ -1150,7 +1149,7 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
                register corresponding to the return address column.
                Incidentally, that's how we should treat a return
                address column specifying "same value" too.  */
-	    if (fs.retaddr_column < fs.regs.num_regs
+	    if (fs.retaddr_column < fs.regs.reg.size ()
 		&& retaddr_reg->how != DWARF2_FRAME_REG_UNSPECIFIED
 		&& retaddr_reg->how != DWARF2_FRAME_REG_SAME_VALUE)
 	      {
@@ -1176,7 +1175,7 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
       }
   }
 
-  if (fs.retaddr_column < fs.regs.num_regs
+  if (fs.retaddr_column < fs.regs.reg.size ()
       && fs.regs.reg[fs.retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
     cache->undefined_retaddr = 1;
 
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index 52316e5e168..b89f931651a 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -98,19 +98,14 @@ struct dwarf2_frame_state_reg_info
   ~dwarf2_frame_state_reg_info ()
   {
     delete prev;
-    xfree (reg);
   }
 
   /* Copy constructor.  */
   dwarf2_frame_state_reg_info (const dwarf2_frame_state_reg_info &src)
-    : num_regs (src.num_regs), cfa_offset (src.cfa_offset),
+    : reg (src.reg), cfa_offset (src.cfa_offset),
       cfa_reg (src.cfa_reg), cfa_how (src.cfa_how), cfa_exp (src.cfa_exp),
       prev (src.prev)
   {
-    size_t size = src.num_regs * sizeof (struct dwarf2_frame_state_reg);
-
-    reg = (struct dwarf2_frame_state_reg *) xmalloc (size);
-    memcpy (reg, src.reg, size);
   }
 
   /* Assignment operator for both move-assignment and copy-assignment.  */
@@ -123,33 +118,26 @@ struct dwarf2_frame_state_reg_info
 
   /* Move constructor.  */
   dwarf2_frame_state_reg_info (dwarf2_frame_state_reg_info &&rhs) noexcept
-    : reg (rhs.reg), num_regs (rhs.num_regs), cfa_offset (rhs.cfa_offset),
+    : reg (std::move (rhs.reg)), cfa_offset (rhs.cfa_offset),
       cfa_reg (rhs.cfa_reg), cfa_how (rhs.cfa_how), cfa_exp (rhs.cfa_exp),
       prev (rhs.prev)
   {
     rhs.prev = nullptr;
-    rhs.reg = nullptr;
   }
 
-/* Assert that the register set RS is large enough to store gdbarch_num_regs
-   columns.  If necessary, enlarge the register set.  */
+  /* If necessary, enlarge the register set to hold NUM_REGS_REQUESTED
+     registers.  */
   void alloc_regs (int num_regs_requested)
   {
-    if (num_regs_requested <= num_regs)
-      return;
+    gdb_assert (num_regs_requested > 0);
 
-    size_t size = sizeof (struct dwarf2_frame_state_reg);
-
-    reg = (struct dwarf2_frame_state_reg *)
-      xrealloc (reg, num_regs_requested * size);
+    if (num_regs_requested <= reg.size ())
+      return;
 
-    /* Initialize newly allocated registers.  */
-    memset (reg + num_regs, 0, (num_regs_requested - num_regs) * size);
-    num_regs = num_regs_requested;
+    reg.resize (num_regs_requested);
   }
 
-  struct dwarf2_frame_state_reg *reg = NULL;
-  int num_regs = 0;
+  std::vector<struct dwarf2_frame_state_reg> reg;
 
   LONGEST cfa_offset = 0;
   ULONGEST cfa_reg = 0;
@@ -166,7 +154,6 @@ private:
     using std::swap;
 
     swap (lhs.reg, rhs.reg);
-    swap (lhs.num_regs, rhs.num_regs);
     swap (lhs.cfa_offset, rhs.cfa_offset);
     swap (lhs.cfa_reg, rhs.cfa_reg);
     swap (lhs.cfa_how, rhs.cfa_how);
-- 
2.13.6

[PATCH v2 00/10] Enable undefined behavior sanitizer

[Tom Tromey]

This is v2 of the series to enable the undefined behavior sanitizer
for gdb.  This is done by default in development mode, but can be
disabled.

I believe this version addresses all the review comments.

Tested (*) by the buildbot.

(*) However, as I mentioned in another thread, the s390 builders do
not like this series.  I get many, many failures.  Looking at the
gdb.log, though, the failures seem to be coming from libstdc++ debug
mode -- which this series does not enable.

    Here's a link to one such build in case you are curious:

    Full Build URL:
	    <http://gdb-build.sergiodj.net/builders/RHEL-s390x-m64/builds/8180>

    Testsuite log (gdb.sum and gdb.log) URL(s):
	    <http://gdb-build.sergiodj.net/results/RHEL-s390x-m64/try/5f/5fe3f3e4633df1ea76ff24a2732d7c73dc983b90/0/>

Until this is understood I am reluctant to push this series.

Tom

[PATCH v2 08/10] Avoid undefined behavior in ada_operator_length

[Tom Tromey]

-fsanitize=undefined pointed out this error:

    runtime error: load of value 2887952, which is not a valid value for type 'exp_opcode'

This happens in gdb.ada/complete.exp when processing "complete p
my_glob".  This does not parse, so the Ada parser throws an exception;
but then the code in parse_exp_in_context_1 accepts the expression
anyway.  However, as no elements have been written to the expression,
undefined behavior results.

The fix is to notice this case in parse_exp_in_context_1.  This patch
also adds an assertion to prefixify_expression to enforce this
pre-existing constraint.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* parse.c (prefixify_expression): Add assert.
	(parse_exp_in_context_1): Throw exception if the expression is
	empty.
---
 gdb/ChangeLog | 8 +++++++-
 gdb/parse.c   | 6 +++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/gdb/parse.c b/gdb/parse.c
index 163852cb84c..8a128a74d77 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -792,6 +792,7 @@ copy_name (struct stoken token)
 int
 prefixify_expression (struct expression *expr)
 {
+  gdb_assert (expr->nelts > 0);
   int len = sizeof (struct expression) + EXP_ELEM_TO_BYTES (expr->nelts);
   struct expression *temp;
   int inpos = expr->nelts, outpos = 0;
@@ -1205,7 +1206,10 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
     }
   CATCH (except, RETURN_MASK_ALL)
     {
-      if (! parse_completion)
+      /* If parsing for completion, allow this to succeed; but if no
+	 expression elements have been written, then there's nothing
+	 to do, so fail.  */
+      if (! parse_completion || ps.expout_ptr == 0)
 	throw_exception (except);
     }
   END_CATCH
-- 
2.13.6

[PATCH v2 06/10] Avoid undefined behavior in parse_number

[Tom Tromey]

-fsanitize=undefined pointed out that c-exp.y relied on undefined
behavior here:

      if (c != 'l' && c != 'u')
	n *= base;

...when a large hex constant "just fit" into a LONGEST, causing the
high bit to be set.

This fixes the problem by having the function work in an unsigned
type.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* c-exp.y (parse_number): Work in unsigned.  Remove casts.
---
 gdb/ChangeLog |  4 ++++
 gdb/c-exp.y   | 10 ++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 0326ee090e1..09e31d2283a 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1760,10 +1760,8 @@ static int
 parse_number (struct parser_state *par_state,
 	      const char *buf, int len, int parsed_float, YYSTYPE *putithere)
 {
-  /* FIXME: Shouldn't these be unsigned?  We don't deal with negative values
-     here, and we do kind of silly things like cast to unsigned.  */
-  LONGEST n = 0;
-  LONGEST prevn = 0;
+  ULONGEST n = 0;
+  ULONGEST prevn = 0;
   ULONGEST un;
 
   int i = 0;
@@ -1922,7 +1920,7 @@ parse_number (struct parser_state *par_state,
 	 on 0x123456789 when LONGEST is 32 bits.  */
       if (c != 'l' && c != 'u' && n != 0)
 	{	
-	  if ((unsigned_p && (ULONGEST) prevn >= (ULONGEST) n))
+	  if (unsigned_p && prevn >= n)
 	    error (_("Numeric constant too large."));
 	}
       prevn = n;
@@ -1940,7 +1938,7 @@ parse_number (struct parser_state *par_state,
      the case where it is we just always shift the value more than
      once, with fewer bits each time.  */
 
-  un = (ULONGEST)n >> 2;
+  un = n >> 2;
   if (long_p == 0
       && (un >> (gdbarch_int_bit (parse_gdbarch (par_state)) - 2)) == 0)
     {
-- 
2.13.6

[PATCH v2 04/10] Avoid undefined behavior in extract_integer

[Tom Tromey]

-fsanitize=undefined showed that extract_integer could left-shift a
negative value, which is undefined.  This patch fixes the problem by
doing all the work in an unsigned type.  This relies on
implementation-defined behavior, but I tend to think we are on safe
ground there.  (Also, if need be, violations of this could probably be
detected, either by configure or by a static_assert.)

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* findvar.c (extract_integer): Do work in an unsigned type.
---
 gdb/ChangeLog | 4 ++++
 gdb/findvar.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/findvar.c b/gdb/findvar.c
index 9256833ab60..be6c9d6f60b 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -50,7 +50,7 @@ template<typename T, typename>
 T
 extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order)
 {
-  T retval = 0;
+  typename std::make_unsigned<T>::type retval = 0;
   const unsigned char *p;
   const unsigned char *startaddr = addr;
   const unsigned char *endaddr = startaddr + len;
-- 
2.13.6

[PATCH v2 01/10] Do not pass NULL to memcpy

[Tom Tromey]

-fsanitize=undefined pointed out a spot that passes NULL to memcpy,
which is undefined behavior according to the C standard.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* namespace.c (add_using_directive): Don't pass NULL to memcpy.
---
 gdb/ChangeLog   | 4 ++++
 gdb/namespace.c | 5 +++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/namespace.c b/gdb/namespace.c
index be998d9d491..85c0c4b14d7 100644
--- a/gdb/namespace.c
+++ b/gdb/namespace.c
@@ -111,8 +111,9 @@ add_using_directive (struct using_direct **using_directives,
   else
     newobj->declaration = declaration;
 
-  memcpy (newobj->excludes, excludes.data (),
-	  excludes.size () * sizeof (*newobj->excludes));
+  if (!excludes.empty ())
+    memcpy (newobj->excludes, excludes.data (),
+	    excludes.size () * sizeof (*newobj->excludes));
   newobj->excludes[excludes.size ()] = NULL;
 
   newobj->next = *using_directives;
-- 
2.13.6

[PATCH v2 03/10] Use unsigned as base type for some enums

[Tom Tromey]

-fsanitize=undefined complains about using operator~ on various enum
types that are used with DEF_ENUM_FLAGS_TYPE.  This patch fixes these
problems by explicitly setting the base type for these enums to
unsigned.  It also adds a static assert to enum_flags to ensure that
future enums used this way have an unsigned underlying type.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* common/enum-flags.h (enum_flags::operator~): Add static assert.
	* symfile-add-flags.h (enum symfile_add_flag): Use unsigned as
	base type.
	* objfile-flags.h (enum objfile_flag): Use unsigned as base type.
	* gdbtypes.h (enum type_instance_flag_value): Use unsigned as base
	type.
	* c-lang.h (enum c_string_type_values): Use unsigned as base
	type.
	* btrace.h (enum btrace_thread_flag): Use unsigned as base type.
---
 gdb/ChangeLog           | 12 ++++++++++++
 gdb/btrace.h            |  2 +-
 gdb/c-lang.h            |  2 +-
 gdb/common/enum-flags.h |  1 +
 gdb/gdbtypes.h          |  2 +-
 gdb/objfile-flags.h     |  2 +-
 gdb/symfile-add-flags.h |  2 +-
 7 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/gdb/btrace.h b/gdb/btrace.h
index bfb0b9f2787..0448bd6d498 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -228,7 +228,7 @@ struct btrace_call_history
 };
 
 /* Branch trace thread flags.  */
-enum btrace_thread_flag
+enum btrace_thread_flag : unsigned
 {
   /* The thread is to be stepped forwards.  */
   BTHR_STEP = (1 << 0),
diff --git a/gdb/c-lang.h b/gdb/c-lang.h
index ae17abd20f9..f9eab04730c 100644
--- a/gdb/c-lang.h
+++ b/gdb/c-lang.h
@@ -35,7 +35,7 @@ struct parser_state;
 /* The various kinds of C string and character.  Note that these
    values are chosen so that they may be or'd together in certain
    ways.  */
-enum c_string_type_values
+enum c_string_type_values : unsigned
   {
     /* An ordinary string: "value".  */
     C_STRING = 0,
diff --git a/gdb/common/enum-flags.h b/gdb/common/enum-flags.h
index 82568a5a3d9..dad59cec867 100644
--- a/gdb/common/enum-flags.h
+++ b/gdb/common/enum-flags.h
@@ -164,6 +164,7 @@ public:
   }
   enum_flags operator~ () const
   {
+    gdb_static_assert (std::is_unsigned<underlying_type>::value);
     return (enum_type) ~underlying_value ();
   }
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 14059ab3dc5..f7ea2ac6df5 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -192,7 +192,7 @@ enum type_code
 /* * Some bits for the type's instance_flags word.  See the macros
    below for documentation on each bit.  */
 
-enum type_instance_flag_value
+enum type_instance_flag_value : unsigned
 {
   TYPE_INSTANCE_FLAG_CONST = (1 << 0),
   TYPE_INSTANCE_FLAG_VOLATILE = (1 << 1),
diff --git a/gdb/objfile-flags.h b/gdb/objfile-flags.h
index aeaa8fbc04d..6f5760d021d 100644
--- a/gdb/objfile-flags.h
+++ b/gdb/objfile-flags.h
@@ -25,7 +25,7 @@
 /* Defines for the objfile flags field.  Defined in a separate file to
    break circular header dependencies.  */
 
-enum objfile_flag
+enum objfile_flag : unsigned
   {
     /* When an object file has its functions reordered (currently
        Irix-5.2 shared libraries exhibit this behaviour), we will need
diff --git a/gdb/symfile-add-flags.h b/gdb/symfile-add-flags.h
index 3c07513e395..35241fe4a04 100644
--- a/gdb/symfile-add-flags.h
+++ b/gdb/symfile-add-flags.h
@@ -26,7 +26,7 @@
    symbol_file_add, etc.  Defined in a separate file to break circular
    header dependencies.  */
 
-enum symfile_add_flag
+enum symfile_add_flag : unsigned
   {
     /* Be chatty about what you are doing.  */
     SYMFILE_VERBOSE = 1 << 1,
-- 
2.13.6

[PATCH v2 05/10] Avoid undefined behavior in read_subrange_type

[Tom Tromey]

-fsanitize=undefined pointed out an undefined shift of a negative
value in read_subrange_type.  The fix is to do the work in an unsigned
type, where this is defined.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* dwarf2read.c (read_subrange_type): Make "negative_mask"
	unsigned.
---
 gdb/ChangeLog    | 5 +++++
 gdb/dwarf2read.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 8834d08a1c6..86ef1c4040b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -17682,7 +17682,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   int low_default_is_valid;
   int high_bound_is_count = 0;
   const char *name;
-  LONGEST negative_mask;
+  ULONGEST negative_mask;
 
   orig_base_type = die_type (die, cu);
   /* If ORIG_BASE_TYPE is a typedef, it will not be TYPE_UNSIGNED,
@@ -17815,7 +17815,7 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
      the bounds as signed, and thus sign-extend their values, when
      the base type is signed.  */
   negative_mask =
-    -((LONGEST) 1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1));
+    -((ULONGEST) 1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1));
   if (low.kind == PROP_CONST
       && !TYPE_UNSIGNED (base_type) && (low.data.const_val & negative_mask))
     low.data.const_val |= negative_mask;
-- 
2.13.6

[PATCH v2 07/10] Avoid undefined behavior in read_signed_leb128

[Tom Tromey]

-fsanitize=undefined pointed out that read_signed_leb128 had an
undefined left-shift when processing the final byte of a 64-bit leb:

    runtime error: left shift of 127 by 63 places cannot be represented in type 'long int'

and an undefined negation:

    runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself

Both of these problems are readily avoided by havinng
read_signed_leb128 work in an unsigned type, and then casting to the
signed type at the return.

gdb/ChangeLog
2018-08-29  Tom Tromey  <tom@tromey.com>

	* dwarf2read.c (read_signed_leb128): Work in ULONGEST.
---
 gdb/ChangeLog    | 4 ++++
 gdb/dwarf2read.c | 6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 86ef1c4040b..e61d6e04cb4 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -19600,7 +19600,7 @@ static LONGEST
 read_signed_leb128 (bfd *abfd, const gdb_byte *buf,
 		    unsigned int *bytes_read_ptr)
 {
-  LONGEST result;
+  ULONGEST result;
   int shift, num_read;
   unsigned char byte;
 
@@ -19612,7 +19612,7 @@ read_signed_leb128 (bfd *abfd, const gdb_byte *buf,
       byte = bfd_get_8 (abfd, buf);
       buf++;
       num_read++;
-      result |= ((LONGEST) (byte & 127) << shift);
+      result |= ((ULONGEST) (byte & 127) << shift);
       shift += 7;
       if ((byte & 128) == 0)
 	{
@@ -19620,7 +19620,7 @@ read_signed_leb128 (bfd *abfd, const gdb_byte *buf,
 	}
     }
   if ((shift < 8 * sizeof (result)) && (byte & 0x40))
-    result |= -(((LONGEST) 1) << shift);
+    result |= -(((ULONGEST) 1) << shift);
   *bytes_read_ptr = num_read;
   return result;
 }
-- 
2.13.6

Re: [PATCH v2 00/10] Enable undefined behavior sanitizer

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This is v2 of the series to enable the undefined behavior sanitizer
Tom> for gdb.  This is done by default in development mode, but can be
Tom> disabled.

Update on this series...

Tom> I believe this version addresses all the review comments.

For the record, subsequently, Pedro and I discussed the need to document
this a bit more.  I'll include a patch for this in the next revision of
the series.

Tom> Tested (*) by the buildbot.

Tom> (*) However, as I mentioned in another thread, the s390 builders do
Tom> not like this series.  I get many, many failures.  Looking at the
Tom> gdb.log, though, the failures seem to be coming from libstdc++ debug
Tom> mode -- which this series does not enable.

This is all explained now.

Sergio pointed out that the builders are enabling -D_GLIBCXX_DEBUG.  I'm
glad that mystery is solved.

A failure with -D_GLIBCXX_DEBUG enabled just results in a crash, without
much useful information to go by.  This is often fine, but in this case
I could only reproduce on s390, where I couldn't log in to reproduce for
myself.

So, I hacked the testsuite Makefile to run just a single test .exp that
I knew to fail, and then arranged to get a real stack trace.  See the
appended patch.

This pointed out that there was a bug in one of the patches in the series.
Namely, this code in dwarf2-frame.c:

	    struct dwarf2_frame_state_reg *retaddr_reg =
	      &fs.regs.reg[fs.retaddr_column];

... will reference past the end of the array in some cases.  When reg is
just an ordinary pointer this is "ok" -- possibly undefined behavior
depending on the index in use, but not likely to cause a crash.  But the
series changes reg to a std::vector, where this is invalid.

Tom

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index 07e0928a9f8..3bfc4d9f847 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -48,7 +48,7 @@ EXPECT = `if [ "$${READ1}" != "" ] ; then \
 
 RUNTEST = $(RUNTEST_FOR_TARGET)
 
-RUNTESTFLAGS =
+override RUNTESTFLAGS := gdb.cp/extern-c.exp
 
 FORCE_PARALLEL =
 
@@ -153,7 +153,11 @@ CHECK_TARGET = $(if $(RACY_ITER),$(addsuffix -racy,$(CHECK_TARGET_TMP)),$(CHECK_
 # because GNU make 3.82 has a bug preventing MAKEFLAGS from being used
 # in conditions.
 check: all $(abs_builddir)/site.exp
-	$(MAKE) $(CHECK_TARGET)
+	-$(MAKE) $(CHECK_TARGET)
+	cat gdb.log
+	/bin/gdb -batch -ex 'run' -ex 'bt' \
+		--args ../gdb -batch -ex 'file outputs/gdb.cp/extern-c/extern-c' \
+		-ex 'break c_func' -ex 'run' -ex 'rbreak c_funcs'
 
 check-read1:
 	$(MAKE) READ1="1" check

Re: [PATCH v2 02/10] Change dwarf2_frame_state_reg_info::reg to be std::vector

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> On 10/02/2018 05:44 AM, Tom Tromey wrote:
>> This changes dwarf2_frame_state_reg_info::reg to be a std::vector.
>> This avoids undefined behavior in the copy constructor when the
>> original object does not have any registers.

Pedro> I don't have anything to say against the patch, I think it's a nice
Pedro> cleanup regardless, but could you expand this log a little to point
Pedro> out exactly what is triggering the undefined behavior.  I guess we're
Pedro> passing NULL to the memcpy?

Yes, that was the issue.  Simon asked for this cleanup IIRC.
I've added a note about this to the commit message.

Tom

Re: [PATCH v2 02/10] Change dwarf2_frame_state_reg_info::reg to be std::vector

[Pedro Alves]

On 10/02/2018 05:44 AM, Tom Tromey wrote:
> This changes dwarf2_frame_state_reg_info::reg to be a std::vector.
> This avoids undefined behavior in the copy constructor when the
> original object does not have any registers.

I don't have anything to say against the patch, I think it's a nice
cleanup regardless, but could you expand this log a little to point
out exactly what is triggering the undefined behavior.  I guess we're
passing NULL to the memcpy?  LGTM otherwise.

Thanks,
Pedro Alves

[PATCH v2 02/10] Change dwarf2_frame_state_reg_info::reg to be std::vector

[Tom Tromey]

This changes dwarf2_frame_state_reg_info::reg to be a std::vector.
This avoids undefined behavior in the copy constructor when the
original object does not have any registers.

gdb/ChangeLog
2018-10-01  Tom Tromey  <tom@tromey.com>

	* dwarf2-frame.h (dwarf2_frame_state_reg_info)
	<~dwarf2_frame_state_reg_info>: Update.
	<dwarf2_frame_state_reg_info>: Update.
	<alloc_regs>: Add assertion.  Update.
	<reg>: Now a std::vector.
	<num_regs>: Remove.
	<swap>: Update.
	* dwarf2-frame.c (dwarf2_restore_rule, execute_cfa_program)
	(execute_cfa_program_test, dwarf2_frame_cache): Update.
---
 gdb/ChangeLog      | 12 ++++++++++++
 gdb/dwarf2-frame.c | 28 ++++++++++++++--------------
 gdb/dwarf2-frame.h | 31 +++++++++----------------------
 3 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index f7dc820f4d..118bc11217 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -204,14 +204,13 @@ dwarf2_restore_rule (struct gdbarch *gdbarch, ULONGEST reg_num,
 {
   ULONGEST reg;
 
-  gdb_assert (fs->initial.reg);
   reg = dwarf2_frame_adjust_regnum (gdbarch, reg_num, eh_frame_p);
   fs->regs.alloc_regs (reg + 1);
 
   /* Check if this register was explicitly initialized in the
   CIE initial instructions.  If not, default the rule to
   UNSPECIFIED.  */
-  if (reg < fs->initial.num_regs)
+  if (reg < fs->initial.reg.size ())
     fs->regs.reg[reg] = fs->initial.reg[reg];
   else
     fs->regs.reg[reg].how = DWARF2_FRAME_REG_UNSPECIFIED;
@@ -602,7 +601,7 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
 	}
     }
 
-  if (fs->initial.reg == NULL)
+  if (fs->initial.reg.empty ())
     {
       /* Don't allow remember/restore between CIE and FDE programs.  */
       delete fs->regs.prev;
@@ -653,12 +652,12 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
   auto r1 = dwarf2_frame_adjust_regnum (gdbarch, 1, fde.eh_frame_p);
   auto r2 = dwarf2_frame_adjust_regnum (gdbarch, 2, fde.eh_frame_p);
 
-  SELF_CHECK (fs.regs.num_regs == (std::max (r1, r2) + 1));
+  SELF_CHECK (fs.regs.reg.size () == (std::max (r1, r2) + 1));
 
   SELF_CHECK (fs.regs.reg[r2].how == DWARF2_FRAME_REG_SAVED_OFFSET);
   SELF_CHECK (fs.regs.reg[r2].loc.offset == -4);
 
-  for (auto i = 0; i < fs.regs.num_regs; i++)
+  for (auto i = 0; i < fs.regs.reg.size (); i++)
     if (i != r2)
       SELF_CHECK (fs.regs.reg[i].how == DWARF2_FRAME_REG_UNSPECIFIED);
 
@@ -1097,7 +1096,7 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
   {
     int column;		/* CFI speak for "register number".  */
 
-    for (column = 0; column < fs.regs.num_regs; column++)
+    for (column = 0; column < fs.regs.reg.size (); column++)
       {
 	/* Use the GDB register number as the destination index.  */
 	int regnum = dwarf_reg_to_regnum (gdbarch, column);
@@ -1140,8 +1139,9 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
 	if (cache->reg[regnum].how == DWARF2_FRAME_REG_RA
 	    || cache->reg[regnum].how == DWARF2_FRAME_REG_RA_OFFSET)
 	  {
-	    struct dwarf2_frame_state_reg *retaddr_reg =
-	      &fs.regs.reg[fs.retaddr_column];
+	    const std::vector<struct dwarf2_frame_state_reg> &regs
+	      = fs.regs.reg;
+	    ULONGEST retaddr_column = fs.retaddr_column;
 
 	    /* It seems rather bizarre to specify an "empty" column as
                the return adress column.  However, this is exactly
@@ -1150,14 +1150,14 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
                register corresponding to the return address column.
                Incidentally, that's how we should treat a return
                address column specifying "same value" too.  */
-	    if (fs.retaddr_column < fs.regs.num_regs
-		&& retaddr_reg->how != DWARF2_FRAME_REG_UNSPECIFIED
-		&& retaddr_reg->how != DWARF2_FRAME_REG_SAME_VALUE)
+	    if (fs.retaddr_column < fs.regs.reg.size ()
+		&& regs[retaddr_column].how != DWARF2_FRAME_REG_UNSPECIFIED
+		&& regs[retaddr_column].how != DWARF2_FRAME_REG_SAME_VALUE)
 	      {
 		if (cache->reg[regnum].how == DWARF2_FRAME_REG_RA)
-		  cache->reg[regnum] = *retaddr_reg;
+		  cache->reg[regnum] = regs[retaddr_column];
 		else
-		  cache->retaddr_reg = *retaddr_reg;
+		  cache->retaddr_reg = regs[retaddr_column];
 	      }
 	    else
 	      {
@@ -1176,7 +1176,7 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
       }
   }
 
-  if (fs.retaddr_column < fs.regs.num_regs
+  if (fs.retaddr_column < fs.regs.reg.size ()
       && fs.regs.reg[fs.retaddr_column].how == DWARF2_FRAME_REG_UNDEFINED)
     cache->undefined_retaddr = 1;
 
diff --git a/gdb/dwarf2-frame.h b/gdb/dwarf2-frame.h
index 52316e5e16..b89f931651 100644
--- a/gdb/dwarf2-frame.h
+++ b/gdb/dwarf2-frame.h
@@ -98,19 +98,14 @@ struct dwarf2_frame_state_reg_info
   ~dwarf2_frame_state_reg_info ()
   {
     delete prev;
-    xfree (reg);
   }
 
   /* Copy constructor.  */
   dwarf2_frame_state_reg_info (const dwarf2_frame_state_reg_info &src)
-    : num_regs (src.num_regs), cfa_offset (src.cfa_offset),
+    : reg (src.reg), cfa_offset (src.cfa_offset),
       cfa_reg (src.cfa_reg), cfa_how (src.cfa_how), cfa_exp (src.cfa_exp),
       prev (src.prev)
   {
-    size_t size = src.num_regs * sizeof (struct dwarf2_frame_state_reg);
-
-    reg = (struct dwarf2_frame_state_reg *) xmalloc (size);
-    memcpy (reg, src.reg, size);
   }
 
   /* Assignment operator for both move-assignment and copy-assignment.  */
@@ -123,33 +118,26 @@ struct dwarf2_frame_state_reg_info
 
   /* Move constructor.  */
   dwarf2_frame_state_reg_info (dwarf2_frame_state_reg_info &&rhs) noexcept
-    : reg (rhs.reg), num_regs (rhs.num_regs), cfa_offset (rhs.cfa_offset),
+    : reg (std::move (rhs.reg)), cfa_offset (rhs.cfa_offset),
       cfa_reg (rhs.cfa_reg), cfa_how (rhs.cfa_how), cfa_exp (rhs.cfa_exp),
       prev (rhs.prev)
   {
     rhs.prev = nullptr;
-    rhs.reg = nullptr;
   }
 
-/* Assert that the register set RS is large enough to store gdbarch_num_regs
-   columns.  If necessary, enlarge the register set.  */
+  /* If necessary, enlarge the register set to hold NUM_REGS_REQUESTED
+     registers.  */
   void alloc_regs (int num_regs_requested)
   {
-    if (num_regs_requested <= num_regs)
-      return;
+    gdb_assert (num_regs_requested > 0);
 
-    size_t size = sizeof (struct dwarf2_frame_state_reg);
-
-    reg = (struct dwarf2_frame_state_reg *)
-      xrealloc (reg, num_regs_requested * size);
+    if (num_regs_requested <= reg.size ())
+      return;
 
-    /* Initialize newly allocated registers.  */
-    memset (reg + num_regs, 0, (num_regs_requested - num_regs) * size);
-    num_regs = num_regs_requested;
+    reg.resize (num_regs_requested);
   }
 
-  struct dwarf2_frame_state_reg *reg = NULL;
-  int num_regs = 0;
+  std::vector<struct dwarf2_frame_state_reg> reg;
 
   LONGEST cfa_offset = 0;
   ULONGEST cfa_reg = 0;
@@ -166,7 +154,6 @@ private:
     using std::swap;
 
     swap (lhs.reg, rhs.reg);
-    swap (lhs.num_regs, rhs.num_regs);
     swap (lhs.cfa_offset, rhs.cfa_offset);
     swap (lhs.cfa_reg, rhs.cfa_reg);
     swap (lhs.cfa_how, rhs.cfa_how);
-- 
2.17.1