public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Yao Qi <qiyaoltc@gmail.com>
To: Philipp Rudo <prudo@linux.vnet.ibm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFC 3/7] Add basic Linux kernel support
Date: Tue, 07 Feb 2017 17:39:00 -0000	[thread overview]
Message-ID: <20170207173944.GA32023@E107787-LIN> (raw)
In-Reply-To: <20170207160444.34c46791@ThinkPad>

On 17-02-07 16:04:44, Philipp Rudo wrote:
> Hi Yao
> 
> 
> On Tue, 7 Feb 2017 10:54:03 +0000
> Yao Qi <qiyaoltc@gmail.com> 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.

> 
> > >     (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?

> > 
> > (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.

-- 
Yao (齐尧)

  reply	other threads:[~2017-02-07 17:39 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-12 11:32 [RFC 0/7] Support for Linux kernel debugging Philipp Rudo
2017-01-12 11:32 ` [RFC 7/7] Add S390 support for linux-kernel target Philipp Rudo
2017-01-12 17:09   ` Luis Machado
2017-01-13 11:46     ` Philipp Rudo
2017-02-06 15:52     ` Yao Qi
2017-02-06 18:48       ` Andreas Arnez
2017-01-12 11:32 ` [RFC 1/7] Convert substitute_path_component to C++ Philipp Rudo
2017-01-12 11:32 ` [RFC 2/7] Add libiberty/concat styled concat_path function Philipp Rudo
2017-01-12 12:00   ` Pedro Alves
2017-01-12 13:33     ` Philipp Rudo
2017-01-12 13:48       ` Pedro Alves
2017-01-12 15:09         ` Philipp Rudo
2017-01-12 15:42           ` Pedro Alves
2017-01-12 11:32 ` [RFC 6/7] Add privileged registers for s390x Philipp Rudo
2017-01-12 11:32 ` [RFC 3/7] Add basic Linux kernel support Philipp Rudo
2017-02-07 10:54   ` Yao Qi
2017-02-07 15:04     ` Philipp Rudo
2017-02-07 17:39       ` Yao Qi [this message]
2017-02-09  9:54         ` Philipp Rudo
2017-02-09 13:06     ` Yao Qi
2017-02-09 15:09       ` Philipp Rudo
2017-01-12 11:32 ` [RFC 5/7] Add commands for linux-kernel target Philipp Rudo
2017-01-12 12:56 ` [RFC 0/7] Support for Linux kernel debugging Philipp Rudo
2017-01-12 13:02 ` [RFC 4/7] Add kernel module support for linux-kernel target Philipp Rudo
2017-01-25 18:10 ` [RFC 0/7] Support for Linux kernel debugging Peter Griffin
2017-01-26 13:12   ` Philipp Rudo
2017-02-03 17:45     ` Yao Qi
2017-02-03 19:46       ` Andreas Arnez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170207173944.GA32023@E107787-LIN \
    --to=qiyaoltc@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=prudo@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).