2013-03-10 18:41:47

by Jiri Olsa

Subject: [PATCHv2 0/6] perf, signal x86: Fix breakpoint events overflow handling

hi,
sendign v2 for initial RFC patchset:
https://lkml.org/lkml/2013/3/1/324

v2 changes:
- added patch to merge EFLAGS bit clearing into single statement

Also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/linux.git
perf/RF5

thanks,
jirka

Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: Stephane Eranian <[email protected]>
---
Jiri Olsa (6):
signal x86: Propage RF EFLAGS bit throught the signal restore call
signal x86: Clear RF EFLAGS bit for signal handler
signal x86: Merge EFLAGS bit clearing into single statement
perf: Fix hw breakpoints overflow period sampling
perf tests: Test breakpoint overflow signal handler
perf tests: Test breakpoint overflow signal handler counts

arch/x86/ia32/ia32_signal.c | 2 -
arch/x86/include/asm/sighandling.h | 4 +-
arch/x86/kernel/signal.c | 16 +++-----
include/linux/perf_event.h | 2 +
kernel/events/core.c | 2 +-
kernel/events/hw_breakpoint.c | 5 +++
tools/perf/Makefile | 2 +
tools/perf/tests/bp_signal.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/tests/bp_signal_overflow.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/tests/builtin-test.c | 8 ++++
tools/perf/tests/tests.h | 2 +
11 files changed, 341 insertions(+), 15 deletions(-)
create mode 100644 tools/perf/tests/bp_signal.c
create mode 100644 tools/perf/tests/bp_signal_overflow.c

2013-03-10 18:41:49

by Jiri Olsa

Subject: [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call

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.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: Stephane Eranian <[email protected]>
---
arch/x86/ia32/ia32_signal.c | 2 --
arch/x86/include/asm/sighandling.h | 4 ++--
arch/x86/kernel/signal.c | 6 ------
3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index cf1a471..bccfca6 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -34,8 +34,6 @@
#include <asm/sys_ia32.h>
#include <asm/smap.h>

-#define FIX_EFLAGS __FIX_EFLAGS
-
int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
{
int err = 0;
diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
index beff97f..7a95816 100644
--- a/arch/x86/include/asm/sighandling.h
+++ b/arch/x86/include/asm/sighandling.h
@@ -7,10 +7,10 @@

#include <asm/processor-flags.h>

-#define __FIX_EFLAGS (X86_EFLAGS_AC | X86_EFLAGS_OF | \
+#define FIX_EFLAGS (X86_EFLAGS_AC | X86_EFLAGS_OF | \
X86_EFLAGS_DF | X86_EFLAGS_TF | X86_EFLAGS_SF | \
X86_EFLAGS_ZF | X86_EFLAGS_AF | X86_EFLAGS_PF | \
- X86_EFLAGS_CF)
+ X86_EFLAGS_CF | X86_EFLAGS_RF)

void signal_fault(struct pt_regs *regs, void __user *frame, char *where);

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 6956299..9df4c0b 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -43,12 +43,6 @@

#include <asm/sigframe.h>

-#ifdef CONFIG_X86_32
-# define FIX_EFLAGS (__FIX_EFLAGS | X86_EFLAGS_RF)
-#else
-# define FIX_EFLAGS __FIX_EFLAGS
-#endif
-
#define COPY(x) do { \
get_user_ex(regs->x, &sc->x); \
} while (0)
--
1.7.11.7

2013-03-10 18:42:00

by Jiri Olsa

Subject: [PATCH 6/6] perf tests: Test breakpoint overflow signal handler counts

Adding automated test to check the exact number of
breakpoint event overflows and counts.

This test was originally done by Vince Weaver for
perf_event_tests.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: Stephane Eranian <[email protected]>
---
tools/perf/Makefile | 1 +
tools/perf/tests/bp_signal_overflow.c | 126 ++++++++++++++++++++++++++++++++++
tools/perf/tests/builtin-test.c | 4 ++
tools/perf/tests/tests.h | 1 +
4 files changed, 132 insertions(+)
create mode 100644 tools/perf/tests/bp_signal_overflow.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index bea66fe..34a63ce 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -498,6 +498,7 @@ LIB_OBJS += $(OUTPUT)tests/pmu.o
LIB_OBJS += $(OUTPUT)tests/hists_link.o
LIB_OBJS += $(OUTPUT)tests/python-use.o
LIB_OBJS += $(OUTPUT)tests/bp_signal.o
+LIB_OBJS += $(OUTPUT)tests/bp_signal_overflow.o

BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/bp_signal_overflow.c b/tools/perf/tests/bp_signal_overflow.c
new file mode 100644
index 0000000..556d2ce
--- /dev/null
+++ b/tools/perf/tests/bp_signal_overflow.c
@@ -0,0 +1,126 @@
+/*
+ * Originally done by Vince Weaver <[email protected]> for
+ * perf_event_tests (git://github.com/deater/perf_event_tests)
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <time.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/compiler.h>
+#include <linux/hw_breakpoint.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+
+static int overflows;
+
+__attribute__ ((noinline))
+static int test_function(void)
+{
+ return time(NULL);
+}
+
+static void sig_handler(int signum __maybe_unused,
+ siginfo_t *oh __maybe_unused,
+ void *uc __maybe_unused)
+{
+ overflows++;
+}
+
+static long long bp_count(int fd)
+{
+ long long count;
+ int ret;
+
+ ret = read(fd, &count, sizeof(long long));
+ if (ret != sizeof(long long)) {
+ pr_err("failed to read: %d\n", ret);
+ return TEST_FAIL;
+ }
+
+ return count;
+}
+
+#define EXECUTIONS 10000
+#define THRESHOLD 100
+
+int test__bp_signal_overflow(void)
+{
+ struct perf_event_attr pe;
+ struct sigaction sa;
+ long long count;
+ int fd, i, fails = 0;
+
+ /* setup SIGIO signal handler */
+ memset(&sa, 0, sizeof(struct sigaction));
+ sa.sa_sigaction = (void *) sig_handler;
+ sa.sa_flags = SA_SIGINFO;
+
+ if (sigaction(SIGIO, &sa, NULL) < 0) {
+ pr_err("failed setting up signal handler\n");
+ return TEST_FAIL;
+ }
+
+ memset(&pe, 0, sizeof(struct perf_event_attr));
+ pe.type = PERF_TYPE_BREAKPOINT;
+ pe.size = sizeof(struct perf_event_attr);
+
+ pe.config = 0;
+ pe.bp_type = HW_BREAKPOINT_X;
+ pe.bp_addr = (unsigned long) test_function;
+ pe.bp_len = sizeof(long);
+
+ pe.sample_period = THRESHOLD;
+ pe.sample_type = PERF_SAMPLE_IP;
+ pe.wakeup_events = 1;
+
+ pe.disabled = 1;
+ pe.exclude_kernel = 1;
+ pe.exclude_hv = 1;
+
+ fd = sys_perf_event_open(&pe, 0, -1, -1, 0);
+ if (fd < 0) {
+ pr_err("failed opening event %llx\n", pe.config);
+ return TEST_FAIL;
+ }
+
+ fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC);
+ fcntl(fd, F_SETSIG, SIGIO);
+ fcntl(fd, F_SETOWN, getpid());
+
+ ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+ ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
+
+ for (i = 0; i < EXECUTIONS; i++)
+ test_function();
+
+ ioctl(fd, PERF_EVENT_IOC_DISABLE, 0);
+
+ count = bp_count(fd);
+
+ close(fd);
+
+ pr_debug("count %lld, overflow %d\n",
+ count, overflows);
+
+ if (count != EXECUTIONS) {
+ pr_err("\tWrong number of executions %lld != %d\n",
+ count, EXECUTIONS);
+ fails++;
+ }
+
+ if (overflows != EXECUTIONS / THRESHOLD) {
+ pr_err("\tWrong number of overflows %d != %d\n",
+ overflows, EXECUTIONS / THRESHOLD);
+ fails++;
+ }
+
+ return fails ? TEST_FAIL : TEST_OK;
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 37b108b..45d9ad4 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -82,6 +82,10 @@ static struct test {
.func = test__bp_signal,
},
{
+ .desc = "Test breakpoint overflow sampling",
+ .func = test__bp_signal_overflow,
+ },
+ {
.func = NULL,
},
};
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 05d0e58..6cf1ec4 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -24,5 +24,6 @@ int test__parse_events(void);
int test__hists_link(void);
int test__python_use(void);
int test__bp_signal(void);
+int test__bp_signal_overflow(void);

#endif /* TESTS_H */
--
1.7.11.7

2013-03-10 18:42:11

by Jiri Olsa

Subject: [PATCH 3/6] signal x86: Merge EFLAGS bit clearing into single statement

Merging EFLAGS bit clearing into a single statement, to
ensure EFLAGS bits being cleared in single instruction.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: Stephane Eranian <[email protected]>
---
arch/x86/kernel/signal.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index cb12fc9..cf91358 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -662,21 +662,17 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
if (!failed) {
/*
* Clear the direction flag as per the ABI for function entry.
- */
- regs->flags &= ~X86_EFLAGS_DF;
- /*
+ *
* Clear RF when entering the signal handler, because
* it might disable possible debug exception from the
* signal handler.
- */
- regs->flags &= ~X86_EFLAGS_RF;
- /*
+ *
* Clear TF when entering the signal handler, but
* notify any tracer that was single-stepping it.
* The tracer may want to single-step inside the
* handler too.
*/
- regs->flags &= ~X86_EFLAGS_TF;
+ regs->flags &= ~(X86_EFLAGS_DF|X86_EFLAGS_RF|X86_EFLAGS_TF);
}
signal_setup_done(failed, ksig, test_thread_flag(TIF_SINGLESTEP));
}
--
1.7.11.7

