Re: [patch] validate binary before use

Jan Kratochvil
2013-01-30T19:17:19Z

On Tue, 29 Jan 2013 17:15:13 +0100, Aleksandar Ristovski wrote:
> Index: gdb/mips-linux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/mips-linux-tdep.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 mips-linux-tdep.c
> --- gdb/mips-linux-tdep.c 1 Jan 2013 06:32:47 -0000 1.97
> +++ gdb/mips-linux-tdep.c 29 Jan 2013 15:46:39 -0000
> @@ -1495,6 +1495,8 @@ mips_linux_init_abi (struct gdbarch_info
> mips_svr4_so_ops.in_dynsym_resolve_code
> = mips_linux_in_dynsym_resolve_code;
> }
> + if (mips_svr4_so_ops.validate == NULL)
> + mips_svr4_so_ops.validate = solib_validate;
> set_solib_ops (gdbarch, &mips_svr4_so_ops);
>
> set_gdbarch_write_pc (gdbarch, mips_linux_write_pc);
> Index: gdb/ppc-linux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 ppc-linux-tdep.c
> --- gdb/ppc-linux-tdep.c 1 Jan 2013 06:32:49 -0000 1.141
> +++ gdb/ppc-linux-tdep.c 29 Jan 2013 15:46:39 -0000
> @@ -1732,6 +1732,8 @@ ppc_linux_init_abi (struct gdbarch_info
> powerpc_so_ops.in_dynsym_resolve_code =
> powerpc_linux_in_dynsym_resolve_code;
> }
> + if (powerpc_so_ops.validate == NULL)
> + powerpc_so_ops.validate = solib_validate;
> set_solib_ops (gdbarch, &powerpc_so_ops);
>
> set_gdbarch_skip_solib_resolver (gdbarch, glibc_skip_solib_resolver);
> Index: gdb/solib-darwin.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-darwin.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 solib-darwin.c
> --- gdb/solib-darwin.c 1 Jan 2013 06:32:50 -0000 1.35
> +++ gdb/solib-darwin.c 29 Jan 2013 15:46:39 -0000
> @@ -647,4 +647,5 @@ _initialize_darwin_solib (void)
> darwin_so_ops.in_dynsym_resolve_code = darwin_in_dynsym_resolve_code;
> darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol;
> darwin_so_ops.bfd_open = darwin_bfd_open;
> + darwin_so_ops.validate = solib_validate;
> }
> Index: gdb/solib-dsbt.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-dsbt.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 solib-dsbt.c
> --- gdb/solib-dsbt.c 1 Jan 2013 06:32:50 -0000 1.9
> +++ gdb/solib-dsbt.c 29 Jan 2013 15:46:39 -0000
> @@ -1182,6 +1182,7 @@ _initialize_dsbt_solib (void)
> dsbt_so_ops.open_symbol_file_object = open_symbol_file_object;
> dsbt_so_ops.in_dynsym_resolve_code = dsbt_in_dynsym_resolve_code;
> dsbt_so_ops.bfd_open = solib_bfd_open;
> + dsbt_so_ops.validate = solib_validate;
>
> /* Debug this file's internals. */
> add_setshow_zuinteger_cmd ("solib-dsbt", class_maintenance,
> Index: gdb/solib-frv.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-frv.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 solib-frv.c
> --- gdb/solib-frv.c 1 Jan 2013 06:32:50 -0000 1.49
> +++ gdb/solib-frv.c 29 Jan 2013 15:46:39 -0000
> @@ -1182,6 +1182,7 @@ _initialize_frv_solib (void)
> frv_so_ops.open_symbol_file_object = open_symbol_file_object;
> frv_so_ops.in_dynsym_resolve_code = frv_in_dynsym_resolve_code;
> frv_so_ops.bfd_open = solib_bfd_open;
> + frv_so_ops.validate = solib_validate;
>
> /* Debug this file's internals. */
> add_setshow_zuinteger_cmd ("solib-frv", class_maintenance,
> Index: gdb/solib-ia64-hpux.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-ia64-hpux.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 solib-ia64-hpux.c
> --- gdb/solib-ia64-hpux.c 1 Jan 2013 06:32:50 -0000 1.6
> +++ gdb/solib-ia64-hpux.c 29 Jan 2013 15:46:39 -0000
> @@ -686,6 +686,7 @@ ia64_hpux_target_so_ops (void)
> ops->open_symbol_file_object = ia64_hpux_open_symbol_file_object;
> ops->in_dynsym_resolve_code = ia64_hpux_in_dynsym_resolve_code;
> ops->bfd_open = solib_bfd_open;
> + ops->validate = solib_validate;
>
> return ops;
> }
> Index: gdb/solib-irix.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-irix.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 solib-irix.c
> --- gdb/solib-irix.c 1 Jan 2013 06:32:50 -0000 1.46
> +++ gdb/solib-irix.c 29 Jan 2013 15:46:39 -0000
> @@ -652,4 +652,5 @@ _initialize_irix_solib (void)
> irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object;
> irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code;
> irix_so_ops.bfd_open = solib_bfd_open;
> + irix_so_ops.validate = solib_validate;
> }
> Index: gdb/solib-osf.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-osf.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 solib-osf.c
> --- gdb/solib-osf.c 1 Jan 2013 06:32:50 -0000 1.38
> +++ gdb/solib-osf.c 29 Jan 2013 15:46:39 -0000
> @@ -633,6 +633,7 @@ _initialize_osf_solib (void)
> osf_so_ops.open_symbol_file_object = osf_open_symbol_file_object;
> osf_so_ops.in_dynsym_resolve_code = osf_in_dynsym_resolve_code;
> osf_so_ops.bfd_open = solib_bfd_open;
> + osf_so_ops.validate = solib_validate;
>
> /* FIXME: Don't do this here. *_gdbarch_init() should set so_ops. */
> current_target_so_ops = &osf_so_ops;
> Index: gdb/solib-pa64.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-pa64.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 solib-pa64.c
> --- gdb/solib-pa64.c 1 Jan 2013 06:32:51 -0000 1.40
> +++ gdb/solib-pa64.c 29 Jan 2013 15:46:39 -0000
> @@ -623,6 +623,7 @@ _initialize_pa64_solib (void)
> pa64_so_ops.open_symbol_file_object = pa64_open_symbol_file_object;
> pa64_so_ops.in_dynsym_resolve_code = pa64_in_dynsym_resolve_code;
> pa64_so_ops.bfd_open = solib_bfd_open;
> + pa64_so_ops.validate = solib_validate;
>
> memset (&dld_cache, 0, sizeof (dld_cache));
> }
> Index: gdb/solib-som.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-som.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 solib-som.c
> --- gdb/solib-som.c 1 Jan 2013 06:32:51 -0000 1.39
> +++ gdb/solib-som.c 29 Jan 2013 15:46:39 -0000
> @@ -811,6 +811,7 @@ _initialize_som_solib (void)
> som_so_ops.open_symbol_file_object = som_open_symbol_file_object;
> som_so_ops.in_dynsym_resolve_code = som_in_dynsym_resolve_code;
> som_so_ops.bfd_open = solib_bfd_open;
> + som_so_ops.validate = solib_validate;
> }
>
> void
> Index: gdb/solib-spu.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-spu.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 solib-spu.c
> --- gdb/solib-spu.c 1 Jan 2013 06:32:51 -0000 1.24
> +++ gdb/solib-spu.c 29 Jan 2013 15:46:39 -0000
> @@ -519,6 +519,7 @@ set_spu_solib_ops (struct gdbarch *gdbar
> spu_so_ops.current_sos = spu_current_sos;
> spu_so_ops.bfd_open = spu_bfd_open;
> spu_so_ops.lookup_lib_global_symbol = spu_lookup_lib_symbol;
> + spu_so_ops.validate = solib_validate;
> }
>
> set_solib_ops (gdbarch, &spu_so_ops);
> Index: gdb/solib-sunos.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-sunos.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 solib-sunos.c
> --- gdb/solib-sunos.c 1 Jan 2013 06:32:51 -0000 1.51
> +++ gdb/solib-sunos.c 29 Jan 2013 15:46:39 -0000
> @@ -738,6 +738,7 @@ _initialize_sunos_solib (void)
> sunos_so_ops.open_symbol_file_object = open_symbol_file_object;
> sunos_so_ops.in_dynsym_resolve_code = sunos_in_dynsym_resolve_code;
> sunos_so_ops.bfd_open = solib_bfd_open;
> + sunos_so_ops.validate = solib_validate;
>
> /* FIXME: Don't do this here. *_gdbarch_init() should set so_ops. */
> current_target_so_ops = &sunos_so_ops;
> Index: gdb/solib-svr4.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-svr4.c,v
> retrieving revision 1.172
> diff -u -p -r1.172 solib-svr4.c
> --- gdb/solib-svr4.c 1 Jan 2013 06:32:51 -0000 1.172
> +++ gdb/solib-svr4.c 29 Jan 2013 15:46:39 -0000

