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: Tue, 26 Feb 2013 03:00:00 -0000	[thread overview]
Message-ID: <20130226025956.GA13814@host2.jankratochvil.net> (raw)
In-Reply-To: <512BA904.8080604@redhat.com>

On Mon, 25 Feb 2013 19:10:12 +0100, Pedro Alves wrote:
> as I didn't want to set precedent for adding checks for unknown problems
> without reason.  It's better to wait for the problems to appear, so at least
> our future selves wanting to change this code know what sort of problems to
> expect.

This contradicts your earlier mail for the same feature:
	Re: [patch+7.5] Fix assertion fail on PaX gentoo (PR 14564)
	http://sourceware.org/ml/gdb-patches/2012-09/msg00108.html
	Message-ID: <504DF20E.102@redhat.com>

	The linux_test_for_tracefork family of functions seems to warn instead
	of asserting if something unexpected comes out of ptrace/waitpid.
	Might be a good idea for this function too?

You wrote there first:

# It seems like one should be able to make them trigger
# by sending a signal to gdb's process group, or "killall gdb" at
# the wrong time?  E.g., a SIGTERM, a SIGINT, or a SIGSTOP? 

For this specific part I have to disagree, this does not solve the
warning-vs-assert problem, even if SIG* reception does not get handled
correctly still bogus warning gets reported.

If someone sends SIGTERM/SIGINT then it will kill also GDB process itself so
it is not an issue.  SIGSTOP should not be a problem as waitpid does not use
WSTOPPED there.  Just guessing, I have not tried either, though.

And by turning it assert -> warning makes the situation only worse (whether it
is worse or not is IIUC the subject of disagreement).  On assert it gets
automatically bugreport by ABRT and a fix gets possibly automatically
delivered in several days in a system update.  Contrary to it a warning annoys
people for a long time before someone finally bugreports it by hand, usually
omitting various details needed for a fix so it gets all expensive for user
+ developer to investigate the problem first.

So while it is clear what is better for normal modern systems like Fedora
there remains a question what is the direction of upstream GDB.  According to
your 2012 direction errors should be reported "softly".  In your current mail
you prefer rather build failure (instread of omitting the feature unsupported
by the build machine).

While I have discussed here only this specific issue of these two mails this
was always an issue with GDB, what should be an internal error vs. warning
only in some unexpected cases.



> I find the non-x86* argument somewhat compelling.  I do still
> think the mention of valgrind/valgrind.h here:
> 
>   [--with-valgrind was requested but <valgrind/valgrind.h> was not found])
> 
> isn't 100% correct, and will be misleading -- your non-x86* example
> failing to build RUNNING_ON_VALGRIND would show that error, though
> the header was indeed found.  If you don't like the other suggestion for
> "support" and you still want to mention the header, then adding a "working"
> would be fine with me:
> 
>   [--with-valgrind was requested but working <valgrind/valgrind.h> was not found])

OK.


> (*) - and I wish in commit logs - following Joel's lead, I've
> been personally making an effort to push self-contained commit
> logs with full change description myself.

I normally look up the mail thread around it.  It would be great to have some
automated tracking of commit vs. its mail (and therefore its mail thread).
The longer commit logs are fine but they do not replace the mail thread.


Jan


  reply	other threads:[~2013-02-26  3:00 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
2013-02-25 18:10                 ` Pedro Alves
2013-02-26  3:00                   ` Jan Kratochvil [this message]
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=20130226025956.GA13814@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