2013-03-10 18:41:57

by Jiri Olsa

Subject: [PATCH 4/6] perf: Fix hw breakpoints overflow period sampling

The hw breakpoint pmu 'add' function is missing the
period_left update needed for SW events.

The perf HW breakpoint events use SW events framework
for to process the overflow, so it needs to be properly
initialized during PMU 'add' method.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: Stephane Eranian <[email protected]>
---
include/linux/perf_event.h | 2 ++
kernel/events/core.c | 2 +-
kernel/events/hw_breakpoint.c | 5 +++++
3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 8737e1c..24befdf 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -738,6 +738,7 @@ extern unsigned int perf_output_skip(struct perf_output_handle *handle,
unsigned int len);
extern int perf_swevent_get_recursion_context(void);
extern void perf_swevent_put_recursion_context(int rctx);
+extern u64 perf_swevent_set_period(struct perf_event *event);
extern void perf_event_enable(struct perf_event *event);
extern void perf_event_disable(struct perf_event *event);
extern int __perf_event_disable(void *info);
@@ -777,6 +778,7 @@ static inline void perf_event_fork(struct task_struct *tsk) { }
static inline void perf_event_init(void) { }
static inline int perf_swevent_get_recursion_context(void) { return -1; }
static inline void perf_swevent_put_recursion_context(int rctx) { }
+static inline u64 perf_swevent_set_period(struct perf_event *event) { return 0; }
static inline void perf_event_enable(struct perf_event *event) { }
static inline void perf_event_disable(struct perf_event *event) { }
static inline int __perf_event_disable(void *info) { return -1; }
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5976a2a..bd076b8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4972,7 +4972,7 @@ static DEFINE_PER_CPU(struct swevent_htable, swevent_htable);
* sign as trigger.
*/

-static u64 perf_swevent_set_period(struct perf_event *event)
+u64 perf_swevent_set_period(struct perf_event *event)
{
struct hw_perf_event *hwc = &event->hw;
u64 period = hwc->last_period;
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index a64f8ae..966a241 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -612,6 +612,11 @@ static int hw_breakpoint_add(struct perf_event *bp, int flags)
if (!(flags & PERF_EF_START))
bp->hw.state = PERF_HES_STOPPED;

+ if (is_sampling_event(bp)) {
+ bp->hw.last_period = bp->hw.sample_period;
+ perf_swevent_set_period(bp);
+ }
+
return arch_install_hw_breakpoint(bp);
}

--
1.7.11.7

2013-03-10 18:41:55

by Jiri Olsa

Subject: [PATCH 5/6] perf tests: Test breakpoint overflow signal handler

Adding automated test for breakpoint event signal
handler checking if it's executed properly.

The test is related to the proper handling of the RF
EFLAGS bit on x86_64, but it's generic for all archs.

First we check the signal handler is properly called
and that the following debug exception return to user
space wouldn't trigger recursive breakpoint.

This is related to x86_64 RF EFLAGS bit being managed
in a wrong way.

Second we check that we can set breakpoint in signal
handler, which is not possible on x86_64 if the signal
handler is executed with RF EFLAG set.

This test is inpired by overflow tests done by Vince
Weaver.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: Stephane Eranian <[email protected]>
---
tools/perf/Makefile | 1 +
tools/perf/tests/bp_signal.c | 187 ++++++++++++++++++++++++++++++++++++++++
tools/perf/tests/builtin-test.c | 4 +
tools/perf/tests/tests.h | 1 +
4 files changed, 193 insertions(+)
create mode 100644 tools/perf/tests/bp_signal.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index a2108ca..bea66fe 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -497,6 +497,7 @@ LIB_OBJS += $(OUTPUT)tests/evsel-tp-sched.o
LIB_OBJS += $(OUTPUT)tests/pmu.o
LIB_OBJS += $(OUTPUT)tests/hists_link.o
LIB_OBJS += $(OUTPUT)tests/python-use.o
+LIB_OBJS += $(OUTPUT)tests/bp_signal.o

BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
new file mode 100644
index 0000000..a40b019
--- /dev/null
+++ b/tools/perf/tests/bp_signal.c
@@ -0,0 +1,187 @@
+/*
+ * Inspired by breakpoint overflow test done by
+ * Vince Weaver <[email protected]> for perf_event_tests
+ * (git://github.com/deater/perf_event_tests)
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <time.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/compiler.h>
+#include <linux/hw_breakpoint.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+
+static int fd1;
+static int fd2;
+static int overflows;
+
+__attribute__ ((noinline))
+static int test_function(void)
+{
+ return time(NULL);
+}
+
+static void sig_handler(int signum __maybe_unused,
+ siginfo_t *oh __maybe_unused,
+ void *uc __maybe_unused)
+{
+ overflows++;
+
+ if (overflows > 10) {
+ /*
+ * This should be executed only once during
+ * this test, if we are here for the 10th
+ * time, consider this the recursive issue.
+ *
+ * We can get out of here by disable events,
+ * so no new SIGIO is delivered.
+ */
+ ioctl(fd1, PERF_EVENT_IOC_DISABLE, 0);
+ ioctl(fd2, PERF_EVENT_IOC_DISABLE, 0);
+ }
+}
+
+static int bp_event(void *fn, int setup_signal)
+{
+ struct perf_event_attr pe;
+ int fd;
+
+ memset(&pe, 0, sizeof(struct perf_event_attr));
+ pe.type = PERF_TYPE_BREAKPOINT;
+ pe.size = sizeof(struct perf_event_attr);
+
+ pe.config = 0;
+ pe.bp_type = HW_BREAKPOINT_X;
+ pe.bp_addr = (unsigned long) fn;
+ pe.bp_len = sizeof(long);
+
+ pe.sample_period = 1;
+ pe.sample_type = PERF_SAMPLE_IP;
+ pe.wakeup_events = 1;
+
+ pe.disabled = 1;
+ pe.exclude_kernel = 1;
+ pe.exclude_hv = 1;
+
+ fd = sys_perf_event_open(&pe, 0, -1, -1, 0);
+ if (fd < 0) {
+ pr_err("failed opening event %llx\n", pe.config);
+ return TEST_FAIL;
+ }
+
+ if (setup_signal) {
+ fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC);
+ fcntl(fd, F_SETSIG, SIGIO);
+ fcntl(fd, F_SETOWN, getpid());
+ }
+
+ ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+
+ return fd;
+}
+
+static long long bp_count(int fd)
+{
+ long long count;
+ int ret;
+
+ ret = read(fd, &count, sizeof(long long));
+ if (ret != sizeof(long long)) {
+ pr_err("failed to read: %d\n", ret);
+ return TEST_FAIL;
+ }
+
+ return count;
+}
+
+int test__bp_signal(void)
+{
+ struct sigaction sa;
+ long long count1, count2;
+
+ /* setup SIGIO signal handler */
+ memset(&sa, 0, sizeof(struct sigaction));
+ sa.sa_sigaction = (void *) sig_handler;
+ sa.sa_flags = SA_SIGINFO;
+
+ if (sigaction(SIGIO, &sa, NULL) < 0) {
+ pr_err("failed setting up signal handler\n");
+ return TEST_FAIL;
+ }
+
+ /*
+ * We create following events:
+ *
+ * fd1 - breakpoint event on test_function with SIGIO
+ * signal configured. We should get signal
+ * notification each time the breakpoint is hit
+ *
+ * fd2 - breakpoint event on sig_handler without SIGIO
+ * configured.
+ *
+ * Following processing should happen:
+ * - execute test_function
+ * - fd1 event breakpoint hit -> count1 == 1
+ * - SIGIO is delivered -> overflows == 1
+ * - fd2 event breakpoint hit -> count2 == 1
+ *
+ * The test case check following error conditions:
+ * - we get stuck in signal handler because of debug
+ * exception being triggered receursively due to
+ * the wrong RF EFLAG management
+ *
+ * - we never trigger the sig_handler breakpoint due
+ * to the rong RF EFLAG management
+ *
+ */
+
+ fd1 = bp_event(test_function, 1);
+ fd2 = bp_event(sig_handler, 0);
+
+ ioctl(fd1, PERF_EVENT_IOC_ENABLE, 0);
+ ioctl(fd2, PERF_EVENT_IOC_ENABLE, 0);
+
+ /*
+ * Kick off the test by trigering 'fd1'
+ * breakpoint.
+ */
+ test_function();
+
+ ioctl(fd1, PERF_EVENT_IOC_DISABLE, 0);
+ ioctl(fd2, PERF_EVENT_IOC_DISABLE, 0);
+
+ count1 = bp_count(fd1);
+ count2 = bp_count(fd2);
+
+ close(fd1);
+ close(fd2);
+
+ pr_debug("count1 %lld, count2 %lld, overflow %d\n",
+ count1, count2, overflows);
+
+ if (count1 != 1) {
+ if (count1 == 11)
+ pr_err("failed: RF EFLAG recursion issue detected\n");
+ else
+ pr_err("failed: wrong count for bp1%lld\n",
+ count1);
+ }
+
+ if (overflows != 1)
+ pr_err("failed: wrong overflow hit\n");
+
+ if (count2 != 1)
+ pr_err("failed: wrong count for bp2\n");
+
+ return count1 == 1 && overflows == 1 && count2 == 1 ?
+ TEST_OK : TEST_FAIL;
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index acb98e0..37b108b 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -78,6 +78,10 @@ static struct test {
.func = test__python_use,
},
{
+ .desc = "Test breakpoint overflow signal handler",
+ .func = test__bp_signal,
+ },
+ {
.func = NULL,
},
};
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 5de0be1..05d0e58 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -23,5 +23,6 @@ int test__dso_data(void);
int test__parse_events(void);
int test__hists_link(void);
int test__python_use(void);
+int test__bp_signal(void);

#endif /* TESTS_H */
--
1.7.11.7

