From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 71500 invoked by alias); 31 Jan 2019 11:43:42 -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 71489 invoked by uid 89); 31 Jan 2019 11:43:42 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=findings, H*Ad:D*de.ibm.com X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 31 Jan 2019 11:43:40 +0000 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x0VBg9X8079890 for ; Thu, 31 Jan 2019 06:43:38 -0500 Received: from e06smtp03.uk.ibm.com (e06smtp03.uk.ibm.com [195.75.94.99]) by mx0b-001b2d01.pphosted.com with ESMTP id 2qbypr9r41-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 31 Jan 2019 06:43:38 -0500 Received: from localhost by e06smtp03.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 31 Jan 2019 11:43:37 -0000 Received: from b06cxnps4074.portsmouth.uk.ibm.com (9.149.109.196) by e06smtp03.uk.ibm.com (192.168.101.133) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Thu, 31 Jan 2019 11:43:33 -0000 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id x0VBhWDI8847750 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 31 Jan 2019 11:43:32 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id CAFCAAE053; Thu, 31 Jan 2019 11:43:32 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 80E40AE045; Thu, 31 Jan 2019 11:43:32 +0000 (GMT) Received: from laptop-ibm (unknown [9.152.212.204]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 31 Jan 2019 11:43:32 +0000 (GMT) Date: Thu, 31 Jan 2019 11:43:00 -0000 From: Philipp Rudo To: John Baldwin Cc: Omair Javaid , gdb-patches@sourceware.org, palves@redhat.com, arnez@linux.vnet.ibm.com, peter.griffin@linaro.org, Ulrich.Weigand@de.ibm.com, kieran@linuxembedded.co.uk Subject: Re: [RFC PATCH 4/6] Linux kernel debug infrastructure and target Linux kernel In-Reply-To: <0ac8d4e2-6cbe-19b5-d9dd-52bec9cec00a@FreeBSD.org> References: <1548738199-9403-1-git-send-email-omair.javaid@linaro.org> <1548738199-9403-5-git-send-email-omair.javaid@linaro.org> <0ac8d4e2-6cbe-19b5-d9dd-52bec9cec00a@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit x-cbid: 19013111-0012-0000-0000-000002EF6A23 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 19013111-0013-0000-0000-00002126B529 Message-Id: <20190131124331.6a7d6964@laptop-ibm> X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00620.txt.bz2 Hi John, your points all originate from my original patch set. So let me try to give you some background (although I haven't worked on the project for quite some time and my memory is a little rusty...) @Omair: I'm still looking at the patches. I'll send my findings in a separate mail. > A couple of thoughts I had: > > 1) You might be able to simplify some of the code by using 'parse_and_eval_long' > to read the value of global variables. This takes care of endianness, etc. > without requiring you to manually use msymbols, read_memory_* etc. For > example, I read some global read-only variables this way that describe > the offsets of some fields used to enumerate kernel threads in FreeBSD > using this: > > /* > * Newer kernels export a set of global variables with the offsets > * of certain members in struct proc and struct thread. For older > * kernels, try to extract these offsets using debug symbols. If > * that fails, use native values. > */ > TRY { > proc_off_p_pid = parse_and_eval_long("proc_off_p_pid"); > proc_off_p_comm = parse_and_eval_long("proc_off_p_comm"); > proc_off_p_list = parse_and_eval_long("proc_off_p_list"); > proc_off_p_threads = parse_and_eval_long("proc_off_p_threads"); > thread_off_td_tid = parse_and_eval_long("thread_off_td_tid"); > thread_off_td_name = parse_and_eval_long("thread_off_td_name"); > thread_off_td_oncpu = parse_and_eval_long("thread_off_td_oncpu"); > thread_off_td_pcb = parse_and_eval_long("thread_off_td_pcb"); > thread_off_td_plist = parse_and_eval_long("thread_off_td_plist"); > thread_oncpu_size = 4; > } CATCH(e, RETURN_MASK_ERROR) { Do you mean the lk_read_{int,ulong,...} or the lk_find_address function? Your example looks more like lk_find_address. There parse_and_eval_address could do the trick. While implementing however I played around quite a lot and I remember using parse_and_eval_* for some time. But, if I recall correct, there where some problems with Assembler labels (like _text) and I switched to the current implementation. For lk_read_* I don't think parse_and_eval_* is a proper replacement. Their purpose is to read arbitrary (dynamically allocated) memory locations. So to replace them with parse_and_eval you'd first would have to build some C-like expression which then gets evaluated again. This sounds like a huge overhead to me. > > 2) The code to compute offsetof is more generally useful and is probably > worth pulling out. I recently had to do something similar for TLS > support for FreeBSD userland binaries to deal with offsets in the > runtime linker link_map. Currently I'm just relying on a manual > offsetof, but it would be better to implement a 'lookup_struct_elt_offset' > helper I think which you have the guts of for at least C. You can find > my mail about that here: > > https://sourceware.org/ml/gdb-patches/2019-01/msg00540.html Hmm, right. It shouldn't take much to convert lk_find_filed to some generic offsetof function. > > 3) Finally, adding a new field to gdbarch for lk_ops is probably fine. I > chose to use gdbarch_data for this instead for the FreeBSD bits. To do > this I setup a key and exposed some helper functions to allow a gdbarch > to set pointers in a similar structure in > https://github.com/bsdjhb/gdb/blob/kgdb/gdb/fbsd-kvm.c: For me both ways look pretty much the same. The nice thing about adding a field to gdbarch is that you don't have to implement the helpers yourself but let gdbarch.sh do the work for you :-) Thanks Philipp > > /* Per-architecture data key. */ > static struct gdbarch_data *fbsd_vmcore_data; > > struct fbsd_vmcore_ops > { > /* Supply registers for a pcb to a register cache. */ > void (*supply_pcb)(struct regcache *, CORE_ADDR); > > /* Return address of pcb for thread running on a CPU. */ > CORE_ADDR (*cpu_pcb_addr)(u_int); > }; > > static void * > fbsd_vmcore_init (struct obstack *obstack) > { > struct fbsd_vmcore_ops *ops; > > ops = OBSTACK_ZALLOC (obstack, struct fbsd_vmcore_ops); > return ops; > } > > /* Set the function that supplies registers from a pcb > for architecture GDBARCH to SUPPLY_PCB. */ > > void > fbsd_vmcore_set_supply_pcb (struct gdbarch *gdbarch, > void (*supply_pcb) (struct regcache *, > CORE_ADDR)) > { > struct fbsd_vmcore_ops *ops = (struct fbsd_vmcore_ops *) > gdbarch_data (gdbarch, fbsd_vmcore_data); > ops->supply_pcb = supply_pcb; > } > > /* Set the function that returns the address of the pcb for a thread > running on a CPU for > architecture GDBARCH to CPU_PCB_ADDR. */ > > void > fbsd_vmcore_set_cpu_pcb_addr (struct gdbarch *gdbarch, > CORE_ADDR (*cpu_pcb_addr) (u_int)) > { > struct fbsd_vmcore_ops *ops = (struct fbsd_vmcore_ops *) > gdbarch_data (gdbarch, fbsd_vmcore_data); > ops->cpu_pcb_addr = cpu_pcb_addr; > } > > ... > > void > _initialize_kgdb_target(void) > { > > ... > fbsd_vmcore_data = gdbarch_data_register_pre_init(fbsd_vmcore_init); > ... > } > > Then the various architecture-specific kernel architectures invoke those > extra methods while setting up an architecture similar to what you are doing, > e.g. from arm-fbsd-kern.c: > > /* Implement the 'init_osabi' method of struct gdb_osabi_handler. */ > > static void > arm_fbsd_kernel_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) > { > struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > > frame_unwind_prepend_unwinder (gdbarch, &arm_fbsd_trapframe_unwind); > > set_solib_ops (gdbarch, &kld_so_ops); > > tdep->jb_pc = 24; > tdep->jb_elt_size = 4; > > fbsd_vmcore_set_supply_pcb (gdbarch, arm_fbsd_supply_pcb); > fbsd_vmcore_set_cpu_pcb_addr (gdbarch, kgdb_trgt_stop_pcb); > > /* Single stepping. */ > set_gdbarch_software_single_step (gdbarch, arm_software_single_step); > } >