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

Yao Qi
2017-02-07T17:39:56Z

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 (é½å°§)