Re: [patch+NEWS] Avoid false valgrind warnings on linux_ptrace_test_ret_to_nx

Jan Kratochvil
2013-02-26T03:00:38Z

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