Re: [PATCH 1/2] GDB test suite: Add helper for locating core files

[Kevin Buettner <kevinb at redhat dot com>]

On Mon, 18 Sep 2017 20:41:51 +0200
Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:

> The test suite contains a convenience proc `core_find' that basically
> performs two tasks:
> 
> 1. Run a given command (expecting it to dump core).
> 2. Locate the core dump generated by the command, and return it.
> 
> However, some test cases just need the second part, because they run the
> command in a different way.  Hence they implement their own logic for
> locating the core dump.
> 
> This change replaces the instances of core dump retrieval logic by
> invocations of a new separate proc.  And it renames `core_find' to
> `run_and_get_core', to reduce confusion.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* lib/gdb.exp (exec_in_dir): New helper proc.
> 	(core_find): Rename to...
> 	(run_and_get_core): ...this.  Remove the deletefiles argument.
> 	Split out the logic for locating the dump into...
> 	(find_core): ...this new proc.
> 	* gdb.arch/s390-multiarch.exp: Adjust invocations of core_find.
> 	* gdb.base/break-interp.exp: Likewise.
> 	* gdb.base/corefile.exp: Likewise.
> 	* gdb.threads/corethreads.exp: Likewise.
> 	* gdb.arch/s390-vregs.exp: Exploit find_core.
> 	* gdb.base/bigcore.exp: Likewise.

Hi Andreas,

I've looked your patches over and for the most part, I like them.
They better consolidate the logic for finding core files to
lib/gdb.exp.

However, one serious problem is that your rewrite of code in
lib/gdb.exp removes support for the handling of remote core files.

E.g. when I test your patch on my x86-64 linux box using the
following command...

    make check RUNTESTFLAGS="--target_board=native-gdbserver"

...I see 32 fewer passes than before and also one more known failure.

Here are the passes that no longer occur when using your patch:

    PASS: gdb.base/corefile.exp: args: -core=corefile.core
    PASS: gdb.base/corefile.exp: args: execfile -core=corefile.core
    PASS: gdb.base/corefile.exp: core-file command
    PASS: gdb.base/corefile.exp: print coremaker_data
    PASS: gdb.base/corefile.exp: print coremaker_bss
    PASS: gdb.base/corefile.exp: print coremaker_ro
    PASS: gdb.base/corefile.exp: print func2::coremaker_local
    PASS: gdb.base/corefile.exp: $_exitsignal prints SIGABRT (6)
    PASS: gdb.base/corefile.exp: $_exitcode is void
    PASS: gdb.base/corefile.exp: backtrace in corefile.exp
    PASS: gdb.base/corefile.exp: up in corefile.exp
    PASS: gdb.base/corefile.exp: accessing original mmap data in core file
    PASS: gdb.base/corefile.exp: accessing mmapped data in core file
    PASS: gdb.base/corefile.exp: up in corefile.exp (reinit)
    PASS: gdb.base/corefile.exp: core
    PASS: gdb.base/corefile.exp: run: load core again
    PASS: gdb.base/corefile.exp: run: sanity check we see the core file
    PASS: gdb.base/corefile.exp: run: with core
    PASS: gdb.base/corefile.exp: run: core file is cleared
    PASS: gdb.base/corefile.exp: quit with a process
    PASS: gdb.base/corefile.exp: quit with processes: n
    PASS: gdb.base/corefile.exp: no question: load core
    PASS: gdb.base/corefile.exp: quit with a core file
    PASS: gdb.base/corefile.exp: core-file warning-free
    PASS: gdb.threads/corethreads.exp: load core
    PASS: gdb.threads/corethreads.exp: sanity check we see the core file
    PASS: gdb.threads/corethreads.exp: print pthread_t of thread0
    PASS: gdb.threads/corethreads.exp: print pthread_t of thread1
    PASS: gdb.threads/corethreads.exp: thread0 found
    PASS: gdb.threads/corethreads.exp: thread1 found
    PASS: gdb.threads/corethreads.exp: no other thread found

Instead, several warnings are now printed instead:

    WARNING: Can not generate core dump on remote target.

If you can restore support for handling of remote core files, I'd very
much like to review this patch again.

Thanks,

Kevin