Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: Pedro Alves <palves@redhat.com>
Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>,
	       gdb-patches@sourceware.org
Subject: Re: [patch+NEWS] Avoid false valgrind warnings on linux_ptrace_test_ret_to_nx
Date: Mon, 25 Feb 2013 14:33:00 -0000	[thread overview]
Message-ID: <20130225143242.GB25286@host2.jankratochvil.net> (raw)
In-Reply-To: <5125082A.8040901@redhat.com>

On Wed, 20 Feb 2013 18:30:18 +0100, Pedro Alves wrote:
> On 02/20/2013 04:08 PM, Jan Kratochvil wrote:
> > OTOH here you ask for the opposite, unchecked inclusion of include file
> > which is very complicated
> > with compiler-dependent parts and arch-specific code,
> > which all seem to me to possibly cause compilation failures.
> 
> Now I'm at a total loss.  I have no idea what you're considering
> very complicated, and what needs to be compiler dependent.

> All I was thinking was something like the patch below.

It primarily misses the --with-valgrind feature: to error out when user does
want the (valgrind) support but system does not have the prerequisite files.

As the packaging system may omit some of the prerequisites (such as when PKG
splits out its PKG-devel subpackage) one needs to ensure such build will fail.
We do not want to quietly build new GDB missing some of the expected features.
--with-valgrind makes this easy.  Otherwise the packager needs to place in
the .spec file:
	grep HAVE_VALGRIND_VALGRIND_H gdb/config.h
Moreover multiple times as config.h gets sometimes rebuilt during some build
scenarios.

For example Fedora gdb.spec already has such a hack because there is no
corresponding --with-*/--enable-* option for this one:
	! grep '_RELOCATABLE.*1' gdb/config.h

It may be clear I do not want to introduce more such greps there.


So with the --with-valgrind requirement we end up discussing whether these 8
lines of code should or should not be there:

+    AC_MSG_CHECKING([for RUNNING_ON_VALGRIND macro])
+    AC_CACHE_VAL(gdb_cv_check_running_on_valgrind, [
+      AC_LINK_IFELSE(
+       [AC_LANG_PROGRAM([[#include <valgrind/valgrind.h>]],
+         [[return RUNNING_ON_VALGRIND;]])],
+       [gdb_cv_check_running_on_valgrind=yes],
+       [gdb_cv_check_running_on_valgrind=no])])
+    AC_MSG_RESULT($gdb_cv_check_running_on_valgrind)

I believe there is a real risk there can exist old system(s) on non-x86* arch
having valgrind/valgrind.h available but failing to build RUNNING_ON_VALGRIND.

Sure people have different opinions what is useful and what is just a bloat.
Although when I already wrote the code I do not find such a serious problem to
put there such 8 lines to sanity check the compatibility.


Jan


  reply	other threads:[~2013-02-25 14:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-19 18:42 [patch] " Jan Kratochvil
2013-02-19 19:11 ` Pedro Alves
2013-02-20 14:06   ` Jan Kratochvil
2013-02-20 15:06     ` Pedro Alves
2013-02-20 15:24       ` [patch+NEWS] " Jan Kratochvil
2013-02-20 15:56         ` Pedro Alves
2013-02-20 16:08           ` Jan Kratochvil
2013-02-20 17:30             ` Pedro Alves
2013-02-25 14:33               ` Jan Kratochvil [this message]
2013-02-25 18:10                 ` Pedro Alves
2013-02-26  3:00                   ` Jan Kratochvil
2013-02-20 20:15         ` Andreas Schwab
2013-02-25 14:19           ` Jan Kratochvil
2013-02-19 21:09 ` [patch] " Philippe Waroquiers

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=20130225143242.GB25286@host2.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    --cc=philippe.waroquiers@skynet.be \
    /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