diff mbox series

[v3,2/3] gdb: wrap mdebug debuginfo reading in ifdefs

Message ID 20250404201050.3857371-3-guinevere@redhat.com
State New
Headers
Series Allow debuginfo support to be selected at configure time |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed

Commit Message

Guinevere Larsen April 4, 2025, 8:10 p.m. UTC 
  This commit aims to allow a user to enable or disable mdebug support at
compilation time. To do that, a new configure option is added, called
--enable-gdb-mdebug-support (and the accompanying --disable version). By
default, support is enabled, and if a user decides to disable support,
the file mdebugread.c won't be compiled in the final binary, and the
macro MDEBUG_FORMAT_AVAILABLE won't be defined.

That macro is used to control the definitions of mdebug reading, either
the actual definition in mdebugread.c, or a static inline version that
only emits the following warning:

> No mdebug support available.

Ideally, we'd like to guard the entirity of mdebugread in the macro, but
the alpha-mdebug-tdep file uses those directly, and I don't think we
should restrict alpha hosts to requiring that debug format compiled in,
nor do I understand the tdep file enough to be comfortable disentangling
the requirements.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/Makefile.in  |  1 -
 gdb/NEWS         |  4 ++++
 gdb/config.in    |  3 +++
 gdb/configure    | 33 +++++++++++++++++++++++++++++++--
 gdb/configure.ac | 14 ++++++++++++++
 gdb/mdebugread.c |  2 +-
 gdb/mdebugread.h | 25 +++++++++++++++++++++----
 7 files changed, 74 insertions(+), 8 deletions(-)

Comments

Andrew Burgess April 10, 2025, 9:33 a.m. UTC | #1 
Guinevere Larsen <guinevere@redhat.com> writes:

> This commit aims to allow a user to enable or disable mdebug support at
> compilation time. To do that, a new configure option is added, called
> --enable-gdb-mdebug-support (and the accompanying --disable version). By
> default, support is enabled, and if a user decides to disable support,
> the file mdebugread.c won't be compiled in the final binary, and the
> macro MDEBUG_FORMAT_AVAILABLE won't be defined.
>
> That macro is used to control the definitions of mdebug reading, either
> the actual definition in mdebugread.c, or a static inline version that
> only emits the following warning:
>
>> No mdebug support available.
>
> Ideally, we'd like to guard the entirity of mdebugread in the macro, but
> the alpha-mdebug-tdep file uses those directly, and I don't think we
> should restrict alpha hosts to requiring that debug format compiled in,
> nor do I understand the tdep file enough to be comfortable disentangling
> the requirements.

Thanks for this.  I have a few of the smallest nits every, but
otherwise, this looks great.

> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 2411b10ca40..0175f50fd06 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -192,6 +192,20 @@ AS_HELP_STRING([--enable-targets=TARGETS], [alternative target configurations]),
>  esac])
>  
>  
> +# Check whether to support mdebug/ecoff debug information

Needs '.' at the end of this sentence, which will mean regenerating the
configure file.

