From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7025 invoked by alias); 20 Feb 2013 14:06:30 -0000 Received: (qmail 6950 invoked by uid 22791); 20 Feb 2013 14:06:28 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,FAKE_REPLY_C,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 20 Feb 2013 14:06:09 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1KE5Rwa019522 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 20 Feb 2013 09:05:27 -0500 Received: from host2.jankratochvil.net (ovpn-116-18.ams2.redhat.com [10.36.116.18]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1KE5LoW008380 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 20 Feb 2013 09:05:24 -0500 Date: Wed, 20 Feb 2013 14:06:00 -0000 From: Jan Kratochvil To: Pedro Alves , Philippe Waroquiers Cc: gdb-patches@sourceware.org Subject: Re: [patch] Avoid false valgrind warnings on linux_ptrace_test_ret_to_nx Message-ID: <20130220140520.GA10822@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1361308206.2244.1.camel@soleil> <5123CE5E.6090202@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-02/txt/msg00531.txt.bz2 On Tue, 19 Feb 2013 20:11:26 +0100, Pedro Alves wrote: > On 02/19/2013 06:42 PM, Jan Kratochvil wrote: > > + [AS_HELP_STRING([--without-valgrind], > > + [do not include false valgrind warning message skip])],, > > I have trouble parsing this help string. I suggest instead: > > "do not include support for running-on-valgrind detection" The problem is this message does not say what is it good for from the user point of view. But used it as I do not mind which message is there, particularly as I agree the message of mine was very cryptic. > (Nit, the mmap warning is not really false, as the code is > doing what the warning says. It's only that it's annoying.) >From the user point of view valgrind should report problems in the inferior. This test is not a problem in GDB. > > + AC_ERROR([--with-valgrind was requested but it has not been found]) > > A reader is left confused over what's "it"? A possible simple/small > fix is just s/it/support/ or s/it/necessary support/. OK, so why to be so abstract: [--with-valgrind was requested but was not found]) > /* Below we'll mmap a non-executable page, which under Valgrind > results in annoying warnings such as > "Bad permissions for mapped region at address 0xfoobar. > Just skip the whole test if running under Valgrind. */ Added there, with closing '"'. On Tue, 19 Feb 2013 22:10:06 +0100, Philippe Waroquiers wrote: > On Tue, 2013-02-19 at 19:42 +0100, Jan Kratochvil wrote: > > > +#ifdef HAVE_RUNNING_ON_VALGRIND > > + if (HAVE_RUNNING_ON_VALGRIND) > Isn't it rather > if (RUNNING_ON_VALGRIND) > which is needed ? > > + return; > > +#endif Oops. Thanks, Jan gdb/ 2013-02-20 Jan Kratochvil * acinclude.m4: Include common/acinclude.m4. * common/acinclude.m4: New file. * common/linux-ptrace.c : Include valgrind/valgrind.h. (linux_ptrace_test_ret_to_nx) : Return. * config.in: Regenerate. * configure: Regenerate. * configure.ac: Call GDB_AC_CHECK_VALGRIND. gdb/gdbserver/ 2013-02-20 Jan Kratochvil * acinclude.m4: Include ../common/acinclude.m4. * config.in: Regenerate. * configure: Regenerate. * configure.ac: Call GDB_AC_CHECK_VALGRIND. diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4 index 25caddd..12da851 100644 --- a/gdb/acinclude.m4 +++ b/gdb/acinclude.m4 @@ -49,6 +49,8 @@ sinclude([../config/codeset.m4]) sinclude([../config/zlib.m4]) +sinclude([common/acinclude.m4]) + ## ----------------------------------------- ## ## ANSIfy the C compiler whenever possible. ## ## From Franc,ois Pinard ## diff --git a/gdb/common/acinclude.m4 b/gdb/common/acinclude.m4 new file mode 100644 index 0000000..fcb1c71 --- /dev/null +++ b/gdb/common/acinclude.m4 @@ -0,0 +1,44 @@ +# Copyright 2013 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +dnl GDB_AC_CHECK_VALGRIND +dnl Check for HAVE_RUNNING_ON_VALGRIND. +dnl If such symbol is defined is also available. +AC_DEFUN([GDB_AC_CHECK_VALGRIND], [ + AC_ARG_WITH([valgrind], + [AS_HELP_STRING([--without-valgrind], + [do not include support for running-on-valgrind detection])],, + [with_valgrind=check]) + if test "$with_valgrind" != no; then + AC_MSG_CHECKING([for RUNNING_ON_VALGRIND macro]) + AC_CACHE_VAL(gdb_cv_check_running_on_valgrind, [ + AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[#include ]], + [[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) + if test "$gdb_cv_check_running_on_valgrind" = yes; then + with_valgrind=yes + elif test "$with_valgrind" = yes; then + AC_ERROR( + [--with-valgrind was requested but was not found]) + fi + fi + if test "$with_valgrind" = yes; then + AC_DEFINE(HAVE_RUNNING_ON_VALGRIND, 1, + [Define if you have and RUNNING_ON_VALGRIND.]) + fi +]) diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c index 886be80..97a314b 100644 --- a/gdb/common/linux-ptrace.c +++ b/gdb/common/linux-ptrace.c @@ -29,6 +29,10 @@ #include "gdb_assert.h" #include "gdb_wait.h" +#ifdef HAVE_RUNNING_ON_VALGRIND +# include +#endif + /* Find all possible reasons we could fail to attach PID and append these newline terminated reason strings to initialized BUFFER. '\0' termination of BUFFER must be done by the caller. */ @@ -76,6 +80,15 @@ linux_ptrace_test_ret_to_nx (void) long l; int status; + /* Below we'll mmap a non-executable page, which under Valgrind + results in annoying warnings such as + "Bad permissions for mapped region at address 0xfoobar". + Just skip the whole test if running under Valgrind. */ +#ifdef HAVE_RUNNING_ON_VALGRIND + if (RUNNING_ON_VALGRIND) + return; +#endif + return_address = mmap (NULL, 2, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); if (return_address == MAP_FAILED) diff --git a/gdb/configure.ac b/gdb/configure.ac index e501766..a7bac76 100644 --- a/gdb/configure.ac +++ b/gdb/configure.ac @@ -1166,6 +1166,7 @@ AC_CHECK_FUNCS([canonicalize_file_name realpath getrusage getuid getgid \ ttrace wborder wresize setlocale iconvlist libiconvlist btowc \ setrlimit getrlimit posix_madvise waitpid lstat]) AM_LANGINFO_CODESET +GDB_AC_CHECK_VALGRIND # Check the return and argument types of ptrace. No canned test for # this, so roll our own. diff --git a/gdb/gdbserver/acinclude.m4 b/gdb/gdbserver/acinclude.m4 index 0e0bdc8..c5a93b7 100644 --- a/gdb/gdbserver/acinclude.m4 +++ b/gdb/gdbserver/acinclude.m4 @@ -12,6 +12,8 @@ sinclude(../../config/acx.m4) m4_include(../../config/depstand.m4) m4_include(../../config/lead-dot.m4) +sinclude([../common/acinclude.m4]) + dnl Check for existence of a type $1 in libthread_db.h dnl Based on BFD_HAVE_SYS_PROCFS_TYPE in bfd/bfd.m4. diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac index 55fb461..d492e8b 100644 --- a/gdb/gdbserver/configure.ac +++ b/gdb/gdbserver/configure.ac @@ -71,6 +71,7 @@ AC_CHECK_HEADERS(sgtty.h termio.h termios.h sys/reg.h string.h dnl linux/perf_event.h) AC_CHECK_FUNCS(pread pwrite pread64 readlink) AC_REPLACE_FUNCS(vasprintf vsnprintf) +GDB_AC_CHECK_VALGRIND # Check for UST ustlibs=""