2013-03-10 18:41:48

by Jiri Olsa

Subject: [PATCH 2/6] signal x86: Clear RF EFLAGS bit for signal handler

Clearing RF EFLAGS bit for signal handler. The reason is,
that this flag is set by debug exception code to prevent
the recursive exception entry.

Leaving it set for signal handler might prevent debug
exception of the signal handler itself.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Vince Weaver <[email protected]>
Cc: Stephane Eranian <[email protected]>
---
arch/x86/kernel/signal.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 9df4c0b..cb12fc9 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -665,6 +665,12 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
*/
regs->flags &= ~X86_EFLAGS_DF;
/*
+ * Clear RF when entering the signal handler, because
+ * it might disable possible debug exception from the
+ * signal handler.
+ */
+ regs->flags &= ~X86_EFLAGS_RF;
+ /*
* Clear TF when entering the signal handler, but
* notify any tracer that was single-stepping it.
* The tracer may want to single-step inside the
--
1.7.11.7

2013-03-21 11:41:26

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/core] perf tests: Test breakpoint overflow signal handler counts

Commit-ID: 06933e3a732bb305b0721f1051a45264588e0519
Gitweb: http://git.kernel.org/tip/06933e3a732bb305b0721f1051a45264588e0519
Author: Jiri Olsa <[email protected]>
AuthorDate: Sun, 10 Mar 2013 19:41:11 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 15 Mar 2013 13:06:10 -0300

perf tests: Test breakpoint overflow signal handler counts

Adding automated test to check the exact number of breakpoint event
overflows and counts.

This test was originally done by Vince Weaver for perf_event_tests.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vince Weaver <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ committer note: s/pr_err/pr_debug/g i.e. print just OK or FAILED in non verbose mode ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 1 +
tools/perf/tests/bp_signal_overflow.c | 126 ++++++++++++++++++++++++++++++++++
tools/perf/tests/builtin-test.c | 4 ++
tools/perf/tests/tests.h | 1 +
4 files changed, 132 insertions(+)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 21e0b4b..990e9a1 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -512,6 +512,7 @@ LIB_OBJS += $(OUTPUT)tests/pmu.o
LIB_OBJS += $(OUTPUT)tests/hists_link.o
LIB_OBJS += $(OUTPUT)tests/python-use.o
LIB_OBJS += $(OUTPUT)tests/bp_signal.o
+LIB_OBJS += $(OUTPUT)tests/bp_signal_overflow.o

BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/bp_signal_overflow.c b/tools/perf/tests/bp_signal_overflow.c
new file mode 100644
index 0000000..fe7ed28
--- /dev/null
+++ b/tools/perf/tests/bp_signal_overflow.c
@@ -0,0 +1,126 @@
+/*
+ * Originally done by Vince Weaver <[email protected]> for
+ * perf_event_tests (git://github.com/deater/perf_event_tests)
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <time.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/compiler.h>
+#include <linux/hw_breakpoint.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+
+static int overflows;
+
+__attribute__ ((noinline))
+static int test_function(void)
+{
+ return time(NULL);
+}
+
+static void sig_handler(int signum __maybe_unused,
+ siginfo_t *oh __maybe_unused,
+ void *uc __maybe_unused)
+{
+ overflows++;
+}
+
+static long long bp_count(int fd)
+{
+ long long count;
+ int ret;
+
+ ret = read(fd, &count, sizeof(long long));
+ if (ret != sizeof(long long)) {
+ pr_debug("failed to read: %d\n", ret);
+ return TEST_FAIL;
+ }
+
+ return count;
+}
+
+#define EXECUTIONS 10000
+#define THRESHOLD 100
+
+int test__bp_signal_overflow(void)
+{
+ struct perf_event_attr pe;
+ struct sigaction sa;
+ long long count;
+ int fd, i, fails = 0;
+
+ /* setup SIGIO signal handler */
+ memset(&sa, 0, sizeof(struct sigaction));
+ sa.sa_sigaction = (void *) sig_handler;
+ sa.sa_flags = SA_SIGINFO;
+
+ if (sigaction(SIGIO, &sa, NULL) < 0) {
+ pr_debug("failed setting up signal handler\n");
+ return TEST_FAIL;
+ }
+
+ memset(&pe, 0, sizeof(struct perf_event_attr));
+ pe.type = PERF_TYPE_BREAKPOINT;
+ pe.size = sizeof(struct perf_event_attr);
+
+ pe.config = 0;
+ pe.bp_type = HW_BREAKPOINT_X;
+ pe.bp_addr = (unsigned long) test_function;
+ pe.bp_len = sizeof(long);
+
+ pe.sample_period = THRESHOLD;
+ pe.sample_type = PERF_SAMPLE_IP;
+ pe.wakeup_events = 1;
+
+ pe.disabled = 1;
+ pe.exclude_kernel = 1;
+ pe.exclude_hv = 1;
+
+ fd = sys_perf_event_open(&pe, 0, -1, -1, 0);
+ if (fd < 0) {
+ pr_debug("failed opening event %llx\n", pe.config);
+ return TEST_FAIL;
+ }
+
+ fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC);
+ fcntl(fd, F_SETSIG, SIGIO);
+ fcntl(fd, F_SETOWN, getpid());
+
+ ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+ ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
+
+ for (i = 0; i < EXECUTIONS; i++)
+ test_function();
+
+ ioctl(fd, PERF_EVENT_IOC_DISABLE, 0);
+
+ count = bp_count(fd);
+
+ close(fd);
+
+ pr_debug("count %lld, overflow %d\n",
+ count, overflows);
+
+ if (count != EXECUTIONS) {
+ pr_debug("\tWrong number of executions %lld != %d\n",
+ count, EXECUTIONS);
+ fails++;
+ }
+
+ if (overflows != EXECUTIONS / THRESHOLD) {
+ pr_debug("\tWrong number of overflows %d != %d\n",
+ overflows, EXECUTIONS / THRESHOLD);
+ fails++;
+ }
+
+ return fails ? TEST_FAIL : TEST_OK;
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 37b108b..45d9ad4 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -82,6 +82,10 @@ static struct test {
.func = test__bp_signal,
},
{
+ .desc = "Test breakpoint overflow sampling",
+ .func = test__bp_signal_overflow,
+ },
+ {
.func = NULL,
},
};
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 05d0e58..6cf1ec4 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -24,5 +24,6 @@ int test__parse_events(void);
int test__hists_link(void);
int test__python_use(void);
int test__bp_signal(void);
+int test__bp_signal_overflow(void);