> diff --git a/gdb/mdebugread.h b/gdb/mdebugread.h
> index 3ef8f5163b1..7d33e2d5285 100644
> --- a/gdb/mdebugread.h
> +++ b/gdb/mdebugread.h
> @@ -37,10 +37,7 @@ struct mdebug_extra_func_info
>  
>  #define MDEBUG_EFI_SYMBOL_NAME "__GDB_EFI_INFO__"
>  
> -extern void mdebug_build_psymtabs (minimal_symbol_reader &,
> -				   struct objfile *,
> -				   const struct ecoff_debug_swap *,
> -				   struct ecoff_debug_info *);
> +#if defined(MDEBUG_FORMAT_AVAILABLE)
>  
>  extern void elfmdebug_build_psymtabs (struct objfile *,
>  				      const struct ecoff_debug_swap *,
> @@ -54,4 +51,24 @@ extern void mipsmdebug_build_psymtabs (struct objfile *,
>  				       const struct ecoff_debug_swap *,
>  				       struct ecoff_debug_info *);
>  
> +#else /* MDEBUG_FORMAT_AVAILABLE */
> +
> +static inline void
> +elfmdebug_build_psymtabs (struct objfile *,
> +			  const struct ecoff_debug_swap *,
> +			  asection *)
> +{
> +  warning ("No mdebug support available");

This warning should wrap the string in _(...).

> +}
> +
> +static inline void
> +mipsmdebug_build_psymtabs (struct objfile *,
> +			   const struct ecoff_debug_swap *,
> +			   struct ecoff_debug_info *)
> +{
> +  warning ("No mdebug support available");

And again here with _(...)

With these minor nits fixed:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> +}
> +
> +#endif /* MDEBUG_FORMAT_AVAILABLE */
> +
>  #endif /* GDB_MDEBUGREAD_H */
> -- 
> 2.49.0
Guinevere Larsen April 10, 2025, 1:43 p.m. UTC | #2 
On 4/10/25 6:33 AM, Andrew Burgess wrote:
> Guinevere Larsen <guinevere@redhat.com> writes:
>
>> This commit aims to allow a user to enable or disable mdebug support at
>> compilation time. To do that, a new configure option is added, called
>> --enable-gdb-mdebug-support (and the accompanying --disable version). By
>> default, support is enabled, and if a user decides to disable support,
>> the file mdebugread.c won't be compiled in the final binary, and the
>> macro MDEBUG_FORMAT_AVAILABLE won't be defined.
>>
>> That macro is used to control the definitions of mdebug reading, either
>> the actual definition in mdebugread.c, or a static inline version that
>> only emits the following warning:
>>
>>> No mdebug support available.
>> Ideally, we'd like to guard the entirity of mdebugread in the macro, but
>> the alpha-mdebug-tdep file uses those directly, and I don't think we
>> should restrict alpha hosts to requiring that debug format compiled in,
>> nor do I understand the tdep file enough to be comfortable disentangling
>> the requirements.
> Thanks for this.  I have a few of the smallest nits every, but
> otherwise, this looks great.
>
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index 2411b10ca40..0175f50fd06 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -192,6 +192,20 @@ AS_HELP_STRING([--enable-targets=TARGETS], [alternative target configurations]),
>>   esac])
>>   
>>   
>> +# Check whether to support mdebug/ecoff debug information
> Needs '.' at the end of this sentence, which will mean regenerating the
> configure file.
>
>> diff --git a/gdb/mdebugread.h b/gdb/mdebugread.h
>> index 3ef8f5163b1..7d33e2d5285 100644
>> --- a/gdb/mdebugread.h
>> +++ b/gdb/mdebugread.h
>> @@ -37,10 +37,7 @@ struct mdebug_extra_func_info
>>   
>>   #define MDEBUG_EFI_SYMBOL_NAME "__GDB_EFI_INFO__"
>>   
>> -extern void mdebug_build_psymtabs (minimal_symbol_reader &,
>> -				   struct objfile *,
>> -				   const struct ecoff_debug_swap *,
>> -				   struct ecoff_debug_info *);
>> +#if defined(MDEBUG_FORMAT_AVAILABLE)
>>   
>>   extern void elfmdebug_build_psymtabs (struct objfile *,
>>   				      const struct ecoff_debug_swap *,
>> @@ -54,4 +51,24 @@ extern void mipsmdebug_build_psymtabs (struct objfile *,
>>   				       const struct ecoff_debug_swap *,
>>   				       struct ecoff_debug_info *);
>>   
>> +#else /* MDEBUG_FORMAT_AVAILABLE */
>> +
>> +static inline void
>> +elfmdebug_build_psymtabs (struct objfile *,
>> +			  const struct ecoff_debug_swap *,
>> +			  asection *)
>> +{
>> +  warning ("No mdebug support available");
> This warning should wrap the string in _(...).
>
>> +}
>> +
>> +static inline void
>> +mipsmdebug_build_psymtabs (struct objfile *,
>> +			   const struct ecoff_debug_swap *,
>> +			   struct ecoff_debug_info *)
>> +{
>> +  warning ("No mdebug support available");
> And again here with _(...)
>
> With these minor nits fixed:
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks for the review! I applied all the fixes and the tag to this commit :)
Andrew Burgess April 11, 2025, 10:10 a.m. UTC | #3 
Guinevere Larsen <guinevere@redhat.com> writes:

> This commit aims to allow a user to enable or disable mdebug support at
> compilation time. To do that, a new configure option is added, called
> --enable-gdb-mdebug-support (and the accompanying --disable version). By
> default, support is enabled, and if a user decides to disable support,
> the file mdebugread.c won't be compiled in the final binary, and the
> macro MDEBUG_FORMAT_AVAILABLE won't be defined.
>
> That macro is used to control the definitions of mdebug reading, either
> the actual definition in mdebugread.c, or a static inline version that
> only emits the following warning:
>
>> No mdebug support available.
>
> Ideally, we'd like to guard the entirity of mdebugread in the macro, but
> the alpha-mdebug-tdep file uses those directly, and I don't think we
> should restrict alpha hosts to requiring that debug format compiled in,
> nor do I understand the tdep file enough to be comfortable disentangling
> the requirements.
>
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> ---
>  gdb/Makefile.in  |  1 -
>  gdb/NEWS         |  4 ++++

I notice that patch #3 has an entry for gdb/README, but this one
doesn't, though they both add a new configure option.  Is this an
oversight?

Thanks,
Andrew
diff mbox series

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0c4102d2565..503326869c8 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1170,7 +1170,6 @@  COMMON_SFILES = \
 	maint.c \
 	maint-test-options.c \
 	maint-test-settings.c \
-	mdebugread.c \
 	mem-break.c \
 	memattr.c \
 	memory-map.c \
diff --git a/gdb/NEWS b/gdb/NEWS
index 6a557bb4af9..fc8f4ed8bf0 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -116,6 +116,10 @@  qXfer:threads:read
   subsystem to be disabled at configure time, in the form of
   --disable-gdb-compile.
 
+* A new configure option was added, allowing support for mdebug/ecoff
+  debug information to be disabled at configure time.  The flag to do
+  that is --disable-gdb-mdebug-support.
+
 *** Changes in GDB 16
 
 * Support for Nios II targets has been removed as this architecture
diff --git a/gdb/config.in b/gdb/config.in
index 86ff67d2898..8e016fa7c34 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -639,6 +639,9 @@ 
    */
 #undef LT_OBJDIR
 
+/* defined if mdebug format was requested. */
+#undef MDEBUG_FORMAT_AVAILABLE
+
 /* Name of this package. */
 #undef PACKAGE
 
diff --git a/gdb/configure b/gdb/configure
index e8a649f2ef0..1d5be360a43 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -934,6 +934,7 @@  with_relocated_sources
 with_auto_load_dir
 with_auto_load_safe_path
 enable_targets
+enable_gdb_mdebug_support
 enable_64_bit_bfd
 with_amd_dbgapi
 enable_tui
@@ -1646,6 +1647,9 @@  Optional Features:
   --disable-nls           do not use Native Language Support
   --enable-targets=TARGETS
                           alternative target configurations
+  --enable-gdb-mdebug-support
+                          Enable support for the mdebug debuginfo format
+                          (default 'yes')
   --enable-64-bit-bfd     64-bit support (on hosts with narrower word sizes)
   --enable-tui            enable full-screen terminal user interface (TUI)
   --enable-gdbtk          enable gdbtk graphical user interface (GUI)
@@ -11503,7 +11507,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11506 "configure"
+#line 11510 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11609,7 +11613,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11612 "configure"
+#line 11616 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -24879,6 +24883,31 @@  fi
 
 
 
+# Check whether to support mdebug/ecoff debug information
+# Check whether --enable-gdb-mdebug-support was given.
+if test "${enable_gdb_mdebug_support+set}" = set; then :
+  enableval=$enable_gdb_mdebug_support;
+	   case $enableval in
+	     yes | no)
+	       ;;
+	     *)
+	       as_fn_error $? "bad value $enableval for --enable-gdb-mdebug-support" "$LINENO" 5
+	       ;;
+	   esac
+
+else
+  enable_gdb_mdebug_support=yes
+fi
+
+
+if test "x${enable_gdb_mdebug_support}" != "xno"; then
+    CONFIG_SRCS="$CONFIG_SRCS mdebugread.c"
+    CONFIG_OBS="$CONFIG_OBS mdebugread.o"
+
+$as_echo "#define MDEBUG_FORMAT_AVAILABLE 1" >>confdefs.h
+
+fi
+
 # Check whether --enable-64-bit-bfd was given.
 if test "${enable_64_bit_bfd+set}" = set; then :
   enableval=$enable_64_bit_bfd; case $enableval in #(
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 2411b10ca40..0175f50fd06 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -192,6 +192,20 @@  AS_HELP_STRING([--enable-targets=TARGETS], [alternative target configurations]),
 esac])
 
 
