From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81412 invoked by alias); 9 Feb 2017 09:54:43 -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 81401 invoked by uid 89); 9 Feb 2017 09:54:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.1 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,KAM_STOCKGEN,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 spammy=htab, UD:tgt, configure.tgt, UD:configure.tgt 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; Thu, 09 Feb 2017 09:54:39 +0000 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v199sAab112036 for ; Thu, 9 Feb 2017 04:54:38 -0500 Received: from e06smtp11.uk.ibm.com (e06smtp11.uk.ibm.com [195.75.94.107]) by mx0a-001b2d01.pphosted.com with ESMTP id 28gm1ppf9f-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 09 Feb 2017 04:54:38 -0500 Received: from localhost by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 9 Feb 2017 09:54:35 -0000 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp11.uk.ibm.com (192.168.101.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 9 Feb 2017 09:54:32 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id BF91F1B08023; Thu, 9 Feb 2017 09:57:19 +0000 (GMT) Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v199sP2c65863914; Thu, 9 Feb 2017 09:54:25 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 107AC5236C; Thu, 9 Feb 2017 08:53:10 +0000 (GMT) Received: from ThinkPad (unknown [9.152.212.148]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id E90845235C; Thu, 9 Feb 2017 08:53:09 +0000 (GMT) Date: Thu, 09 Feb 2017 09:54:00 -0000 From: Philipp Rudo To: Yao Qi Cc: gdb-patches@sourceware.org, Andreas Arnez Subject: Re: [RFC 3/7] Add basic Linux kernel support In-Reply-To: <20170207173944.GA32023@E107787-LIN> References: <20170112113217.48852-1-prudo@linux.vnet.ibm.com> <20170112113217.48852-4-prudo@linux.vnet.ibm.com> <20170207105403.GA1630@E107787-LIN> <20170207160444.34c46791@ThinkPad> <20170207173944.GA32023@E107787-LIN> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17020909-0040-0000-0000-000003441856 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17020909-0041-0000-0000-00001ED72266 Message-Id: <20170209105424.29428e1c@ThinkPad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-02-09_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702090098 X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00212.txt.bz2 Hi Yao, On Tue, 7 Feb 2017 17:39:44 +0000 Yao Qi wrote: > On 17-02-07 16:04:44, Philipp Rudo wrote: > > Hi Yao > > > > > > On Tue, 7 Feb 2017 10:54:03 +0000 > > Yao Qi wrote: > > > > > On 17-01-12 12:32:13, Philipp Rudo wrote: > > > > (ALL_TARGET_OBS): Add lk-low.o > > > > > > ALL_TARGET_OBS is used with --enable-targets=all, so if we put > > > lk-low.o in it, lk-low.o can't be linked into GDB if we don't > > > enable all targets. > > > > you kind of lost me here. As I understand it lk-low.o needs to be > > added to ALL_TARGET_OPS. The thing I could do is adding entries in > > configure.tgt. Although I'm not sure if adding lk-low to all > > arch*-*-linux* (like Peter did) is better than adding a general > > *-*-lk* target. The benefit of the first is that the target is > > added automatically to all linux builds. While the second allows > > more flexibility and turning kernel debugging on/off as needed. > > > > I don't have a strong opinion on which way to go. Do you prefer one > > way? > > Yes, lk-low.o needs to be added to ALL_TARGET_OBS, but that is not > enough. I get the following compile error on x86_64-linux, > > lk-lists.o: In function `lk_find': > /home/yao/SourceCode/gnu/gdb/git/gdb/lk-low.h:125: undefined > reference to > `linux_kernel_ops' /home/yao/SourceCode/gnu/gdb/git/gdb/lk-low.h:125: > undefined reference to `linux_kernel_ops' lk-lists.o: In function > `lk_list_head_next(unsigned long, char const*, char > const*)': /home/yao/SourceCode/gnu/gdb/git/gdb/lk-lists.c:35: > undefined reference to `lk_read_addr(unsigned > long)' /home/yao/SourceCode/gnu/gdb/git/gdb/lk-lists.c:36: undefined > reference to `lk_read_addr(unsigned long)' > > because lk-low.o isn't linked into the executable. > > I think both lk-low.o and lk-lists.o should be added for each > $ARCH-linux target in configure.tgt. Ok, now I get it. Thanks for pointing this out I simply forgot to test without --enable-targets=all... While fixing this I also noticed that I forgot adding the new s390-tdep.o in my split up patch in v2. I will fix it in v3. > > > > (COMMON_OBS): Add lk-lists.o > > > > --- > > > > > > > + > > > > +/* 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 returns > > > > + NULL (SILENT = TRUE) or throws an error (SILENT = FALSE). > > > > If SILENT = TRUE > > > > + the caller is responsible to check for errors. > > > > + > > > > + Do not use directly, use LK_DECLARE_* macros defined in > > > > lk-low.h instead. */ + > > > > +struct lk_private_data * > > > > +lk_init_addr (const char *name, const char *alias, int silent) > > > > +{ > > > > + /* Initialize to NULL to silence gcc. */ > > > > + struct value *val = NULL; > > > > + struct lk_private_data *data; > > > > + void **new_slot; > > > > + void *old_slot; > > > > + > > > > + if ((old_slot = lk_find (alias)) != NULL) > > > > + return (struct lk_private_data *) old_slot; > > > > + > > > > + TRY > > > > + { > > > > + /* Choose global block for search, in case the variable > > > > was redefined > > > > + in the current context. */ > > > > + const struct block *global = block_global_block > > > > (get_selected_block (0)); > > > > + const char *tmp = name; > > > > + expression_up expr = parse_exp_1 (&tmp, 0, global, > > > > 0); > > > > > > Why don't you look up symbol or msymbol? like Peter's patch > > > does. > > > > I used lookup_symbol in the beginning. But at some point it > > suddenly made problems and couldn't find a symbol. I actually never > > found out what went wrong but to parse_exp solved the problem for > > me. And, as it's an init function only used only once per symbol I > > thought that adding this extra overhead is excusable. > > Do you have an example that lookup_symbol doesn't work but parse_exp > works? No sorry not anymore it got lost in some cleanup. I did it quite early in development and can't remember everything but I guess that it was either a memory corruption (I had an use-after-relocate bug in my htab handling) or that I just didn't understood GDB's symbolhandling when I stared working on the project. Nevertheless I looked at it again and it works. In addition it is much nicer and shorter. The only thing I don't really like are the "goto out/error" in lk_init_struct but that is just a minor flaw. The two functions now look like this struct lk_private_data * lk_init_addr (const char *name, const char *alias, int silent) { struct lk_private_data *data; struct bound_minimal_symbol bmsym; void **new_slot; void *old_slot; if ((old_slot = lk_find (alias)) != NULL) return (struct lk_private_data *) old_slot; bmsym = lookup_minimal_symbol (name, NULL, NULL); if (bmsym.minsym == NULL) { if (!silent) error (_("Could not find address %s. Aborting."), alias); return NULL; } data = XCNEW (struct lk_private_data); data->alias = alias; data->data.addr = BMSYMBOL_VALUE_ADDRESS (bmsym); new_slot = lk_find_slot (alias); *new_slot = data; return data; } struct lk_private_data * lk_init_struct (const char *name, const char *alias, int 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 = lk_find (alias)) != NULL) return (struct lk_private_data *) old_slot; global = block_global_block(get_selected_block (0)); sym = lookup_symbol (name, global, STRUCT_DOMAIN, NULL).symbol; if (sym != NULL) { type = SYMBOL_TYPE (sym); goto out; } /* Chek for "typedef struct { ... } name;"-like definitions. */ sym = lookup_symbol (name, global, VAR_DOMAIN, NULL).symbol; if (sym == NULL) goto error; type = check_typedef (SYMBOL_TYPE (sym)); if (TYPE_CODE (type) == TYPE_CODE_STRUCT) goto out; error: if (!silent) error (_("Could not find %s. Aborting."), alias); return NULL; out: data = XCNEW (struct lk_private_data); data->alias = alias; data->data.type = type; new_slot = lk_find_slot (alias); *new_slot = data; return data; } > > > (gdb) b main > > > Breakpoint 1 at 0x4004fa: > > > file /home/yao/SourceCode/gnu/gdb/git/gdb/testsuite/gdb.base/linux-kernel.c, > > > line 59. (gdb) run Starting > > > program: /scratch/yao/gdb/build-git/x86_64/gdb/testsuite/outputs/gdb.base/linux-kernel/linux-kernel > > > Could not map thread with pid 28278, lwp 28278 to a cpu. > > > > > > I hope we can have a small test case in gdb testsuite to test > > > linux kernel debugging. Is it possible? We can generate a > > > normal coredump to linux-kernel in test, and we can also set up > > > task_struct and expect GDB sees some "threads". What do you > > > think? > > > > Andreas and I had the same idea :-) > > I don't know If it really works. But we think its the most > > promising approach for the testsuite. The biggest problem I see is > > the relocation done for the kernel modules. Here you need to > > somehow mimic a dynamic linking done in the kernel. Which I > > suppose not to be that simple, but still doable. > > > > How is it related to kernel modules? I don't want to test kernel > module handling in this test case. When you are only interested in threads it isn't related at all. My intention was to indicate that when you want to have test cases for the full target, including module handling, you will need to somehow cope with it. Although I'm not sure if this approach is the best for modules. Besides the relocation problem, they also live in kernel virtual memory. Thus you'd need a test case which simulates it's own address space... In my opinion both functions need to be tested when the code is integrated to master. Otherwise GDB might print wrong and misleading information to the user without noticing it. Although I must admit that writing working test cases won't be easy. Thanks Philipp