From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7252 invoked by alias); 4 Jul 2016 10:50:00 -0000 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 Received: (qmail 7230 invoked by uid 89); 4 Jul 2016 10:49:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_LOTSOFHASH,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=985, 917, 91,7, Ken X-HELO: mail-pf0-f196.google.com Received: from mail-pf0-f196.google.com (HELO mail-pf0-f196.google.com) (209.85.192.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 04 Jul 2016 10:49:47 +0000 Received: by mail-pf0-f196.google.com with SMTP id i123so16138373pfg.3 for ; Mon, 04 Jul 2016 03:49:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=Ev7bzpqg276OOvRr7Gz8v/hkaZ3sHLl2ZnbVUV3+lWQ=; b=Hf6F+DOOp1h7pQErKyzlMV1A9weiS94OdjDPcyLKIwGgTcUt+RX66NFg12v1nvBt0b 4TAULW+TA5MvF0cUUATn3Y44JOIGFz0bga6RhpWzyGGSAHvd+VArCDq9tTksZFD3reTU UAzXzxGCy/0nE8ZseE5I6KYR+pdhHdgaEOsBSpydHuTX+C1ALdYMOGlgXL+tkPQLgJvm A9VfAGB6AIZ5HP/bhuUM2zMdn3rv/SIwCcCrNfOtOxZuSO3u2Ot0+p2VhaE73ecXMt0Y NrY4+9j8+DdsfA88LoTZKdklpRcG8kl9irlckAOFfEZP9/HeSIQnFgj66uekJilCpFXh L6eg== X-Gm-Message-State: ALyK8tK0ML3ZclOHVgBUkvaSRjwRDXpdOX4oQltjzrHwoxSc7bS6/WJ0UmIVeBdoH6fNng== X-Received: by 10.98.10.25 with SMTP id s25mr21378763pfi.44.1467629385804; Mon, 04 Jul 2016 03:49:45 -0700 (PDT) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id qc16sm4427161pab.1.2016.07.04.03.49.40 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 04 Jul 2016 03:49:44 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [RFC] Set process affinity in test to work around ARM ptrace bug References: <1467295036-2816-1-git-send-email-yao.qi@linaro.org> Date: Mon, 04 Jul 2016 10:50:00 -0000 In-Reply-To: (Pedro Alves's message of "Thu, 30 Jun 2016 16:31:58 +0100") Message-ID: <86a8hxzni8.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-07/txt/msg00039.txt.bz2 Pedro Alves writes: > I also think that whatever workaround, if any, should be limited > to known-broken kernels. Otherwise, this is likely to mask > other problems going forward. Maybe all we have is the version > number to work with, but that's still better than unconditionally > enabling this on arm. The updated version adds a linux kernel version check. --=20 Yao (=E9=BD=90=E5=B0=A7) =46rom 27fe094e6a99929f8f281d88beaa599771550025 Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Mon, 27 Jun 2016 08:45:16 +0100 Subject: [PATCH] Set process affinity in test to work around ARM ptrace bug We recently found a ARM kernel ptrace bug http://lists.infradead.org/pipermail/linux-arm-kernel/2016-May/431962.html As a result of this bug, after GDB ptrace set VFP registers, the hardware registers may not be updated. This bug causes some intermittent fails in tests, like return.exp, call-rt-st.exp, callfuncs.exp, etc. The bug was introduced by 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f in 2012 and is fixed in e2dfb4b880146bfd4b6aa8e138c0205407cebbaf in May. The bug is fixed in ARM kernel tree, but it is impractical to upgrade linux kernel from git tree or most recently release. I am wondering we can workaround this kernel bug somehow. My first attempt is to workaround it in GDB, so that GDB still writes the VFP registers and sync them to hardware. The kernel patch is quite simple, which moves vfp_flush_hwstate one line below. Probably, we can call ptrace set vfp registers twice, and then the second vfp set can flush the state correctly. Unfortunately, it doesn't work, because every time of ptrace set, kernel loads VFP registers from hardware first, which might be out of date after the first ptrace set. That is to say, we can't workaround this kernel bug in GDB. Then, I am thinking we can workaround this bug in testing, because the intermittent fails are confusing in comparing test results. We can bind both tracer and tracee on the same core. For example, we can start GDB or GDBserver with "taskset -c 0 ", but this is a global change, may have some affects on gdb.threads tests. I also think about doing "taskset -p PID -c 0" in test harness after the inferior is started, and do the same to the parent process of inferior (which is either GDB or GDBserver), but don't know how to get GDB (in remote host) and GDBserver's process id. The approach in this patch is to have a small c function which sets both process affinity and its parent's affinity to core 0 if the target is arm linux and the kernel version is known broken having the ptrace bug setting VFP registers. The function set_process_affinity should be called in these tests explicitly, but other tests are not affected at all. Note that this kernel bug only exists between commits 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f and e2dfb4b880146bfd4b6aa8e138c020= 5407cebbaf However, a certain commit will be merged to many branches and releases, which makes version checks complicated. I checked all released kernels, and get a list of versions that this bug is fixed. Not all longterm kernels on kernel.org have this bug fix, I don't know why, for example, some 3.x kernels doesn't have this bug fix. Secondly, kernels older than 8130b9d7b9d858aa04ce67805e8951e3cb6e9b2f are not affected by this bug, so the official kernel releases older than 3.0.21 or 3.2.6 are not affected by this bug, but I think the distro may backport the commit to their older kernel, so it makes few sense to check kernel is older than some versions (3.0.21 and 3.2.6). gdb/testsuite: 2016-07-04 Yao Qi * lib/set_process_affinity.c: New file. * gdb.arch/arm-neon.c: Include lib/set_process_affinity.c. (main): Call set_process_affinity. * gdb.base/callfuncs.c: Likewise. * gdb.base/call-rt-st.c: Likewise. * gdb.base/gnu_vector.c: Likewise. * gdb.base/return.c: Likewise. * gdb.base/return2.c: Likewise. * gdb.base/store.c: Likewise. * gdb.base/structs.c: Likewise. * gdb.arch/arm-neon.exp: Set breakpoint and continue to breakpoint. * gdb.base/gnu_vector.exp: Likewise. diff --git a/gdb/testsuite/gdb.arch/arm-neon.c b/gdb/testsuite/gdb.arch/arm= -neon.c index c67191c..f090b63 100644 --- a/gdb/testsuite/gdb.arch/arm-neon.c +++ b/gdb/testsuite/gdb.arch/arm-neon.c @@ -15,6 +15,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . = */ =20 +#include "../lib/set_process_affinity.c" #include =20 #define DEF_FUNC1(N, TYPE, VALUE...) \ @@ -98,5 +99,6 @@ DEF_FUNC2 (3) int main (void) { - return 0; + set_process_affinity (); + return 0; /* breakpoint here */ } diff --git a/gdb/testsuite/gdb.arch/arm-neon.exp b/gdb/testsuite/gdb.arch/a= rm-neon.exp index 053170f..d7a149d 100644 --- a/gdb/testsuite/gdb.arch/arm-neon.exp +++ b/gdb/testsuite/gdb.arch/arm-neon.exp @@ -31,6 +31,9 @@ if ![runto_main] { return -1 } =20 +gdb_breakpoint [gdb_get_line_number "breakpoint here"] +gdb_continue_to_breakpoint "breakpoint here" + # Test passing vectors in function argument in the inferior call. =20 for {set i 1} {$i <=3D 18} {incr i} { diff --git a/gdb/testsuite/gdb.base/call-rt-st.c b/gdb/testsuite/gdb.base/c= all-rt-st.c index 072ea86..ad97e28 100644 --- a/gdb/testsuite/gdb.base/call-rt-st.c +++ b/gdb/testsuite/gdb.base/call-rt-st.c @@ -1,3 +1,4 @@ +#include "../lib/set_process_affinity.c" #include #include #include @@ -565,6 +566,7 @@ int main () { struct two_floats_t *f3; =20 gdb_unbuffer_output (); + set_process_affinity (); =20 /* Allocate space for large structures=20 */ diff --git a/gdb/testsuite/gdb.base/callfuncs.c b/gdb/testsuite/gdb.base/ca= llfuncs.c index 317e7c4..cbc1977 100644 --- a/gdb/testsuite/gdb.base/callfuncs.c +++ b/gdb/testsuite/gdb.base/callfuncs.c @@ -25,6 +25,7 @@ #define PARAMS(paramlist) paramlist #endif =20 +#include "../lib/set_process_affinity.c" # include # include =20 @@ -644,7 +645,7 @@ voidfunc (void) =20 int main () { - void *p =3D malloc (1); + void *p =3D malloc (1); set_process_affinity (); t_double_values(double_val1, double_val2); t_structs_c(struct_val1); free (p); diff --git a/gdb/testsuite/gdb.base/gnu_vector.c b/gdb/testsuite/gdb.base/g= nu_vector.c index ee03ac1..8e0d6a8 100644 --- a/gdb/testsuite/gdb.base/gnu_vector.c +++ b/gdb/testsuite/gdb.base/gnu_vector.c @@ -18,6 +18,7 @@ Contributed by Ken Werner */ =20 #include +#include "../lib/set_process_affinity.c" =20 #define VECTOR(n, type) \ type __attribute__ ((vector_size (n * sizeof(type)))) @@ -137,7 +138,8 @@ main () { int4 res; =20 - res =3D add_some_intvecs (i4a, i4a + i4b, i4b); + set_process_affinity (); + res =3D add_some_intvecs (i4a, i4a + i4b, i4b); /* breakpoint here */ =20 res =3D add_some_intvecs (i4a, i4a + i4b, i4b); =20 diff --git a/gdb/testsuite/gdb.base/gnu_vector.exp b/gdb/testsuite/gdb.base= /gnu_vector.exp index aafaedd..1e57a26 100644 --- a/gdb/testsuite/gdb.base/gnu_vector.exp +++ b/gdb/testsuite/gdb.base/gnu_vector.exp @@ -55,6 +55,9 @@ gdb_test_multiple "show endian" "show endian" { } } =20 +gdb_breakpoint [gdb_get_line_number "breakpoint here"] +gdb_continue_to_breakpoint "breakpoint here" + # Test printing of character vector types gdb_test "print c4" "\\\$$decimal =3D \\{1, 2, 3, 4\\}" gdb_test "print c4\[2\]" "\\\$$decimal =3D 3" diff --git a/gdb/testsuite/gdb.base/return.c b/gdb/testsuite/gdb.base/retur= n.c index c365e88..6ff38e6 100644 --- a/gdb/testsuite/gdb.base/return.c +++ b/gdb/testsuite/gdb.base/return.c @@ -15,6 +15,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . = */ =20 +#include "../lib/set_process_affinity.c" #include /* Test "return" command. */ =20 @@ -40,6 +41,7 @@ double tmp3; =20 int main () { + set_process_affinity (); func1 (); printf("in main after func1\n"); tmp2 =3D func2 (); diff --git a/gdb/testsuite/gdb.base/return2.c b/gdb/testsuite/gdb.base/retu= rn2.c index ced472a..53e292f 100644 --- a/gdb/testsuite/gdb.base/return2.c +++ b/gdb/testsuite/gdb.base/return2.c @@ -15,6 +15,7 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . = */ =20 +#include "../lib/set_process_affinity.c" /* Test gdb's "return" command. */ =20 int void_test =3D 0; @@ -90,6 +91,7 @@ int main (int argc, char **argv) double double_resultval; int i; =20 + set_process_affinity (); /* A "test load" that will insure that the function really returns=20 a ${type} (as opposed to just a truncated or part of a ${type}). */ for (i =3D 0; i < sizeof (testval.ffff); i++) diff --git a/gdb/testsuite/gdb.base/store.c b/gdb/testsuite/gdb.base/store.c index 545515d..d878142 100644 --- a/gdb/testsuite/gdb.base/store.c +++ b/gdb/testsuite/gdb.base/store.c @@ -7,6 +7,8 @@ function calls within main even when no optimization flags were passed. */ =20 +#include "../lib/set_process_affinity.c" + typedef signed char charest; =20 charest @@ -254,6 +256,7 @@ wack_field_4 (void) int main () { + set_process_affinity (); /* These calls are for current frame test. */ wack_charest (-1, -2); wack_short (-1, -2); diff --git a/gdb/testsuite/gdb.base/structs.c b/gdb/testsuite/gdb.base/stru= cts.c index b5832cc..7be1fe0 100644 --- a/gdb/testsuite/gdb.base/structs.c +++ b/gdb/testsuite/gdb.base/structs.c @@ -15,6 +15,8 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . = */ =20 +#include "../lib/set_process_affinity.c" + /* Useful abreviations. */ typedef void t; typedef char tc; @@ -313,6 +315,8 @@ int main() { int i; =20 + set_process_affinity (); + for (i =3D 0; i < 256; i++) chartest[i].c =3D i; chartest[0].c =3D 0; /* chartest-done */ diff --git a/gdb/testsuite/lib/set_process_affinity.c b/gdb/testsuite/lib/s= et_process_affinity.c new file mode 100644 index 0000000..1d2a0e4 --- /dev/null +++ b/gdb/testsuite/lib/set_process_affinity.c @@ -0,0 +1,98 @@ +/* Copyright (C) 2016 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . = */ + +#if defined(__arm__) && defined(__linux__) +#define _GNU_SOURCE +#include +#include +#include +#include +#include + +struct version +{ + long major; + long minor; + long patch; +}; + +/* Probe the kernel version into V, and return 0 on success. */ + +static int +probe_kernel_version (struct version *v) +{ + struct utsname buffer; + + if (uname (&buffer) =3D=3D 0) + { + char *start, *end; + + start =3D buffer.release; + v->major =3D strtol (start, &end, 10); + + start =3D end + 1; + v->minor =3D strtol (start, &end, 10); + + start =3D end + 1; + v->patch =3D strtol (start, &end, 10); + return 0; + } + else + return -1; +} + +#define VERSION_NEWER_THAN(VER, MAJOR, MINOR, PATCH) \ + VER.major =3D=3D MAJOR && VER.minor =3D=3D MINOR && VER.patch >=3D PATCH + +#endif + +static void +set_process_affinity (void) +{ +#if defined(__arm__) && defined(__linux__) + struct version kernel; + cpu_set_t my_set; + + if (probe_kernel_version (&kernel)) + { + /* Can't get kernel version, do nothing. */ + return; + } + + if (kernel.major >=3D 5 + || (kernel.major =3D=3D 4 && kernel.minor >=3D 7) /* 4.7 and later */ + || VERSION_NEWER_THAN (kernel, 4, 6, 3) + || VERSION_NEWER_THAN (kernel, 4, 4, 14) + || VERSION_NEWER_THAN (kernel, 4, 1, 27) + || VERSION_NEWER_THAN (kernel, 3, 18, 36) + || VERSION_NEWER_THAN (kernel, 3, 14, 73)) + { + /* Kernel is new enough to have bug fixed, do nothing. */ + return; + } + + /* Set both process and parent process (GDB)'s affinity on core 0 to + workaround ARM linux kernel ptrace bug which doesn't flush the VFP + state to hardware after ptrace set VFP registers. */ + + CPU_ZERO (&my_set); + CPU_SET (0, &my_set); + + sched_setaffinity (0, sizeof(cpu_set_t), &my_set); + sched_setaffinity (getppid (), sizeof(cpu_set_t), &my_set); +#endif +}