#endif /* TESTS_H */

2013-03-21 11:44:22

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/core] perf tests: Test breakpoint overflow signal handler

Commit-ID: 5a6bef47b418676546ab86d25631c3cfb9ffaf2a
Gitweb: http://git.kernel.org/tip/5a6bef47b418676546ab86d25631c3cfb9ffaf2a
Author: Jiri Olsa <[email protected]>
AuthorDate: Sun, 10 Mar 2013 19:41:10 +0100
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 15 Mar 2013 13:06:10 -0300

perf tests: Test breakpoint overflow signal handler

Adding automated test for breakpoint event signal handler checking if
it's executed properly.

The test is related to the proper handling of the RF EFLAGS bit on
x86_64, but it's generic for all archs.

First we check the signal handler is properly called and that the
following debug exception return to user space wouldn't trigger
recursive breakpoint.

This is related to x86_64 RF EFLAGS bit being managed in a wrong way.

Second we check that we can set breakpoint in signal handler, which is
not possible on x86_64 if the signal handler is executed with RF EFLAG
set.

This test is inpired by overflow tests done by Vince Weaver.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Vince Weaver <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ committer note: s/pr_err/pr_debug/g i.e. print just OK or FAILED in non verbose mode ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 1 +
tools/perf/tests/bp_signal.c | 186 ++++++++++++++++++++++++++++++++++++++++
tools/perf/tests/builtin-test.c | 4 +
tools/perf/tests/tests.h | 1 +
4 files changed, 192 insertions(+)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 3dcd627..21e0b4b 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -511,6 +511,7 @@ LIB_OBJS += $(OUTPUT)tests/evsel-tp-sched.o
LIB_OBJS += $(OUTPUT)tests/pmu.o
LIB_OBJS += $(OUTPUT)tests/hists_link.o
LIB_OBJS += $(OUTPUT)tests/python-use.o
+LIB_OBJS += $(OUTPUT)tests/bp_signal.o

BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
new file mode 100644
index 0000000..68daa28
--- /dev/null
+++ b/tools/perf/tests/bp_signal.c
@@ -0,0 +1,186 @@
+/*
+ * Inspired by breakpoint overflow test done by
+ * Vince Weaver <[email protected]> for perf_event_tests
+ * (git://github.com/deater/perf_event_tests)
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <time.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/compiler.h>
+#include <linux/hw_breakpoint.h>
+
+#include "tests.h"
+#include "debug.h"
+#include "perf.h"
+
+static int fd1;
+static int fd2;
+static int overflows;
+
+__attribute__ ((noinline))
+static int test_function(void)
+{
+ return time(NULL);
+}
+
+static void sig_handler(int signum __maybe_unused,
+ siginfo_t *oh __maybe_unused,
+ void *uc __maybe_unused)
+{
+ overflows++;
+
+ if (overflows > 10) {
+ /*
+ * This should be executed only once during
+ * this test, if we are here for the 10th
+ * time, consider this the recursive issue.
+ *
+ * We can get out of here by disable events,
+ * so no new SIGIO is delivered.
+ */
+ ioctl(fd1, PERF_EVENT_IOC_DISABLE, 0);
+ ioctl(fd2, PERF_EVENT_IOC_DISABLE, 0);
+ }
+}
+
+static int bp_event(void *fn, int setup_signal)
+{
+ struct perf_event_attr pe;
+ int fd;
+
+ memset(&pe, 0, sizeof(struct perf_event_attr));
+ pe.type = PERF_TYPE_BREAKPOINT;
+ pe.size = sizeof(struct perf_event_attr);
+
+ pe.config = 0;
+ pe.bp_type = HW_BREAKPOINT_X;
+ pe.bp_addr = (unsigned long) fn;
+ pe.bp_len = sizeof(long);
+
+ pe.sample_period = 1;
+ pe.sample_type = PERF_SAMPLE_IP;
+ pe.wakeup_events = 1;
+
+ pe.disabled = 1;
+ pe.exclude_kernel = 1;
+ pe.exclude_hv = 1;
+
+ fd = sys_perf_event_open(&pe, 0, -1, -1, 0);
+ if (fd < 0) {
+ pr_debug("failed opening event %llx\n", pe.config);
+ return TEST_FAIL;
+ }
+
+ if (setup_signal) {
+ fcntl(fd, F_SETFL, O_RDWR|O_NONBLOCK|O_ASYNC);
+ fcntl(fd, F_SETSIG, SIGIO);
+ fcntl(fd, F_SETOWN, getpid());
+ }
+
+ ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+
+ return fd;
+}
+
+static long long bp_count(int fd)
+{
+ long long count;
+ int ret;
+
+ ret = read(fd, &count, sizeof(long long));
+ if (ret != sizeof(long long)) {
+ pr_debug("failed to read: %d\n", ret);
+ return TEST_FAIL;
+ }
+
+ return count;
+}
+
+int test__bp_signal(void)
+{
+ struct sigaction sa;
+ long long count1, count2;
+
+ /* setup SIGIO signal handler */
+ memset(&sa, 0, sizeof(struct sigaction));
+ sa.sa_sigaction = (void *) sig_handler;
+ sa.sa_flags = SA_SIGINFO;
+
+ if (sigaction(SIGIO, &sa, NULL) < 0) {
+ pr_debug("failed setting up signal handler\n");
+ return TEST_FAIL;
+ }
+
+ /*
+ * We create following events:
+ *
+ * fd1 - breakpoint event on test_function with SIGIO
+ * signal configured. We should get signal
+ * notification each time the breakpoint is hit
+ *
+ * fd2 - breakpoint event on sig_handler without SIGIO
+ * configured.
+ *
+ * Following processing should happen:
+ * - execute test_function
+ * - fd1 event breakpoint hit -> count1 == 1
+ * - SIGIO is delivered -> overflows == 1
+ * - fd2 event breakpoint hit -> count2 == 1
+ *
+ * The test case check following error conditions:
+ * - we get stuck in signal handler because of debug
+ * exception being triggered receursively due to
+ * the wrong RF EFLAG management
+ *
+ * - we never trigger the sig_handler breakpoint due
+ * to the rong RF EFLAG management
+ *
+ */
+
+ fd1 = bp_event(test_function, 1);
+ fd2 = bp_event(sig_handler, 0);
+
+ ioctl(fd1, PERF_EVENT_IOC_ENABLE, 0);
+ ioctl(fd2, PERF_EVENT_IOC_ENABLE, 0);
+
+ /*
+ * Kick off the test by trigering 'fd1'
+ * breakpoint.
+ */
+ test_function();
+
+ ioctl(fd1, PERF_EVENT_IOC_DISABLE, 0);
+ ioctl(fd2, PERF_EVENT_IOC_DISABLE, 0);
+
+ count1 = bp_count(fd1);
+ count2 = bp_count(fd2);
+
+ close(fd1);
+ close(fd2);
+
+ pr_debug("count1 %lld, count2 %lld, overflow %d\n",
+ count1, count2, overflows);
+
+ if (count1 != 1) {
+ if (count1 == 11)
+ pr_debug("failed: RF EFLAG recursion issue detected\n");
+ else
+ pr_debug("failed: wrong count for bp1%lld\n", count1);
+ }
+
+ if (overflows != 1)
+ pr_debug("failed: wrong overflow hit\n");
+
+ if (count2 != 1)
+ pr_debug("failed: wrong count for bp2\n");
+
+ return count1 == 1 && overflows == 1 && count2 == 1 ?
+ TEST_OK : TEST_FAIL;
+}
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index acb98e0..37b108b 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -78,6 +78,10 @@ static struct test {
.func = test__python_use,
},
{
+ .desc = "Test breakpoint overflow signal handler",
+ .func = test__bp_signal,
+ },
+ {
.func = NULL,
},
};
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 5de0be1..05d0e58 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -23,5 +23,6 @@ int test__dso_data(void);
int test__parse_events(void);
int test__hists_link(void);
int test__python_use(void);
+int test__bp_signal(void);

#endif /* TESTS_H */

2013-03-24 15:16:37

by Jiri Olsa

Subject: Re: [PATCHv2 0/6] perf, signal x86: Fix breakpoint events overflow handling

