[RFA] Add support for --enable-lmcheck configure option.

[RFA] Add support for --enable-lmcheck configure option.

[Joel Brobecker]

Hello,

This allows the user to enable this option at configure time if building
a release, or to disable it if building a snapshot. It might be useful
later on, when debugging memory problems on a release version of GDB.
I don't think I'll personally use the --disable-libmcheck on a snapshot,
but someone distributing a snaphot version might enjoy it too. The
default is to preserve the current behavior.

Also, I went the minimal route: If --enable-libmcheck is specified,
but AC_CHECK_LIB(mcheck) fails, configure is not going to fail. I think
it's OK, given the relative importance of this feature. I suppose
I could add an extra check when --enable-libmcheck is set but libmcheck
support could not be found, if people think it's important.

gdb/ChangeLog:

        * configure.ac: Add --enable-lmcheck configure option.
        * configure: Regenerate.

Tested on x86_64-linux, with and without --disable-lmcheck.
OK?

---
 gdb/configure    |   23 ++++++++++++++++++++++-
 gdb/configure.ac |   21 +++++++++++++++++++--
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/gdb/configure.ac b/gdb/configure.ac
index e48ec05..5771825 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -645,8 +645,25 @@ AC_SUBST(READLINE_DEPS)
 AC_SUBST(READLINE_CFLAGS)
 AC_SUBST(READLINE_TEXI_INCFLAG)
 
-dnl -lmcheck provides cheap enough memory mangling for debugging purposes.
-if $development; then
+# Provide a --enable-libmcheck/--disable-libmcheck set of options
+# allowing a user to enable this option even when building releases,
+# or to disable it when building a snapshot.
+AC_ARG_ENABLE(libmcheck,
+  AS_HELP_STRING([--enable-libmcheck],
+                 [Try building GDB with -lmcheck if available]),
+  [case "${enableval}" in
+     yes | y) ENABLE_LIBMCHECK="yes" ;;
+     no | n)  ENABLE_LIBMCHECK="no" ;;
+     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
+   esac])
+
+# Enable -lmcheck by default (it provides cheap-enough memory mangling),
+# but turn it off for releases.
+if test -z "${ENABLE_LIBMCHECK}" && $development; then
+    ENABLE_LIBMCHECK=yes
+fi
+
+if test "$ENABLE_LIBMCHECK" = "yes" ; then
   AC_CHECK_LIB(mcheck, main)
 fi
 
diff --git a/gdb/configure b/gdb/configure
index 1d03188..1ea4ee9 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -795,6 +795,7 @@ with_zlib
 with_libiconv_prefix
 with_iconv_bin
 with_system_readline
+enable_libmcheck
 with_jit_reader_dir
 with_expat
 with_gnu_ld
@@ -1465,6 +1466,7 @@ Optional Features:
   --enable-tui            enable full-screen terminal user interface (TUI)
   --enable-gdbtk          enable gdbtk graphical user interface (GUI)
   --enable-profiling      enable profiling of GDB
+  --enable-libmcheck      Try building GDB with -lmcheck if available
   --disable-rpath         do not hardcode runtime library paths
   --enable-werror         treat compile warnings as errors
   --enable-build-warnings enable build-time compiler warnings if gcc is used
@@ -7039,7 +7041,26 @@ fi
 
 
 
-if $development; then
+# Provide a --enable-libmcheck/--disable-libmcheck set of options
+# allowing a user to enable this option even when building releases,
+# or to disable it when building a snapshot.
+# Check whether --enable-libmcheck was given.
+if test "${enable_libmcheck+set}" = set; then :
+  enableval=$enable_libmcheck; case "${enableval}" in
+     yes | y) ENABLE_LIBMCHECK="yes" ;;
+     no | n)  ENABLE_LIBMCHECK="no" ;;
+     *) as_fn_error "bad value ${enableval} for --enable-libmcheck" "$LINENO" 5 ;;
+   esac
+fi
+
+
+# Enable -lmcheck by default (it provides cheap-enough memory mangling),
+# but turn it off for releases.
+if test -z "${ENABLE_LIBMCHECK}" && $development; then
+    ENABLE_LIBMCHECK=yes
+fi
+
+if test "$ENABLE_LIBMCHECK" = "yes" ; then
   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for main in -lmcheck" >&5
 $as_echo_n "checking for main in -lmcheck... " >&6; }
 if test "${ac_cv_lib_mcheck_main+set}" = set; then :