OK, I see currently target_so_ops methods are always valid and there is no
common initializer.

> @@ -189,7 +189,7 @@ has_lm_dynamic_from_link_map (void)
> }
>
> static CORE_ADDR
> -lm_addr_check (struct so_list *so, bfd *abfd)
> +lm_addr_check (const struct so_list *so, bfd *abfd)
> {
> if (!so->lm_info->l_addr_p)
> {
> @@ -848,7 +848,6 @@ svr4_keep_data_in_core (CORE_ADDR vaddr,
> CORE_ADDR ldsomap;
> struct so_list *new;
> struct cleanup *old_chain;
> - struct link_map_offsets *lmo;
> CORE_ADDR name_lm;
>
> info = get_svr4_info ();
> @@ -862,7 +861,6 @@ svr4_keep_data_in_core (CORE_ADDR vaddr,
> if (!ldsomap)
> return 0;
>
> - lmo = svr4_fetch_link_map_offsets ();
> new = XZALLOC (struct so_list);
> old_chain = make_cleanup (xfree, new);
> new->lm_info = lm_info_read (ldsomap);

This part is OK as a "Code cleanup" but it should be checked in separately, it
is unrelated.

> @@ -873,6 +871,110 @@ svr4_keep_data_in_core (CORE_ADDR vaddr,
> return (name_lm >= vaddr && name_lm < vaddr + size);
> }
>
> +/* Find containing segment for the section and calculate section's
> + offset. */

Empty line after function comment.

> +static CORE_ADDR
> +svr4_unrelocated_vma (const bfd *const abfd, const asection *const asect)
> +{
> + const Elf_Internal_Ehdr *const ehdr = elf_tdata (abfd)->elf_header;
> + const Elf_Internal_Phdr *const phdr = elf_tdata (abfd)->phdr;
> + CORE_ADDR phdr_base = 0;
> + int phdr_base_p = 0;
> + int i;
> +
> + for (i = 0; i < ehdr->e_phnum; ++i)
> + {
> + if (phdr[i].p_type == PT_LOAD)
> + {
> + if (!phdr_base_p)
> + {
> + /* First PT_LOAD serves as the base. */
> + phdr_base = phdr[i].p_vaddr;
> + phdr_base_p = 1;
> + }
> + if (phdr[i].p_vaddr <= asect->vma
> + && (phdr[i].p_vaddr + phdr[i].p_memsz) > asect->vma)
> + {
> + if (phdr_base_p)
> + return (asect->vma - phdr_base);
> + else
> + return (asect->vma - phdr[i].p_vaddr);
> + }
> + }
> + }
> +
> + return 0;
> +}

This function is not needed, see below.

> +
> +/* If build-id exists, compare it with target in-memory contents.
> + Return 1 if they match, 0 if they don't.
> + If there was no build-id, return 1 (could not be verified). */
> +
> +static int
> +svr4_validate_build_id (const struct so_list *const so)
> +{
> + const asection *asect;
> + int i, res = 1;
> +
> + if (!bfd_check_format (so->abfd, bfd_object)
> + || bfd_get_flavour (so->abfd) != bfd_target_elf_flavour
> + || elf_tdata (so->abfd)->build_id == NULL)
> + return 1;
> +
> + for (asect = so->abfd->sections; asect != NULL; asect = asect->next)
> + {
> + if ((asect->flags & SEC_LOAD) == SEC_LOAD)
> + {
> + const char *const sectname = bfd_get_section_name (so->abfd, asect);
> + const bfd_size_type size = bfd_get_section_size (asect);
> +
> + if (size != 0 && strcmp (sectname, NOTE_GNU_BUILD_ID_NAME) == 0)

spaces->tab.

> + {
> + gdb_byte *const data = xmalloc (size);
> + /* Real load base, not the offset from a possibly rebased
> + object. */
> + const CORE_ADDR so_base_addr = so->lm_info->l_addr_inferior;
> + /* Zero based vma, after undoing link script non zero base
> + and/or prelinked rebasing. */
> + const CORE_ADDR sect_vma_offset
> + = svr4_unrelocated_vma (so->abfd, asect);
> +
> + /* Relocated or not, contents will be corectly loaded from
> + the file by bfd library. */
> +
> + bfd_get_section_contents ((bfd *) so->abfd,
> + (asection *)asect, data, 0,

Just do not make 'asect' so const, then you will not need that '(asection *)'
cast.

> + size);
> + /* Section vma is unrelocated. If SO_BASE_ADDR is zero, then
> + use ASECT->VMA as-is. If not, then use offset + base addr. */
> + res = target_verify_memory (data, (so_base_addr > 0)?

I do not see why to use target_verify_memory in this case.

target_verify_memory is there for large sections to compare only their 32-bit
checksum. But build-id is already only 20 bytes long, with the protocol
overhead the 4 vs. 20 bytes do not make a difference. And it needlessly
weakens the check, it also does some patching of target_verify_memory.
Just use target_read_memory and memcmp.

> + so_base_addr + sect_vma_offset
> + : asect->vma,
> + size);

so->abfd is not properly relocated (not sure why but it is so) but you can
iterate so->sections..so->sections_end which contains relocated ADDR (=target
VMA). Then you can drop the svr4_unrelocated_vma and other calculations
around.

> + xfree (data);
> +
> + if (res == -1 && info_verbose)
> + warning (_("Could not verify section %s\n"),
> + NOTE_GNU_BUILD_ID_NAME);

Here could be break;.

> + }
> + }
> + }
> +
> + return (res != 0);

Just:
return res != 0;

> +}
> +
> +/* Validate SO by checking whether opened file matches
> + in-memory object. */
> +
> +static int
> +svr4_validate (const struct so_list *const so)
> +{
> + gdb_assert (so != NULL);
> + gdb_assert (so->abfd != NULL);
> +
> + return svr4_validate_build_id (so);
> +}
> +
> /* Implement the "open_symbol_file_object" target_so_ops method.
>
> If no open symbol file, attempt to locate and open the main symbol

> @@ -1175,7 +1277,6 @@ svr4_read_so_list (CORE_ADDR lm, struct
>
> for (; lm != 0; prev_lm = lm, lm = next_lm)
> {
> - struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
> struct so_list *new;
> struct cleanup *old_chain;
> int errcode;

This is also unrelated "Code cleanup" to be checked in separately.

> @@ -2461,4 +2562,5 @@ _initialize_svr4_solib (void)
> svr4_so_ops.lookup_lib_global_symbol = elf_lookup_lib_symbol;
> svr4_so_ops.same = svr4_same;
> svr4_so_ops.keep_data_in_core = svr4_keep_data_in_core;
> + svr4_so_ops.validate = svr4_validate;
> }
> Index: gdb/solib-svr4.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-svr4.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 solib-svr4.h
> --- gdb/solib-svr4.h 1 Jan 2013 06:32:51 -0000 1.24
> +++ gdb/solib-svr4.h 29 Jan 2013 15:46:39 -0000
> @@ -84,4 +84,7 @@ extern struct link_map_offsets *svr4_lp6
> SVR4 run time loader. */
> int svr4_in_dynsym_resolve_code (CORE_ADDR pc);
>
> +
> +#define NOTE_GNU_BUILD_ID_NAME ".note.gnu.build-id"

It is more common to place #define somewhere at the top of the file, not at
the bottom or between function declarations.

> +
> #endif /* solib-svr4.h */
> Index: gdb/solib-target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib-target.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 solib-target.c
> --- gdb/solib-target.c 1 Jan 2013 06:32:51 -0000 1.26
> +++ gdb/solib-target.c 29 Jan 2013 15:46:39 -0000
> @@ -25,6 +25,7 @@
> #include "target.h"
> #include "vec.h"
> #include "solib-target.h"
> +#include "solib.h"
>
> #include "gdb_string.h"
>
> @@ -501,6 +502,7 @@ _initialize_solib_target (void)
> solib_target_so_ops.in_dynsym_resolve_code
> = solib_target_in_dynsym_resolve_code;
> solib_target_so_ops.bfd_open = solib_bfd_open;
> + solib_target_so_ops.validate = solib_validate;
>
> /* Set current_target_so_ops to solib_target_so_ops if not already
> set. */
> Index: gdb/solib.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib.c,v
> retrieving revision 1.169
> diff -u -p -r1.169 solib.c
> --- gdb/solib.c 1 Jan 2013 06:32:51 -0000 1.169
> +++ gdb/solib.c 29 Jan 2013 15:46:39 -0000
> @@ -495,6 +495,17 @@ solib_map_sections (struct so_list *so)
> }
> }
>
> + gdb_assert (ops->validate != NULL);
> +
> + if (!ops->validate (so))
> + {
> + warning (_("Shared object could not be validated and will be ignored: %s."),

Either without the trailing dot or the more common case:
_("Shared object '%s' could not be validated and will be ignored."),

> + so->abfd->filename);

There would be rather bfd_get_filename (so->abfd).

But GDB prints for shared libraries their so->so_name, it is also guaraneed to
be the same here.

> + gdb_bfd_unref (so->abfd);
> + so->abfd = NULL;
> + return 0;
> + }
> +
> /* Add the shared object's sections to the current set of file
> section tables. Do this immediately after mapping the object so
> that later nodes in the list can query this object, as is needed
> @@ -1448,6 +1459,14 @@ gdb_bfd_lookup_symbol (bfd *abfd,
> return symaddr;
> }
>
> +/* Default implementation does not perform any validation. */

You provide two similar function comments, one in solib.c and one in solib.h.
There is s risk they become out of sync in the future. As solib.h already
contains comments at its functions I would keep the solib.h as is and replace
this solib.c comment by:
/* See solib.h for the function comment. */

> +
> +int
> +solib_validate (const struct so_list *const so)
> +{
> + return 1; /* No validation. */

Formatting:
/* No validation. */
return 1;

> +}
> +
> extern initialize_file_ftype _initialize_solib; /* -Wmissing-prototypes */
>
> void
> Index: gdb/solib.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib.h,v
> retrieving revision 1.34
> diff -u -p -r1.34 solib.h
> --- gdb/solib.h 1 Jan 2013 06:32:51 -0000 1.34
> +++ gdb/solib.h 29 Jan 2013 15:46:39 -0000
> @@ -90,4 +90,8 @@ extern CORE_ADDR gdb_bfd_lookup_symbol_f
> void *),
> void *data);
>
> +/* Default validation always returns 1. */
> +
> +extern int solib_validate (const struct so_list *so);
> +
> #endif /* SOLIB_H */
> Index: gdb/solist.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/solist.h,v
> retrieving revision 1.38
> diff -u -p -r1.38 solist.h
> --- gdb/solist.h 1 Jan 2013 06:32:51 -0000 1.38
> +++ gdb/solist.h 29 Jan 2013 15:46:39 -0000
> @@ -148,6 +148,11 @@ struct target_so_ops
> core file (in particular, for readonly sections). */
> int (*keep_data_in_core) (CORE_ADDR vaddr,
> unsigned long size);
> +
> +

