Re: [RFC 3/7] Add basic Linux kernel support

[Yao Qi <qiyaoltc at gmail dot com>]

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 (齐尧)