-- 
1.7.1

Re: [RFA] Add support for --enable-lmcheck configure option.

[Jan Kratochvil]

Hi Joel,

On Mon, 11 Jun 2012 22:38:47 +0200, Joel Brobecker wrote:
> This allows the user to enable this option at configure time if building
> a release,
[...]
> It might be useful
> later on, when debugging memory problems on a release version of GDB.

this is bloating the configure script + configure help.  Just use:
	LDFLAGS=-lmcheck ./configure ...


> I don't think I'll personally use the --disable-libmcheck on a snapshot,
> but someone distributing a snaphot version might enjoy it too.

Yes:
	http://pkgs.fedoraproject.org/gitweb/?p=gdb.git;a=blob_plain;f=gdb-disable-mcheck.patch;hb=master

But I do not find it so important to make a configure option to it.


> OK?

Given that --disable-libmcheck for snapshot may make some sense I do not
oppose it but I do not think it is worth it, it is a corner case where one can
edit configure or Makefile IMO.

I understand the reason is to workaround current GDB bugs exposed by -lmcheck.
This is sure not the right solution.


Regards,
Jan

Re: [RFA] Add support for --enable-lmcheck configure option.

[Joel Brobecker]

> this is bloating the configure script + configure help.  Just use:
> 	LDFLAGS=-lmcheck ./configure ...
> 
> 
> > I don't think I'll personally use the --disable-libmcheck on a snapshot,
> > but someone distributing a snaphot version might enjoy it too.
> 
> Yes:
> 	http://pkgs.fedoraproject.org/gitweb/?p=gdb.git;a=blob_plain;f=gdb-disable-mcheck.patch;hb=master
> 
> But I do not find it so important to make a configure option to it.

I read your answer as: "I do not object, but I don't think it is useful".
Therefore, I would like to keep the patch on the table and see if there
are objections. Otherwise, I'd like to commit it, because I think it is
going to be useful.

> Given that --disable-libmcheck for snapshot may make some sense I do not
> oppose it but I do not think it is worth it, it is a corner case where one can
> edit configure or Makefile IMO.

I don't want people to have to modify the sources or maintain such
a modification in their sources. I think that this is unfriendly,
especially given the small size of the patch. Saying that is is
bloating (sic) the configure script and help is a little extreme,
if you ask me.

> I understand the reason is to workaround current GDB bugs exposed by -lmcheck.
> This is sure not the right solution.

This is absolutely wrong. Maybe some people might want to use it that
way, but that is not my goal. It seemed easy to assume that, but that
is not the case. I think it is a very valuable addition for developers,
and I just fixed one buffer overflow thanks to it yesterday. And I expect
that all AdaCore developers will be building GDB with -lmcheck, even
the release versions.

-- 
Joel

Re: [RFA] Add support for --enable-lmcheck configure option.

[Jan Kratochvil]

On Tue, 12 Jun 2012 18:55:23 +0200, Joel Brobecker wrote:
> This is absolutely wrong. Maybe some people might want to use it that
> way, but that is not my goal. It seemed easy to assume that, but that
> is not the case. I think it is a very valuable addition for developers,
> and I just fixed one buffer overflow thanks to it yesterday. And I expect
> that all AdaCore developers will be building GDB with -lmcheck, even
> the release versions.

OK, if it is therefore for the purpose of regular use of a development
snapshot (like what I did for Fedora Rawhide) then I agree it makes sense.

Still:

> I read your answer as: "I do not object, but I don't think it is useful".


Thanks,
Jan

Re: [RFA] Add support for --enable-lmcheck configure option.

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * configure.ac: Add --enable-lmcheck configure option.
>         * configure: Regenerate.

FYI: I checked this patch in...

-- 
Joel