From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 107967 invoked by alias); 15 Jun 2017 15:23:46 -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 107958 invoked by uid 89); 15 Jun 2017 15:23:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.8 required=5.0 tests=BAYES_00,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=beneath, Hooks X-HELO: mail-io0-f177.google.com Received: from mail-io0-f177.google.com (HELO mail-io0-f177.google.com) (209.85.223.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 15 Jun 2017 15:23:42 +0000 Received: by mail-io0-f177.google.com with SMTP id i7so14089778ioe.1 for ; Thu, 15 Jun 2017 08:23:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=XL1fdtsmchQ3MVZgvwezmJ+ZClnWqbWomogNbExy15s=; b=Fc+jc42kkaKwxm+wM5McelJhm3ajXUl2zj+e10asDghMFs2zhRRgvzdc1jJ5gNze8p FL81cHiy9P+I/n4k4RgN2xsSLaJrOum338swTqUaB9FD7kB7SweoqMaxZNNib3jz57CG vhEeHwA8NuMP45c/4+Tr4nIrxp83w5k8+2KVj7VE/7Xyk+i8bwRza0Pj0yfyDBdRJ8h8 wAUTW8AcSHY5ce0ITe06lDti3QdnpDrHHg2d0Jq7tDdsLqhXT1SzwAWCs+NwZ+AgQNTc 9Y8S53rUlNDNzPDNLKk31j7cOobjW/WtdwGNxZ1hWvzRbO7RFEqZ+v+xy5U2Pv7sDKBq 6TAA== X-Gm-Message-State: AKS2vOyiXE9q7Kz0ReTveDHgWHQTqMgzogt1Os7h2vbK+o4GvPfsd4Kg VMiVgfFxN84wSg== X-Received: by 10.107.19.71 with SMTP id b68mr5601046ioj.0.1497540225422; Thu, 15 Jun 2017 08:23:45 -0700 (PDT) Received: from E107787-LIN (static.42.136.251.148.clients.your-server.de. [148.251.136.42]) by smtp.gmail.com with ESMTPSA id 21sm209203itz.6.2017.06.15.08.23.43 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Thu, 15 Jun 2017 08:23:44 -0700 (PDT) From: Yao Qi To: Philipp Rudo Cc: gdb-patches@sourceware.org, omair.javaid@linaro.org, yao.qi@linaro.org, peter.griffin@linaro.org, arnez@linux.vnet.ibm.com Subject: Re: [RFC v4 3/9] Add basic Linux kernel support References: <20170612170836.25174-1-prudo@linux.vnet.ibm.com> <20170612170836.25174-4-prudo@linux.vnet.ibm.com> Date: Thu, 15 Jun 2017 15:23:00 -0000 In-Reply-To: <20170612170836.25174-4-prudo@linux.vnet.ibm.com> (Philipp Rudo's message of "Mon, 12 Jun 2017 19:08:30 +0200") Message-ID: <86vanxtguc.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: 2017-06/txt/msg00449.txt.bz2 Philipp Rudo writes: > /* Initialize a private data entry for an address, where NAME is the name > + of the symbol, i.e. variable name in Linux, ALIAS the name used to > + retrieve the entry from hashtab, and SILENT a flag to determine if > + errors should be ignored. > + > + Returns a pointer to the new entry. In case of an error, either retu= rns > + NULL (SILENT =3D TRUE) or throws an error (SILENT =3D FALSE). If SIL= ENT =3D TRUE > + the caller is responsible to check for errors. > + > + Do not use directly, use LK_DECLARE_* macros defined in lk-low.h inst= ead. */ > + > +struct lk_private_data * > +lk_init_addr (const char *name, const char *alias, int silent) s/bool/int/ > +{ > + struct lk_private_data *data; > + struct bound_minimal_symbol bmsym; > + void **new_slot; > + void *old_slot; > + > + if ((old_slot =3D lk_find (alias)) !=3D NULL) > + return (struct lk_private_data *) old_slot; > + > + bmsym =3D lookup_minimal_symbol (name, NULL, NULL); > + > + if (bmsym.minsym =3D=3D NULL) > + { > + if (!silent) > + error (_("Could not find address %s. Aborting."), alias); > + return NULL; > + } > + > + data =3D XCNEW (struct lk_private_data); > + data->alias =3D alias; > + data->data.addr =3D BMSYMBOL_VALUE_ADDRESS (bmsym); > + > + new_slot =3D lk_find_slot (alias); > + *new_slot =3D data; > + > + return data; > +} > + > +/* Same as lk_init_addr but for structs. */ > + > +struct lk_private_data * > +lk_init_struct (const char *name, const char *alias, int silent) Likewise, bool silent. > +{ > + struct lk_private_data *data; > + const struct block *global; > + const struct symbol *sym; > + struct type *type; > + void **new_slot; > + void *old_slot; > + > + if ((old_slot =3D lk_find (alias)) !=3D NULL) > + return (struct lk_private_data *) old_slot; > + > + global =3D block_global_block(get_selected_block (0)); > + sym =3D lookup_symbol (name, global, STRUCT_DOMAIN, NULL).symbol; > + > + if (sym !=3D NULL) > + { > + type =3D SYMBOL_TYPE (sym); > + goto out; > + } > + > + /* Chek for "typedef struct { ... } name;"-like definitions. */ > + sym =3D lookup_symbol (name, global, VAR_DOMAIN, NULL).symbol; > + if (sym =3D=3D NULL) > + goto error; > + > + type =3D check_typedef (SYMBOL_TYPE (sym)); > + > + if (TYPE_CODE (type) =3D=3D TYPE_CODE_STRUCT) > + goto out; These two "goto" can be removed. > + > +error: > + if (!silent) > + error (_("Could not find %s. Aborting."), alias); > + > + return NULL; > + > +out: > + data =3D XCNEW (struct lk_private_data); > + data->alias =3D alias; > + data->data.type =3D type; > + > + new_slot =3D lk_find_slot (alias); > + *new_slot =3D data; > + > + return data; > +} > + > + > +/* Reads a bitmap at a given ADDRess of size SIZE (in bits). Allocates a= nd > + returns an array of ulongs. The caller is responsible to free the ar= ray > + after it is no longer needed. */ > + > +ULONGEST * > +lk_read_bitmap (CORE_ADDR addr, size_t size) > +{ > + ULONGEST *bitmap; > + size_t ulong_size, len; > + > + ulong_size =3D lk_builtin_type_size (unsigned_long); > + len =3D LK_DIV_ROUND_UP (size, ulong_size * LK_BITS_PER_BYTE); > + bitmap =3D XNEWVEC (ULONGEST, len); > + > + for (size_t i =3D 0; i < len; i++) > + bitmap[i] =3D lk_read_ulong (addr + i * ulong_size); > + > + return bitmap; > +} > + > +/* Return the next set bit in bitmap BITMAP of size SIZE (in bits) > + starting from bit (index) BIT. Return SIZE when the end of the bitmap > + was reached. To iterate over all set bits use macro > + LK_BITMAP_FOR_EACH_SET_BIT defined in lk-low.h. */ > + > +size_t > +lk_bitmap_find_next_bit (ULONGEST *bitmap, size_t size, size_t bit) > +{ > + size_t ulong_size, bits_per_ulong, elt; > + > + ulong_size =3D lk_builtin_type_size (unsigned_long); > + bits_per_ulong =3D ulong_size * LK_BITS_PER_BYTE; > + elt =3D bit / bits_per_ulong; > + > + while (bit < size) > + { > + /* FIXME: Explain why using lsb0 bit order. */ > + if (bitmap[elt] & (1UL << (bit % bits_per_ulong))) > + return bit; > + > + bit++; > + if (bit % bits_per_ulong =3D=3D 0) > + elt++; > + } > + > + return size; > +} > + > +/* Returns the Hamming weight, i.e. number of set bits, of bitmap BITMAP > + with size SIZE (in bits). */ > + > +size_t > +lk_bitmap_hweight (ULONGEST *bitmap, size_t size) > +{ > + size_t ulong_size, bit, bits_per_ulong, elt, retval; > + > + ulong_size =3D lk_builtin_type_size (unsigned_long); > + bits_per_ulong =3D ulong_size * LK_BITS_PER_BYTE; > + elt =3D bit =3D 0; > + retval =3D 0; > + > + while (bit < size) > + { > + if (bitmap[elt] & (1 << bit % bits_per_ulong)) > + retval++; > + > + bit++; > + if (bit % bits_per_ulong =3D=3D 0) > + elt++; > + } > + > + return retval; > +} > + Could you add some unit tests to these operations to bitmap? I am thinking that we need a class BitMap or BitVector later. > +/* Provide the per_cpu_offset of cpu CPU. See comment in lk-low.h for > + details. */ > + > +CORE_ADDR > +lk_get_percpu_offset (unsigned int cpu) > +{ > + size_t ulong_size =3D lk_builtin_type_size (unsigned_long); > + CORE_ADDR percpu_elt; > + > + /* Give the architecture a chance to overwrite default behaviour. */ > + if (LK_HOOK->get_percpu_offset) > + return LK_HOOK->get_percpu_offset (cpu); > + > + percpu_elt =3D LK_ADDR (__per_cpu_offset) + (ulong_size * cpu); > + return lk_read_addr (percpu_elt); > +} > + > + > + > +/* Function for targets to_update_thread_list hook. */ > + > +static void > +lk_update_thread_list (struct target_ops *target) > +{ > + prune_threads (); > + lk_update_running_tasks (); > + lk_update_sleeping_tasks (); > +} > + > +/* Function for targets to_fetch_registers hook. */ > + > +static void > +lk_fetch_registers (struct target_ops *target, > + struct regcache *regcache, int regnum) > +{ > + CORE_ADDR task; > + unsigned int cpu; > + > + task =3D (CORE_ADDR) regcache_get_ptid (regcache).tid (); > + cpu =3D lk_task_running (task); > + > + /* Let the target beneath fetch registers of running tasks. */ > + if (cpu !=3D LK_CPU_INVAL) > + { > + struct cleanup *old_inferior_ptid; > + > + old_inferior_ptid =3D save_inferior_ptid (); > + inferior_ptid =3D lk_cpu_to_old_ptid (cpu); Use make_scoped_restore (&inferior_ptid, lk_cpu_to_old_ptid (cpu));? > + linux_kernel_ops->beneath->to_fetch_registers (target, regcache, r= egnum); > + do_cleanups (old_inferior_ptid); > + } > + else > + { > + struct gdbarch *gdbarch; > + unsigned int i; > + > + LK_HOOK->get_registers (task, target, regcache, regnum); > + > + /* Mark all registers not found as unavailable. */ > + gdbarch =3D get_regcache_arch (regcache); > + for (i =3D 0; i < gdbarch_num_regs (gdbarch); i++) > + { > + if (regcache_register_status (regcache, i) =3D=3D REG_UNKNOWN) > + regcache_raw_supply (regcache, i, NULL); > + } > + } > +} > + > + > +/* Functions to initialize and free target_ops and its private data. As= well This line is too long. > + as functions for targets to_open/close/detach hooks. */ > + > +/* Check if OBFFILE is a Linux kernel. */ > + > + > +/* Initialize the cpu to old ptid map. Prefer the arch dependent > + map_running_task_to_cpu hook if provided, else assume that the PID us= ed > + by target beneath is the same as in task_struct PID task_struct. See > + comment on lk_ptid_map in lk-low.h for details. */ > + > +static void > +lk_init_ptid_map () > +{ > + struct thread_info *ti; > + ULONGEST *cpu_online_mask; > + size_t size; > + unsigned int cpu; > + struct cleanup *old_chain; > + > + if (LK_PRIVATE->old_ptid !=3D NULL) > + lk_free_ptid_map (); > + > + size =3D LK_BITMAP_SIZE (cpumask); > + cpu_online_mask =3D lk_read_bitmap (LK_ADDR (cpu_online_mask), size); > + old_chain =3D make_cleanup (xfree, cpu_online_mask); > + > + ALL_THREADS (ti) > + { > + struct lk_ptid_map *ptid_map =3D XCNEW (struct lk_ptid_map); > + CORE_ADDR rq, curr; > + int pid; > + > + /* Give the architecture a chance to overwrite default behaviour. = */ > + if (LK_HOOK->map_running_task_to_cpu) > + { > + ptid_map->cpu =3D LK_HOOK->map_running_task_to_cpu (ti); > + } > + else > + { > + LK_BITMAP_FOR_EACH_SET_BIT (cpu_online_mask, size, cpu) > + { > + rq =3D LK_ADDR (runqueues) + lk_get_percpu_offset (cpu); > + curr =3D lk_read_addr (rq + LK_OFFSET (rq, curr)); > + pid =3D lk_read_int (curr + LK_OFFSET (task_struct, pid)); > + > + if (pid =3D=3D ti->ptid.lwp ()) > + { > + ptid_map->cpu =3D cpu; > + break; > + } > + } > + if (cpu =3D=3D size) > + error (_("Could not map thread with pid %d, lwp %lu to a cpu."), > + ti->ptid.pid (), ti->ptid.lwp ()); > + } > + ptid_map->old_ptid =3D ti->ptid; > + ptid_map->next =3D LK_PRIVATE->old_ptid; > + LK_PRIVATE->old_ptid =3D ptid_map; > + } > + > + do_cleanups (old_chain); > +} > + > diff --git a/gdb/lk-low.h b/gdb/lk-low.h > new file mode 100644 > index 0000000000..be8c5556df > --- /dev/null > +++ b/gdb/lk-low.h > @@ -0,0 +1,310 @@ > +/* Basic Linux kernel support, architecture independent. > + > + 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 .= */ > + > +#ifndef __LK_LOW_H__ > +#define __LK_LOW_H__ > + > +#include "target.h" > + > +extern struct target_ops *linux_kernel_ops; > + > +/* Copy constants defined in Linux kernel. */ > +#define LK_TASK_COMM_LEN 16 > +#define LK_BITS_PER_BYTE 8 > + > +/* Definitions used in linux kernel target. */ > +#define LK_CPU_INVAL -1U > + > +/* Private data structs for this target. */ > +/* Forward declarations. */ > +struct lk_private_hooks; > +struct lk_ptid_map; > + > +/* Short hand access to private data. */ > +#define LK_PRIVATE ((struct lk_private *) linux_kernel_ops->to_data) > +#define LK_HOOK (LK_PRIVATE->hooks) > + > +struct lk_private > +{ > + /* Hashtab for needed addresses, structs and fields. */ > + htab_t data; > + > + /* Linked list to map between cpu number and original ptid from target > + beneath. */ > + struct lk_ptid_map *old_ptid; > + > + /* Hooks for architecture dependent functions. */ > + struct lk_private_hooks *hooks; > +}; > + > +/* We use the following convention for PTIDs: > + > + ptid->pid =3D inferiors PID > + ptid->lwp =3D PID from task_stuct > + ptid->tid =3D address of task_struct > + > + The task_structs address as TID has two reasons. First, we need it q= uite > + often and there is no other reasonable way to pass it down. Second, = it > + helps us to distinguish swapper tasks as they all have PID =3D 0. > + > + Furthermore we cannot rely on the target beneath to use the same PID = as the > + task_struct. Thus we need a mapping between our PTID and the PTID of = the > + target beneath. Otherwise it is impossible to pass jobs, e.g. fetching > + registers of running tasks, to the target beneath. */ Sorry, I don't understand this design. Can you elaborate? > + > +/* Private data struct to map between our and the target beneath PTID. = */ > + > +struct lk_ptid_map > +{ > + struct lk_ptid_map *next; Can you use C++ stl list instead? > + unsigned int cpu; > + ptid_t old_ptid; > +}; > + > +/* Private data struct to be stored in hashtab. */ > + > +struct lk_private_data > +{ > + const char *alias; > + > + union > + { > + CORE_ADDR addr; > + struct type *type; > + struct field *field; > + } data; > +}; > + > +/* Wrapper for htab_hash_string to work with our private data. */ > + > +static inline hashval_t > +lk_hash_private_data (const struct lk_private_data *entry) > +{ > + return htab_hash_string (entry->alias); > +} > + > +/* Function for htab_eq to work with our private data. */ > + > +static inline int > +lk_private_data_eq (const struct lk_private_data *entry, > + const struct lk_private_data *element) > +{ > + return streq (entry->alias, element->alias); > +} > + > +/* Wrapper for htab_find_slot to work with our private data. Do not use > + directly, use the macros below instead. */ > + > +static inline void ** > +lk_find_slot (const char *alias) > +{ > + const struct lk_private_data dummy =3D { alias }; > + return htab_find_slot (LK_PRIVATE->data, &dummy, INSERT); > +} > + > +/* Wrapper for htab_find to work with our private data. Do not use > + directly, use the macros below instead. */ > + > +static inline struct lk_private_data * > +lk_find (const char *alias) > +{ > + const struct lk_private_data dummy =3D { alias }; > + return (struct lk_private_data *) htab_find (LK_PRIVATE->data, &dummy); > +} > + > + > +struct lk_private_hooks > +{ > + /* required */ > + lk_hook_get_registers get_registers; > + > + /* optional, required if __per_cpu_offset array is not used to determi= ne > + offset. */ > + lk_hook_get_percpu_offset get_percpu_offset; > + > + /* optional, required if the target beneath uses a different PID as st= ruct > + rq. */ > + lk_hook_map_running_task_to_cpu map_running_task_to_cpu; > +}; The lk_private_hooks is still not a class. I raised this in v3 review, https://sourceware.org/ml/gdb-patches/2017-05/msg00004.html --=20 Yao (=E9=BD=90=E5=B0=A7)