+# Check whether to support mdebug/ecoff debug information
+AC_ARG_ENABLE(gdb-mdebug-support,
+AS_HELP_STRING([--enable-gdb-mdebug-support],
+	       [Enable support for the mdebug debuginfo format (default 'yes')]),
+[GDB_CHECK_YES_NO_VAL([$enableval], [--enable-gdb-mdebug-support])],
+[enable_gdb_mdebug_support=yes])
+
+if test "x${enable_gdb_mdebug_support}" != "xno"; then
+    CONFIG_SRCS="$CONFIG_SRCS mdebugread.c"
+    CONFIG_OBS="$CONFIG_OBS mdebugread.o"
+    AC_DEFINE(MDEBUG_FORMAT_AVAILABLE, 1,
+	      [defined if mdebug format was requested.])
+fi
+
 BFD_64_BIT
 
 # Provide defaults for some variables set by the per-host and per-target
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index b4f3d3ad914..cfa541f795a 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -329,7 +329,7 @@  fdr_name (FDR *f)
 /* Read in and parse the symtab of the file OBJFILE.  Symbols from
    different sections are relocated via the SECTION_OFFSETS.  */
 
-void
+static void
 mdebug_build_psymtabs (minimal_symbol_reader &reader,
 		       struct objfile *objfile,
 		       const struct ecoff_debug_swap *swap,
diff --git a/gdb/mdebugread.h b/gdb/mdebugread.h
index 3ef8f5163b1..7d33e2d5285 100644
--- a/gdb/mdebugread.h
+++ b/gdb/mdebugread.h
@@ -37,10 +37,7 @@  struct mdebug_extra_func_info
 
 #define MDEBUG_EFI_SYMBOL_NAME "__GDB_EFI_INFO__"
 
-extern void mdebug_build_psymtabs (minimal_symbol_reader &,
-				   struct objfile *,
-				   const struct ecoff_debug_swap *,
-				   struct ecoff_debug_info *);
+#if defined(MDEBUG_FORMAT_AVAILABLE)
 
 extern void elfmdebug_build_psymtabs (struct objfile *,
 				      const struct ecoff_debug_swap *,
@@ -54,4 +51,24 @@  extern void mipsmdebug_build_psymtabs (struct objfile *,
 				       const struct ecoff_debug_swap *,
 				       struct ecoff_debug_info *);
 
+#else /* MDEBUG_FORMAT_AVAILABLE */
+
+static inline void
+elfmdebug_build_psymtabs (struct objfile *,
+			  const struct ecoff_debug_swap *,
+			  asection *)
+{
+  warning ("No mdebug support available");
+}
+
+static inline void
+mipsmdebug_build_psymtabs (struct objfile *,
+			   const struct ecoff_debug_swap *,
+			   struct ecoff_debug_info *)
+{
+  warning ("No mdebug support available");
+}
+
+#endif /* MDEBUG_FORMAT_AVAILABLE */
+
 #endif /* GDB_MDEBUGREAD_H */