public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCHv3 3/3] gdb: mark disassembler function callback types as noexcept
Date: Fri, 18 Nov 2022 16:57:32 +0000	[thread overview]
Message-ID: <ca85925e33d8e5084bf80d315a3a5ffe657ee2e1.1668790576.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1668790576.git.aburgess@redhat.com>

In disasm.h we define a set of types that are used by the various
disassembler classes to hold callback functions before passing the
callbacks into libopcodes.

Because libopcodes is C code, and on some (many?) targets, C code is
compiled without exception support, it is important that GDB not try
to throw an exception over libopcode code.

In the previous commit all the existing callbacks were marked as
noexcept, however, this doesn't protect us from a future change to GDB
either adding a new callback that is not noexcept, or removing the
noexcept keyword from an existing callback.

In this commit I mark all the callback types as noexcept.  This means
that GDB's disassembler classes will no longer compile if we try to
pass a callback that is not marked as noexcept.

At least, that's the idea.  Unfortunately, it's not that easy.

Before C++17, the noexcept keyword on a function typedef would be
ignored, thus:

  using func_type = void (*) (void) noexcept;

  void
  a_func ()
  {
    throw 123;
  }

  void
  some_func (func_type f)
  {
    f ();
  }

  int
  main ()
  {
    some_func (a_func);
    return 0;
  }

Will compile just fine for C++11 and C++14 with GCC.  Clang on the
other hand complains that 'noexcept' should not appear on function
types, but then does appear to correctly complain that passing a_func
is a mismatch in the set of exceptions that could be thrown.

Switching to C++17 and both GCC and Clang correctly point out that
passing a_func is an invalid conversion relating to the noexcept
keyword.  Changing a_func to:

  void
  a_func () noexcept
  { /* Nothing.  */ }

And for C++17 both GCC and Clang compile this just fine.

My conclusion then is that adding the noexcept keyword to the function
types is pointless while GDB is not compiled as C++17, and silencing
the warnings would require us to jump through a bunch of hoops.

And so, in this commit, I define a macro LIBOPCODE_CALLBACK_NOEXCEPT,
this macro expands to noexcept when compiling for C++17, but otherwise
expands to nothing.  I then add this macro to the function types.

I've compiled GDB as the default C++11 and also forced the compile to
C++17.  When compiling as C++17 I spotted a few additional places
where callbacks needed to be marked noexcept (these fixes were merged
into the previous commit, but this confirmed to be that the macro is
working as expected).
---
 gdb/disasm.h | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/gdb/disasm.h b/gdb/disasm.h
index 58c6c623098..d6aec9a4705 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -26,6 +26,12 @@ struct gdbarch;
 struct ui_out;
 struct ui_file;
 
+#if __cplusplus >= 201703L
+#define LIBOPCODE_CALLBACK_NOEXCEPT noexcept
+#else
+#define LIBOPCODE_CALLBACK_NOEXCEPT
+#endif
+
 /* A wrapper around a disassemble_info and a gdbarch.  This is the core
    set of data that all disassembler sub-classes will need.  This class
    doesn't actually implement the disassembling process, that is something
@@ -51,18 +57,28 @@ struct gdb_disassemble_info
 
 protected:
 
-  /* Types for the function callbacks within m_di.  It would be nice if
-     these function types were all defined to include the noexcept
-     keyword, as every implementation of these must be noexcept.  However,
-     using noexcept within a function typedef like this is a C++17
-     feature, trying to do this for earlier C++ versions results in a
-     warning from GCC/Clang, and the noexcept isn't checked.  After we
-     move to C++17 these should be updated to add noexcept.  */
-  using read_memory_ftype = decltype (disassemble_info::read_memory_func);
-  using memory_error_ftype = decltype (disassemble_info::memory_error_func);
-  using print_address_ftype = decltype (disassemble_info::print_address_func);
-  using fprintf_ftype = decltype (disassemble_info::fprintf_func);
-  using fprintf_styled_ftype = decltype (disassemble_info::fprintf_styled_func);
+  /* Types for the function callbacks within m_di.  The actual function
+     signatures here are taken from include/dis-asm.h.  The noexcept macro
+     expands to 'noexcept' for C++17 and later, otherwise, it expands to
+     nothing.  This is because including noexcept was ignored for function
+     types before C++17, but both GCC and Clang warn that the noexcept
+     will become relevant when you switch to C++17, and this warning
+     causes the build to fail.  */
+  using read_memory_ftype
+    = int (*) (bfd_vma, bfd_byte *, unsigned int, struct disassemble_info *)
+	LIBOPCODE_CALLBACK_NOEXCEPT;
+  using memory_error_ftype
+    = void (*) (int, bfd_vma, struct disassemble_info *)
+	LIBOPCODE_CALLBACK_NOEXCEPT;
+  using print_address_ftype
+    = void (*) (bfd_vma, struct disassemble_info *)
+	LIBOPCODE_CALLBACK_NOEXCEPT;
+  using fprintf_ftype
+    = int (*) (void *, const char *, ...)
+	LIBOPCODE_CALLBACK_NOEXCEPT;
+  using fprintf_styled_ftype
+    = int (*) (void *, enum disassembler_style, const char *, ...)
+	LIBOPCODE_CALLBACK_NOEXCEPT;
 
   /* Constructor, many fields in m_di are initialized from GDBARCH.  The
      remaining arguments are function callbacks that are written into m_di.
-- 
2.25.4


  parent reply	other threads:[~2022-11-18 16:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-24 12:50 [PATCH] gdb/python: avoid throwing an exception over libopcodes code Andrew Burgess
2022-10-24 15:50 ` Simon Marchi
2022-10-24 17:22   ` Andrew Burgess
2022-10-24 17:45     ` [PATCH] gdb/disasm: mark functions passed to the disassembler noexcept Andrew Burgess
2022-10-24 18:24       ` Simon Marchi
2022-10-24 18:20     ` [PATCH] gdb/python: avoid throwing an exception over libopcodes code Simon Marchi
2022-10-27 10:38       ` Andrew Burgess
2022-10-27 15:38 ` [PATCHv2 0/3] " Andrew Burgess
2022-10-27 15:38   ` [PATCHv2 1/3] " Andrew Burgess
2022-11-28 14:39     ` Simon Marchi
2022-11-28 19:26       ` Andrew Burgess
2022-10-27 15:38   ` [PATCHv2 2/3] gdb/disasm: mark functions passed to the disassembler noexcept Andrew Burgess
2022-10-27 15:38   ` [PATCHv2 3/3] gdb: mark disassembler function callback types as noexcept Andrew Burgess
2022-11-18 16:57   ` [PATCHv3 0/3] gdb/python: avoid throwing an exception over libopcodes code Andrew Burgess
2022-11-18 16:57     ` [PATCHv3 1/3] " Andrew Burgess
2022-11-18 16:57     ` [PATCHv3 2/3] gdb/disasm: mark functions passed to the disassembler noexcept Andrew Burgess
2022-11-18 16:57     ` Andrew Burgess [this message]
2022-11-28  8:35     ` [PATCHv3 0/3] gdb/python: avoid throwing an exception over libopcodes code Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ca85925e33d8e5084bf80d315a3a5ffe657ee2e1.1668790576.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).