From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30097 invoked by alias); 20 Feb 2013 15:06:47 -0000 Received: (qmail 30060 invoked by uid 22791); 20 Feb 2013 15:06:45 -0000 X-SWARE-Spam-Status: No, hits=-7.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,KHOP_THREADED,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 15:06:27 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1KF6NTx015416 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 20 Feb 2013 10:06:23 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r1KF6LQO020470; Wed, 20 Feb 2013 10:06:22 -0500 Message-ID: <5124E66D.3060606@redhat.com> Date: Wed, 20 Feb 2013 15:06:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Jan Kratochvil CC: Philippe Waroquiers , gdb-patches@sourceware.org Subject: Re: [patch] Avoid false valgrind warnings on linux_ptrace_test_ret_to_nx References: <20130220140520.GA10822@host2.jankratochvil.net> In-Reply-To: <20130220140520.GA10822@host2.jankratochvil.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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/msg00532.txt.bz2 On 02/20/2013 02:05 PM, Jan Kratochvil wrote: > 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. That reminds us that ideally new configury options are all advertised in NEWS. > 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. GDB is valgrind's inferior, and it is doing something fishy, hence valgrind is printing the corresponding warning. IMO, a"false" warning is a warning that points at something that doesn't really exist, like all those "variable might be used uninitialized" compiler warnings when the variable is actually initialized in all the code paths that use it. But discussing this is pointless. > > >>> + 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]) I was originally going to suggest something like that, but didn't as I noticed the test doesn't look only for the header, it also tries to use RUNNING_ON_VALGRIND. I assumed there'd be a reason for the more complete test, instead of a simpler and unconditional AC_CHECK_HEADERS(valgrind/valgrind.h) and then just #ifdef RUNNING_ON_VALGRIND if (RUNNING_ON_VALGRIND) return; #endif As in, you had found that older valgrind/valgrind.h's didn't have that RUNNING_ON_VALGRIND macro. Hence saying the more general "support". So as the test as written, it didn't look 100% correct to point blame at the missing header. But really I won't argue more about it. We have bigger fish to fry. :-) > > >> /* 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 '"'. Thanks. I have no further comments. -- Pedro Alves