All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andi Kleen <andi@firstfloor.org>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Ingo Molnar <mingo@elte.hu>, Paul Mackerras <paulus@samba.org>,
	Corey Ashford <cjashfor@linux.vnet.ibm.com>,
	Vince Weaver <vincent.weaver@maine.edu>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call
Date: Wed, 17 Apr 2013 22:02:06 +0200	[thread overview]
Message-ID: <20130417200206.GA18253@redhat.com> (raw)
In-Reply-To: <20130416144226.GA12848@redhat.com>

On 04/16, Oleg Nesterov wrote:
>
> On 04/16, Frederic Weisbecker wrote:
> >
> > On Sun, Mar 10, 2013 at 07:41:06PM +0100, Jiri Olsa wrote:
> > > Adding RF EFLAGS bit to be restored on return from signal from
> > > the original register context before the signal was entered.
> > >
> > > This will prevent the RF flag to disappear when returning
> > > from exception due to the signal handler being executed.
> >
> > So that happens if, say, we get a breakpoint exception and then we
> > run a signal handler before returning to the ip that triggered the
> > breakpoint?
>
> Afaics these changes (1 and 2) should fix the bug.
>
> Suppose that the first insn in the signal handler should trigger
> another bp, we should clear X86_EFLAGS_RF (2/6).
>
> Otoh, we should restore it when we return to the original insn
> which triggered the trap to avoid another trap.

I applied 1 and 2, and this fixes the test-case below.

> But. it seems that we have yet another problem? Suppose that
> the signal handler does siglongjmp() and jumps to yet another
> insn which should trigger the trap?

Argh. Sorry for confusion. I tried to say that the signal handler
can play with sigcontext->ip before sigreturn(). But probably we
can ignore this.

Oleg.

-------------------------------------------------------------------------------
#define _GNU_SOURCE 1
#include <stdio.h>
#include <unistd.h>
#include <sys/ptrace.h>
#include <sys/debugreg.h>
#include <sys/user.h>
#include <sys/wait.h>
#include <assert.h>
#include <assert.h>
#include <stddef.h>

#define DR_GLOBAL_ENABLE	0x2
#define DR_LOCAL_ENABLE		0x1

unsigned long encode_dr7(int drnum, int enable, unsigned int type, unsigned int len)
{
	unsigned long dr7;

	dr7 = ((len | type) & 0xf)
		<< (DR_CONTROL_SHIFT + drnum * DR_CONTROL_SIZE);
	if (enable)
		dr7 |= (DR_GLOBAL_ENABLE << (drnum * DR_ENABLE_SIZE));

	return dr7;
}

int write_dr(int pid, int dr, unsigned long val)
{
	return ptrace(PTRACE_POKEUSER, pid,
			offsetof (struct user, u_debugreg[dr]),
			val);
}

#define GET_REG(pid, reg)				\
	ptrace(PTRACE_PEEKUSER, (pid),			\
		offsetof(struct user, regs.reg), 0)

void *get_ip(int pid)
{
	return (void*)GET_REG(pid, rip);
}


void func(void)
{
	printf("bp_1 passed\n");
}

void sigh(int sig)
{
	printf("bp_2 passed\n");
}

int main(void)
{
	int pid, stat;
	unsigned long dr7;

	pid = fork();
	if (!pid) {
		assert(ptrace(PTRACE_TRACEME, 0,0,0) == 0);
		kill(getpid(), SIGHUP);

		signal(SIGINT, sigh);

		func();

		return 0x13;
	}

	assert(pid == waitpid(-1, &stat, 0));
	assert(WSTOPSIG(stat) == SIGHUP);

	assert(write_dr(pid, 0, (long)func) == 0);
	assert(write_dr(pid, 1, (long)sigh) == 0);

	dr7 = 0;
	dr7 |= encode_dr7(0, 1, DR_RW_EXECUTE, DR_LEN_1);
	dr7 |= encode_dr7(1, 1, DR_RW_EXECUTE, DR_LEN_1);

	assert(write_dr(pid, 7, dr7) == 0);

	assert(ptrace(PTRACE_CONT, pid, 0,0) == 0);
	assert(pid == waitpid(-1, &stat, 0));
	assert(WSTOPSIG(stat) == SIGTRAP);
	assert(get_ip(pid) == func);

	kill(pid, SIGINT);
	assert(ptrace(PTRACE_CONT, pid, 0,0) == 0);
	assert(pid == waitpid(-1, &stat, 0));
	assert(WSTOPSIG(stat) == SIGINT);

	assert(ptrace(PTRACE_CONT, pid, 0,SIGINT) == 0);
	assert(pid == waitpid(-1, &stat, 0));
	assert(WSTOPSIG(stat) == SIGTRAP);
	assert(get_ip(pid) == sigh);

	assert(ptrace(PTRACE_CONT, pid, 0,0) == 0);
	assert(pid == waitpid(-1, &stat, 0));
	assert(stat == 0x1300);

	return 0;
}


  reply	other threads:[~2013-04-17 20:06 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-10 18:41 [PATCHv2 0/6] perf, signal x86: Fix breakpoint events overflow handling Jiri Olsa
2013-03-10 18:41 ` [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call Jiri Olsa
2013-04-16  1:05   ` Frederic Weisbecker
2013-04-16 14:42     ` Oleg Nesterov
2013-04-17 20:02       ` Oleg Nesterov [this message]
2013-04-24 15:39         ` Frederic Weisbecker
2013-04-24 15:24   ` Frederic Weisbecker
2013-03-10 18:41 ` [PATCH 2/6] signal x86: Clear RF EFLAGS bit for signal handler Jiri Olsa
2013-03-10 18:41 ` [PATCH 3/6] signal x86: Merge EFLAGS bit clearing into single statement Jiri Olsa
2013-03-10 18:41 ` [PATCH 4/6] perf: Fix hw breakpoints overflow period sampling Jiri Olsa
2013-04-04 16:10   ` Peter Zijlstra
2013-03-10 18:41 ` [PATCH 5/6] perf tests: Test breakpoint overflow signal handler Jiri Olsa
2013-03-21 11:38   ` [tip:perf/core] " tip-bot for Jiri Olsa
2013-03-10 18:41 ` [PATCH 6/6] perf tests: Test breakpoint overflow signal handler counts Jiri Olsa
2013-03-21 11:39   ` [tip:perf/core] " tip-bot for Jiri Olsa
2013-03-24 15:15 ` [PATCHv2 0/6] perf, signal x86: Fix breakpoint events overflow handling Jiri Olsa

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=20130417200206.GA18253@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=andi@firstfloor.org \
    --cc=cjashfor@linux.vnet.ibm.com \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=tglx@linutronix.de \
    --cc=vincent.weaver@maine.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.