On Sun, Mar 10, 2013 at 07:41:05PM +0100, Jiri Olsa wrote:
> hi,
> sendign v2 for initial RFC patchset:
> https://lkml.org/lkml/2013/3/1/324
>
> v2 changes:
> - added patch to merge EFLAGS bit clearing into single statement

hi,
any feedback on this?

thanks,
jirka

2013-04-04 16:10:54

by Peter Zijlstra

Subject: Re: [PATCH 4/6] perf: Fix hw breakpoints overflow period sampling

On Sun, 2013-03-10 at 19:41 +0100, Jiri Olsa wrote:
> The hw breakpoint pmu 'add' function is missing the
> period_left update needed for SW events.
>
> The perf HW breakpoint events use SW events framework
> for to process the overflow, so it needs to be properly
> initialized during PMU 'add' method.

Yeah looks about right..

2013-04-16 01:05:31

by Frederic Weisbecker

Subject: Re: [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call

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?

[...]
> --- a/arch/x86/include/asm/sighandling.h
> +++ b/arch/x86/include/asm/sighandling.h
> @@ -7,10 +7,10 @@
>
> #include <asm/processor-flags.h>
>
> -#define __FIX_EFLAGS (X86_EFLAGS_AC | X86_EFLAGS_OF | \
> +#define FIX_EFLAGS (X86_EFLAGS_AC | X86_EFLAGS_OF | \
> X86_EFLAGS_DF | X86_EFLAGS_TF | X86_EFLAGS_SF | \
> X86_EFLAGS_ZF | X86_EFLAGS_AF | X86_EFLAGS_PF | \
> - X86_EFLAGS_CF)
> + X86_EFLAGS_CF | X86_EFLAGS_RF)
>
> void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
>
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index 6956299..9df4c0b 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -43,12 +43,6 @@
>
> #include <asm/sigframe.h>
>
> -#ifdef CONFIG_X86_32
> -# define FIX_EFLAGS (__FIX_EFLAGS | X86_EFLAGS_RF)
> -#else
> -# define FIX_EFLAGS __FIX_EFLAGS
> -#endif

So, I'm a bit confused here. Why was that only hapenning in X86_64?

> -
> #define COPY(x) do { \
> get_user_ex(regs->x, &sc->x); \
> } while (0)
> --
> 1.7.11.7
>

2013-04-16 14:46:41

by Oleg Nesterov

Subject: Re: [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call

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.

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?

Oleg.

2013-04-17 20:06:14

by Oleg Nesterov

Subject: Re: [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call

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;
}

2013-04-24 15:24:46

by Frederic Weisbecker

Subject: Re: [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call

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.
>
> Signed-off-by: Jiri Olsa <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Andi Kleen <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Corey Ashford <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Vince Weaver <[email protected]>
> Cc: Stephane Eranian <[email protected]>
> ---
> arch/x86/ia32/ia32_signal.c | 2 --
> arch/x86/include/asm/sighandling.h | 4 ++--
> arch/x86/kernel/signal.c | 6 ------
> 3 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
> index cf1a471..bccfca6 100644
> --- a/arch/x86/ia32/ia32_signal.c
> +++ b/arch/x86/ia32/ia32_signal.c
> @@ -34,8 +34,6 @@
> #include <asm/sys_ia32.h>
> #include <asm/smap.h>
>
> -#define FIX_EFLAGS __FIX_EFLAGS
> -
> int copy_siginfo_to_user32(compat_siginfo_t __user *to, siginfo_t *from)
> {
> int err = 0;
> diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
> index beff97f..7a95816 100644
> --- a/arch/x86/include/asm/sighandling.h
> +++ b/arch/x86/include/asm/sighandling.h
> @@ -7,10 +7,10 @@
>
> #include <asm/processor-flags.h>
>
> -#define __FIX_EFLAGS (X86_EFLAGS_AC | X86_EFLAGS_OF | \
> +#define FIX_EFLAGS (X86_EFLAGS_AC | X86_EFLAGS_OF | \
> X86_EFLAGS_DF | X86_EFLAGS_TF | X86_EFLAGS_SF | \
> X86_EFLAGS_ZF | X86_EFLAGS_AF | X86_EFLAGS_PF | \
> - X86_EFLAGS_CF)
> + X86_EFLAGS_CF | X86_EFLAGS_RF)
>
> void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
>
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index 6956299..9df4c0b 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -43,12 +43,6 @@
>
> #include <asm/sigframe.h>
>
> -#ifdef CONFIG_X86_32
> -# define FIX_EFLAGS (__FIX_EFLAGS | X86_EFLAGS_RF)

Does anybody know why we had RF not restored in x86-32?

2013-04-24 15:39:21

by Frederic Weisbecker

Subject: Re: [PATCH 1/6] signal x86: Propage RF EFLAGS bit throught the signal restore call

On Wed, Apr 17, 2013 at 10:02:06PM +0200, Oleg Nesterov wrote:
> 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.

Hmm, ok.

Anyway patches 1-3 look good!

Thanks!