Single empty line only, not two.

> + /* Return 0 if SO does not match target SO it is supposed to
> + represent. Return 1 otherwise. */
> + int (*validate) (const struct so_list *so);
> };
>
> /* Free the memory associated with a (so_list *). */
> Index: gdb/target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.c,v
> retrieving revision 1.318
> diff -u -p -r1.318 target.c
> --- gdb/target.c 1 Jan 2013 06:32:52 -0000 1.318
> +++ gdb/target.c 29 Jan 2013 15:46:39 -0000
> @@ -4045,12 +4045,14 @@ int
> target_verify_memory (const gdb_byte *data, CORE_ADDR memaddr, ULONGEST size)
> {
> struct target_ops *t;
> + gdb_byte *buff;
> + int retval = -1;
>
> for (t = current_target.beneath; t != NULL; t = t->beneath)
> {
> if (t->to_verify_memory != NULL)
> {
> - int retval = t->to_verify_memory (t, data, memaddr, size);
> + retval = t->to_verify_memory (t, data, memaddr, size);
>
> if (targetdebug)
> fprintf_unfiltered (gdb_stdlog,
> @@ -4062,7 +4064,23 @@ target_verify_memory (const gdb_byte *da
> }
> }
>
> - tcomplain ();
> + if (targetdebug)
> + fprintf_unfiltered (gdb_stdlog,
> + "target_verify_memory (%s, %s) - use target_read_memory\n",
> + paddress (target_gdbarch (), memaddr),
> + pulongest (size));
> +
> + /* Default: use memory reads and compare raw memory */
> + buff = xmalloc (size);
> +
> + if (target_read_memory (memaddr, buff, size) == 0)
> + retval = (memcmp (buff, data, size) == 0);
> + else
> + retval = -1;
> +
> + xfree (buff);
> +
> + return retval;

As discussed at its call I do not find this change useful, to be dropped.

> }
>
> /* The documentation for this function is in its prototype declaration in
> Index: gdb/testsuite/gdb.base/solib-mismatch-lib.c
> ===================================================================
> RCS file: gdb/testsuite/gdb.base/solib-mismatch-lib.c
> diff -N gdb/testsuite/gdb.base/solib-mismatch-lib.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.base/solib-mismatch-lib.c 29 Jan 2013 15:46:40 -0000
> @@ -0,0 +1,29 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2013 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +
> +int _bar = 42;
> +
> +int bar(void)
> +{
> + return _bar + 21;
> +}
> +
> +int foo(void)
> +{
> + return _bar;
> +}
> Index: gdb/testsuite/gdb.base/solib-mismatch-libmod.c
> ===================================================================
> RCS file: gdb/testsuite/gdb.base/solib-mismatch-libmod.c
> diff -N gdb/testsuite/gdb.base/solib-mismatch-libmod.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.base/solib-mismatch-libmod.c 29 Jan 2013 15:46:40 -0000
> @@ -0,0 +1,29 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2013 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +
> +int _bar = 21;
> +
> +int bar(void)
> +{
> + return 42 - _bar;
> +}
> +
> +int foo(void)
> +{
> + return 24 + bar();
> +}
> Index: gdb/testsuite/gdb.base/solib-mismatch.c
> ===================================================================
> RCS file: gdb/testsuite/gdb.base/solib-mismatch.c
> diff -N gdb/testsuite/gdb.base/solib-mismatch.c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.base/solib-mismatch.c 29 Jan 2013 15:46:40 -0000
> @@ -0,0 +1,43 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2013 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +
> +#include <dlfcn.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <signal.h>
> +
> +#define lib "./solib-mismatch.so"
> +
> +
> +int main(int argc, char *argv[])
> +{
> + void *h = dlopen(lib, RTLD_NOW);
> + int (*foo)(void);
> +
> + if (h == NULL)
> + {
> + printf("ERROR - could not open lib %s\n", lib);
> + return 1;
> + }
> + foo = dlsym(h, "foo");
> + printf("foo says: %d\n", (*foo)());
> + raise(SIGSTOP); /* Let gdb attach */

No testcase should remain running indefinitely, there should be for example
sleep (60); otherwise there remain during various crashes stale processes on
the system.

But that should not be needed with process spawned from GDB.

> + dlclose(h);
> + return 0;
> +}
> +
> Index: gdb/testsuite/gdb.base/solib-mismatch.exp
> ===================================================================
> RCS file: gdb/testsuite/gdb.base/solib-mismatch.exp
> diff -N gdb/testsuite/gdb.base/solib-mismatch.exp
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.base/solib-mismatch.exp 29 Jan 2013 15:46:40 -0000
> @@ -0,0 +1,175 @@
> +# Copyright 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +# are we on a target board
> +set test "solib-mismatch"
> +set testfile "solib-mismatch"

Use the new macro standard_testfile instead.

> +
> +# Test overview:
> +# generate two shared objects. One that will be used by the process
> +# and another, modified, that will be found by gdb. Gdb should
> +# detect the mismatch and refuse to use mismatched shared object.
> +
> +# First version of the object, to be loaded by ld
> +set srclibfilerun ${testfile}-lib.c
> +# Modified version of the object to be loaded by gdb
> +# Code in -libmod.c is tuned so it gives a mismatch but
> +# leaves .dynamic at the same point.
> +set srclibfilegdb ${testfile}-libmod.c
> +
> +# So file name:
> +set binlibfilebase ${testfile}.so
> +
> +# Setup run directory (where program is run from)
> +# It contains executable and '-lib' version of the library.
> +set binlibfiledirrun ${objdir}/${subdir}/lib${testfile}

Please keep the same prefix of all the files as ${testfile}...
(not lib${testfile}...)

> +set binlibfilerun ${binlibfiledirrun}/${binlibfilebase}

Use the new macro standard_output_file.

> +
> +# Second solib version is in current directory, '-libmod' version.
> +set binlibfiledirgdb ${objdir}/${subdir}
> +set binlibfilegdb ${binlibfiledirgdb}/${binlibfilebase}

Use the new macro standard_output_file.

> +
> +# Executeable
> +set srcfile ${testfile}.c
> +set executable ${testfile}
> +set objfile ${objdir}/${subdir}/${executable}.o
> +set binfile ${objdir}/${subdir}/${executable}

These get set by standard_testfile (except for $executable).

> +
> +file mkdir "${binlibfiledirrun}"
> +
> +set exec_opts [list debug additional_flags=-ldl]
> +
> +if [istarget "*-*-nto-*"] {
> + set exec_opts {}
> +}
> +
> +# build the first test case
> +if { [get_compiler_info]
> + || [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfilerun}" "${binlibfilerun}" [list debug ldflags=-Wl,-soname,${binlibfilebase},--build-id]] != ""
> + || [gdb_gnu_strip_debug $binlibfilerun]
> + || [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfilegdb}" "${binlibfilegdb}" [list debug ldflags=-Wl,-soname,${binlibfilebase},--build-id]] != ""
> + || [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object {debug}] != ""
> + || [gdb_compile "${objfile}" "${binfile}" executable ${exec_opts}] != "" } {
> + untested ${testfile}.exp

${testfile}.exp does not tell anything, use for example:
untested "Compilation failed.

> + return -1
> +}
> +
> +# Start with a fresh gdb

Remove the comment, there is no command for it here.

> +
> +# Start the exe. It will raise SIGSTOP on itself.
> +send_user "Exec: $executable\r\n"

dejagnu uses: verbose -log "Exec: $executable"

send_user also prints messages on screen during runtest, it should not (at
least it is not done in GDB testsuite).

> +set curdir [eval pwd]

Normal
set curdir [pwd]
works for me.

> +set dummy [eval cd "${binlibfiledirrun}"]

Normal
cd ${binlibfiledirrun}
works for me.

> +set testpid [eval exec "../${executable}" &]

The primary problem I have with the testcase is that it needlessly uses
separately spawned process for attach/detach. This makes the testcase
complicated, racy with testing system under load (I do not see any sleep here,
${executable} may be for example still in shell spawning ${executable} when
GDb attaches to it) and also fragile for leftover running processes.

I find it possible to test the feature just with normally "start"-ed process
from GDB.

In general always use only "cd" command in GDB, not in dejagnu.

And then you can:
* start the inferior, stop before dlopen
* "cd" in GDB to the other directory (libsolib-mismatch/).
* next "dlopen".

Now the inferior will use different current directory than GDB, while both
will load "./solib-mismatch.so".

> +send_user "You can now attach to $testpid\r\n"

verbose -log

> +
> +set dummy [eval cd "${binlibfiledirgdb}"]

Again, to be simplified.

But this leaves dejagnu in non-standard directory, current directory should be
always switched only temporarily as otherwise one forgets to restore it in
this or that exit path.

> +
> +proc solib_matching_test { solibfile symsloaded } {
> + global gdb_prompt
> + global testpid
> + global test
> + global executable
> + global srcdir
> + global subdir
> +
> + gdb_exit
> + gdb_start
> + gdb_reinitialize_dir $srcdir/$subdir
> + gdb_load ${executable}

This block can be replaced by:
clean_restart ${executable}

> +
> + send_gdb "set auto-solib-add off\r\n"
> + gdb_expect {
> + -re "${gdb_prompt} $" {
> +# Nothing, just drain the buffer
> + }
> + }

send_gdb never needs to be used in general, it does not handle various corner
cases. In this case it is enough:
gdb_test_no_output "set auto-solib-add off"

The same applies to any send_gdb commands below.

> +
> +# Test unstripped, .dynamic matching so
> + send_gdb "attach ${testpid}\r\n"
> + gdb_expect {
> + -re "Attaching to program:.*${executable}, process ${testpid}.*${gdb_prompt} $" {
> +# Nothing
> + }
> + default {
> + untested "${test}: Could not attach to ${testpid}"
> + return -1
> + }
> + }
> +
> + gdb_test_multiple "sharedlibrary" $test {
> + -re ".*${gdb_prompt} $" {
> + pass "Validate library detects mismatch"
> + }
> + default {
> + fail "${test}: sharedlibrary failure"
> + }
> + }
> +
> + gdb_test "info sharedlibrary ${solibfile}" \
> + ".*From.*To.*Syms.*Read.*Shared.*\r\n0x\[0-9a-f\]+.*0x\[0-9a-f\]+.*${symsloaded}.*" \
> + "Symbols for ${solibfile} loaded: expected '${symsloaded}'"
> +
> + send_gdb "detach\r\n"
> + gdb_expect {
> + -re ".*Detaching from program.*${executable}.*${gdb_prompt} $" {
> +#Nothing, just drain the buffer
> + }
> + default {
> + untested "${test}: Could not detach from ${testpid}"
> + return -1
> + }
> + }
> + return 0
> +}
> +
> +proc teardown {} {
> + global testpid
> +
> + send_user "send SIGCONT to ${testpid}\r\n"
> + send_gdb "attach ${testpid}\r\n"
> + send_gdb "signal SIGCONT\r\n"
> + send_gdb "detach\r\n"
> + gdb_expect {
> + -re ".*" {
> +#Nothing, just drain the buffer
> + }
> + }
> +}

Hopefully no longer needed without attachments.

> +
> +# Test unstripped, .dynamic matching
> +send_user "test unstripped, .dynamic matching\r\n"
> +solib_matching_test "${binlibfilebase}" "No"
> +
> +# Test --only-keep-debug, .dynamic matching so
> +send_user "test --only-keep-debug\r\n"
> +set objcopy_program [transform objcopy]
> +set result [catch "exec $objcopy_program --only-keep-debug ${binlibfilegdb}"]
> +if {$result != 0} {
> + untested "${test} test --only-keep-debug"
> + set dummy [eval cd "${curdir}"]
> + return -1
> +}
> +solib_matching_test "${binlibfilebase}" "No"
> +
> +# Now test it does not mis-invalidate matching libraries
> +send_user "test matching libraries\r\n"
> +file copy -force "${binlibfilerun}" "${binlibfilegdb}"

Here is ${binlibfilegdb} and it is nowhere left. It makes troubleshooting
difficult, one has to modify the .exp file first to get the library for manual
testing. Keep some copy of it around.

> +solib_matching_test "${binlibfilebase}" "Yes"
> +
> +send_user "cleanup\r\n"
> +teardown
> +set dummy [eval cd "${curdir}"]
> +

Thanks,
Jan