From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31385 invoked by alias); 3 Jan 2017 12:55:36 -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 31371 invoked by uid 89); 3 Jan 2017 12:55:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=H*Ad:U*cagney, bright, endless, ghost X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 03 Jan 2017 12:55:25 +0000 Received: from pps.filterd (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id v03CsFut093620 for ; Tue, 3 Jan 2017 07:55:23 -0500 Received: from e06smtp13.uk.ibm.com (e06smtp13.uk.ibm.com [195.75.94.109]) by mx0a-001b2d01.pphosted.com with ESMTP id 27r3q0kt7a-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 03 Jan 2017 07:55:23 -0500 Received: from localhost by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 3 Jan 2017 12:55:20 -0000 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp13.uk.ibm.com (192.168.101.143) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 3 Jan 2017 12:55:15 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id C2C6417D8063; Tue, 3 Jan 2017 12:58:02 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v03CtF9u20447620; Tue, 3 Jan 2017 12:55:15 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 674A9AE053; Tue, 3 Jan 2017 11:53:21 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id EE7EAAE059; Tue, 3 Jan 2017 11:53:20 +0000 (GMT) Received: from ThinkPad (unknown [9.152.212.192]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Tue, 3 Jan 2017 11:53:20 +0000 (GMT) Date: Tue, 03 Jan 2017 12:55:00 -0000 From: Philipp Rudo To: Peter Griffin Cc: gdb-patches@sourceware.org, palves@redhat.com, brobecker@adacore.com, kevinb@redhat.com, cagney@gnu.org, dje@google.com, drow@false.org, kettenis@gnu.org, yao.qi@arm.com, stanshebs@google.com, Ulrich.Weigand@de.ibm.com, elena.zannoni@oracle.com, eliz@gnu.org, arnez@linux.vnet.ibm.com Subject: Re: [PATCH] Linux kernel thread runtime support In-Reply-To: <1482427864-4317-1-git-send-email-peter.griffin@linaro.org> References: <1482427864-4317-1-git-send-email-peter.griffin@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17010312-0012-0000-0000-000004B0F3DC X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17010312-0013-0000-0000-000016C6731F Message-Id: <20170103135514.0dad741e@ThinkPad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-01-03_11:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=8 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1701030208 X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00008.txt.bz2 Hi Peter Hi everybody else well it had to come this way. I am also working on an kernel debugging target quite similar to yours and planed to send my code next week or the one after... On the bright side, till now i worked on core dumps on S390 and also have some features (especially module support with kernel virtual memory handling) you are still missing. So not all the work is in vain we just need to find a common basis. Please give me some days so i can finalize my code and send it to the list. Then we can discuss it in detail. Nevertheless i found some issues in your basic thread handling logic (from kernel point of view) you should have a look at. See comments below. Philipp =46rom linux-kthread.c: [...] > /* asm/generic/percpu.h > * per_cpu_offset() is the offset that has to be added to a > * percpu variable to get to the instance for a certain processor. > * Most arches use the __per_cpu_offset array for those offsets but > * some arches have their own ways of determining the offset (x86_64, > s390). */ Yes, S390 is special in this case. You need to add an arch specific hook. I made the hook optional, i.e. if not set the default method via __per_cpu_offset array is used. > DECLARE_ADDR (__per_cpu_offset); > DECLARE_ADDR (per_cpu__process_counts); > DECLARE_ADDR (process_counts); > DECLARE_ADDR (per_cpu__runqueues); > DECLARE_ADDR (runqueues); >=20 > #define CORE_INVAL (-1) CORE_INVAL is defined a second time in linux-kthread.h. [...] > void > lkthread_get_per_cpu_offsets(int numcores) > { > enum bfd_endian byte_order =3D gdbarch_byte_order (target_gdbarch ()); > int length =3D TYPE_LENGTH (builtin_type (target_gdbarch > ())->builtin_data_ptr); CORE_ADDR curr_addr =3D ADDR (__per_cpu_offset); > int core; >=20 >=20 > if (!HAS_ADDR (__per_cpu_offset)) > { > if (debug_linuxkthread_threads) > fprintf_unfiltered (gdb_stdlog, "Assuming non-SMP kernel.\n"); >=20 > return; > } >=20 > for (core=3D0; core < numcores; core++) This iteration does not work. As CPUs can be added/removed the logical CPU-ids needn't be continuous, e.g. with two CPUs the ids can be 0 and 2. You have to walk the cpu_online_mask to see which CPUs are used at the moment. The iteration appears several times in the code. > { > if (!lkthread_h->per_cpu_offset[core]) > lkthread_h->per_cpu_offset[core] =3D > read_memory_unsigned_integer (curr_addr, length, > byte_order); >=20 > curr_addr +=3D (CORE_ADDR) length; >=20 > if (!lkthread_h->per_cpu_offset[core]) > { > warning ("Suspicious null per-cpu offsets," > " or wrong number of detected cores:\n" > "ADDR (__per_cpu_offset) =3D %s\nmax_cores =3D %d", > phex (ADDR (__per_cpu_offset),4), max_cores); >=20 > break; > } > } >=20 > if (debug_linuxkthread_threads) > fprintf_unfiltered (gdb_stdlog, "SMP kernel. %d cores detected\n", > /numcores); > } [...] > static void > lkthread_init (void) > { > struct thread_info *th =3D NULL; > struct cleanup *cleanup; > int size =3D > TYPE_LENGTH (builtin_type (target_gdbarch > ())->builtin_unsigned_long); >=20 > /* Ensure thread list from beneath target is up to date. */ > cleanup =3D make_cleanup_restore_integer (&print_thread_events); > print_thread_events =3D 0; > update_thread_list (); > do_cleanups (cleanup); >=20 > /* Count the h/w threads. */ > max_cores =3D thread_count (); The number of CPUs does not have to be constant. Especially when you are running in a VM you can add/remove CPUs. Thus max_cores has to be determined dynamically or set to the max number of CPUs, e.g. by taking the size (in bits) of cpu_online_mask.=20 > gdb_assert (max_cores); >=20 > if (debug_linuxkthread_threads) > { > fprintf_unfiltered (gdb_stdlog, "lkthread_init() cores(%d) GDB" > "HW threads\n", max_cores); > iterate_over_threads (thread_print_info, NULL); > } >=20 > /* Allocate per cpu data. */ > lkthread_alloc_percpu_data(max_cores); Together with the comment above, allocating the percpu_data to a fixed size can cause problems when the number of CPUs is increased. > lkthread_get_per_cpu_offsets(max_cores); >=20 > if (!lkthread_get_runqueues_addr () && (max_cores > 1)) > fprintf_unfiltered (gdb_stdlog, "Could not find the address of > CPU" " runqueues current context information maybe " > "less precise\n."); >=20 > /* Invalidate the linux-kthread cached list. */ > lkthread_invalidate_threadlist (); > } [...] > static linux_kthread_info_t ** > lkthread_get_threadlist_helper (linux_kthread_info_t ** ps) > { > struct linux_kthread_arch_ops *arch_ops =3D > gdbarch_linux_kthread_ops (target_gdbarch ()); > CORE_ADDR rq_idle_taskstruct; > CORE_ADDR g, t, init_task_addr; > int core =3D 0, i; >=20 > if (debug_linuxkthread_threads) > fprintf_unfiltered (gdb_stdlog, > "lkthread_get_threadlist_helper\n"); >=20 > init_task_addr =3D ADDR (init_task); > g =3D init_task_addr; >=20 > do > { > t =3D g; > do > { >=20 > if (!arch_ops->is_kernel_address(t)) > { > warning ("parsing of task list stopped because of > invalid address" "%s", phex (t, 4)); > break; > } >=20 > lkthread_get_task_info (t, ps, core /*zero-based */ ); > core =3D CORE_INVAL; >=20 > if (ptid_get_tid (PTID_OF (*ps)) =3D=3D 0) > { > /* This is init_task, let's insert the other cores > swapper now. */ > for (i =3D 1; i < max_cores; i++) > { > ps =3D &((*ps)->next); > rq_idle_taskstruct =3D lkthread_get_rq_idle (i); > lkthread_get_task_info (rq_idle_taskstruct, ps, i); > } > } How does the setting to a core work? For me it looks like only for init_task and the swapper tasks core !=3D CORE_INVAL. Is that the intended behavior? > if (debug_linuxkthread_threads) > fprintf_unfiltered (gdb_stdlog, "Got task info for %s > (%li)\n", (*ps)->comm, ptid_get_lwp (PTID_OF (*ps))); >=20 > ps =3D &((*ps)->next); >=20 > /* Mark end of chain and remove those threads that > disappeared from the thread_list to avoid any_thread_of_process() to > select a ghost. */ > if (*ps) > (*ps)->valid =3D 0; >=20 > t =3D _next_thread (t); > } while (t && (t !=3D g)); >=20 > g =3D _next_task (g); > } while (g && (g !=3D init_task_addr)); Although pretty unlikely, the do {} while can end in an endless loop when you have a memory corruption compromising one of the list_heads. Thus there should be more sanity checks than just the one for NULL. I use list_head->next->prev =3D=3D list_head. > return ps; > } On Thu, 22 Dec 2016 17:31:03 +0000 Peter Griffin wrote: > Hi GDB maintainers, >=20 > The following patch implements a Linux kernel thread runtime stratum > which can be used when using GDB to debug a Linux kernel. For example > when connecting to a QEMU GDB stub, or OpenOCD which then communicates > with the target via JTAG or SWD. >=20 > This patch is a refactored version based on the 'Linux kernel > debugger' GDB plugin written at STMicroelectronics which used to > be packaged with their JTAG debuggers. There has been some discussion > previously on the list by myself [1], Kieran [2] and one of the > original authors at ST Marc Titinger [3]. >=20 > This patchset I'm hoping is a lot closer to something which can > be upstreamed to GDB project after some discussion about structure > with Yao Qi at Linaro Connect, the code has been refactored to be > structured much more like the existing upstream thread runtimes (such > as ravenscar-thread.c and sparc-ravenscar-thread etc). >=20 > Since the original email [1] various helper commands have also been > migrated into python and merged into the Linux kernel source tree. > The GDB python extensions, combined with the linux-kthread GDB > thread runtime implemented in this patchset provide a powerful > Linux kernel debug solution, and is a working implementation > of what Andreas talked about on slide 17 and 18 of his talk at GNU > Cauldron [4]. >=20 > I have currently been testing this patchset using mainline GDB > debugging arm-linux kernels in Qemu and via OpenoCD to real hardware. > It is straight forward with the current strructure to add new > architecture support, and I'm looking at adding PowerPC so I can > easily validate big endian targets. >=20 > What I'm really hoping for now is some patch review on the GDB > mailing list from the GDB maintainers and community with a view to > getting this functionality which has been talked about for quite a > few years finally merged to the upstream GDB project. >=20 > All patch review feedback greatfully received :) >=20 > kind regards, >=20 > Peter. >=20 > [1] https://cygwin.com/ml/gdb/2015-09/msg00032.html > [2] https://www.sourceware.org/ml/gdb/2016-01/msg00028.html > [3] > https://lists.linaro.org/pipermail/linaro-toolchain/2011-November/001754.= html > [4] > https://gcc.gnu.org/wiki/cauldron2015?action=3DAttachFile&do=3Dview&targe= t=3DAndreas+Arnez_+Debugging+Linux+kernel+dumps+with+GDB.pdf >=20 >=20 >=20 > Peter Griffin (1): > Add Linux kernel thread runtime support. >=20 > gdb/ChangeLog | 12 + > gdb/Makefile.in | 8 +- > gdb/arm-linux-kthread.c | 178 +++++ > gdb/arm-linux-kthread.h | 27 + > gdb/arm-tdep.c | 4 + > gdb/configure.tgt | 6 +- > gdb/gdbarch.c | 23 + > gdb/gdbarch.h | 5 + > gdb/gdbarch.sh | 3 + > gdb/linux-kthread.c | 1828 > +++++++++++++++++++++++++++++++++++++++++++++++ > gdb/linux-kthread.h | 223 ++++++ 11 files changed, 2311 > insertions(+), 6 deletions(-) create mode 100644 > gdb/arm-linux-kthread.c create mode 100644 gdb/arm-linux-kthread.h > create mode 100644 gdb/linux-kthread.c > create mode 100644 gdb/linux-kthread.h >=20