This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v11 2/5] Test adding and removing a symbol file at runtime.
- From: Doug Evans <dje at google dot com>
- To: Nicolas Blanc <nicolas dot blanc at intel dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>, Eli Zaretskii <eliz at gnu dot org>, lgustavo at codesourcery dot com
- Date: Mon, 15 Jul 2013 14:00:47 -0700
- Subject: Re: [PATCH v11 2/5] Test adding and removing a symbol file at runtime.
- References: <1372177111-26987-1-git-send-email-nicolas dot blanc at intel dot com> <1372177111-26987-3-git-send-email-nicolas dot blanc at intel dot com>
On Tue, Jun 25, 2013 at 9:18 AM, Nicolas Blanc <nicolas.blanc@intel.com> wrote:
> This test exercises the commands 'add-symbol-file'
> and 'remove-symbol-file'.
>
> 2013-04-04 Nicolas Blanc <nicolas.blanc@intel.com>
>
> gdb/testsuite
> * gdb.base/sym-file-lib.c: New file.
> * gdb.base/sym-file-main.c: New file.
> * gdb.base/sym-file.exp: New file.
Hi.
I would prefer to see the mini-dynamic-loader code broken out into its own file.
No point in hardcoding all that low level knowledge into a single-use
source file.
Further comments inline below.
>
> Signed-off-by: Nicolas Blanc <nicolas.blanc@intel.com>
> ---
> gdb/testsuite/gdb.base/sym-file-lib.c | 26 +++
> gdb/testsuite/gdb.base/sym-file-main.c | 374 ++++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/sym-file.exp | 143 ++++++++++++
> 3 files changed, 543 insertions(+), 0 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/sym-file-lib.c
> create mode 100644 gdb/testsuite/gdb.base/sym-file-main.c
> create mode 100644 gdb/testsuite/gdb.base/sym-file.exp
>
> diff --git a/gdb/testsuite/gdb.base/sym-file-lib.c b/gdb/testsuite/gdb.base/sym-file-lib.c
> new file mode 100644
> index 0000000..586215d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/sym-file-lib.c
> @@ -0,0 +1,26 @@
> +/* 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/>.
> +*/
> +
> +extern int
> +bar ()
> +{
> + return 1; /* gdb break at bar */
> +}
> +
> +extern int
> +foo (int a)
> +{
> + return a; /* gdb break at foo */
> +}
> diff --git a/gdb/testsuite/gdb.base/sym-file-main.c b/gdb/testsuite/gdb.base/sym-file-main.c
> new file mode 100644
> index 0000000..e43ff77
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/sym-file-main.c
> @@ -0,0 +1,374 @@
> +/* 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 <fcntl.h>
> +#include </usr/include/elf.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +
> +#ifdef TARGET_LP64
> +typedef Elf64_Phdr Elf_Phdr;
> +typedef Elf64_Ehdr Elf_Ehdr;
> +typedef Elf64_Shdr Elf_Shdr;
> +typedef Elf64_Sym Elf_Sym;
> +typedef Elf64_Word Elf_Word;
> +
If you're going to have a blank line here, then I think it makes
things more readable
to also have a blank line around the #ifdef/#elif/#endif.
[And I'm all for keeping the blank line here.]
> +unsigned char inline
No need for inline here.
> +elf_st_type (unsigned char st_info)
> +{
> + return ELF64_ST_TYPE (st_info);
> +}
> +#elif defined TARGET_ILP32
> +typedef Elf32_Phdr Elf_Phdr;
> +typedef Elf32_Ehdr Elf_Ehdr;
> +typedef Elf32_Shdr Elf_Shdr;
> +typedef Elf32_Sym Elf_Sym;
> +typedef Elf32_Word Elf_Word;
> +
> +unsigned char inline
Remove "inline".
> +elf_st_type (unsigned char st_info)
> +{
> + return ELF32_ST_TYPE (st_info);
> +}
> +#endif
> +
> +void
> +gdb_add_symbol_file (void *addr, const char *file)
> +{
> + return;
> +}
> +
> +void
> +gdb_remove_symbol_file (void *addr)
> +{
> + return;
> +}
> +
> +struct segment
> +{
> + char *mapped_addr;
> + Elf_Phdr *phdr;
> + struct segment *next;
> +};
> +
> +/* Load a program segment. */
> +
> +struct segment *
> +load (char *addr, Elf_Phdr *phdr, struct segment *tail_seg)
> +{
> + /* For the sake of simplicity all operations are permitted. */
> + unsigned perm = PROT_READ | PROT_WRITE | PROT_EXEC;
> +
> + char *mapped_addr = (char *) mmap ((void *) phdr->p_vaddr,
> + phdr->p_memsz, perm,
> + MAP_ANONYMOUS | MAP_PRIVATE,
> + -1, 0);
> +
> + void *from = (void *) (addr + phdr->p_offset);
> + void *to = (void *) mapped_addr;
> +
> + memcpy (to, from, phdr->p_filesz);
> +
> + struct segment *seg = (struct segment *) malloc (sizeof (struct segment));
Portability: Stick with old (ancient? :-)) style local variable definitions.
> +
> + if (seg == 0)
> + return 0;
> +
> + seg->mapped_addr = mapped_addr;
> + seg->phdr = phdr;
> + seg->next = 0;
> +
> + if (tail_seg != 0)
> + tail_seg->next = seg;
> +
> + return seg;
> +}
> +
> +/* Load a shared library without calling the standard loader. */
> +
> +int
> +load_shlib (const char *file, Elf_Ehdr **ehdr_out, struct segment **seg_out)
> +{
> + unsigned i = 0;
> +
> + /* Map the lib in memory for reading. */
> + int fd = open (file, O_RDONLY);
> + if (fd < 0)
> + {
> + perror ("fopen failed.");
> + return -1;
> + }
> +
> + off_t fsize = lseek (fd, 0, SEEK_END);
> +
> + if (fsize < 0)
> + {
> + perror ("lseek failed.");
> + return -1;
> + }
> +
> + char *addr = (char *) mmap (NULL, fsize, PROT_READ, MAP_PRIVATE, fd, 0);
> + if (addr == (char *) -1)
> + {
> + perror ("mmap failed.");
> + return -1;
> + }
> +
> + /* Check if the lib is an ELF file. */
> + Elf_Ehdr *ehdr = (Elf_Ehdr *) addr;
> + if (ehdr->e_ident[EI_MAG0] != ELFMAG0
> + || ehdr->e_ident[EI_MAG1] != ELFMAG1
> + || ehdr->e_ident[EI_MAG2] != ELFMAG2
> + || ehdr->e_ident[EI_MAG3] != ELFMAG3)
> + {
> + printf ("Not an ELF file: %x\n", ehdr->e_ident[EI_MAG0]);
> + return -1;
> + }
> +
> + if (ehdr->e_ident[EI_CLASS] == ELFCLASS32)
> + {
> + if (sizeof (int *) != 4)
> + {
> + printf ("Architecture mismatch.");
> + return -1;
> + }
> + }
> + else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64)
> + {
> + if (sizeof (int *) != 8)
> + {
> + printf ("Architecture mismatch.");
> + return -1;
> + }
> + }
> +
> + /* Load the program segments. For the sake of simplicity
> + assume that no reallocation is needed. */
> + Elf_Phdr *phdr = (Elf_Phdr *) (addr + ehdr->e_phoff);
> + struct segment *head_seg = 0;
> + struct segment *tail_seg = 0;
> + for (i = 0; i < ehdr->e_phnum; i++, phdr++)
> + {
> + if (phdr->p_type == PT_LOAD)
> + {
> + struct segment *next_seg = load (addr, phdr, tail_seg);
> + if (next_seg == 0)
> + continue;
> + tail_seg = next_seg;
> + if (head_seg == 0)
> + head_seg = next_seg;
> + }
> + }
> + *ehdr_out = ehdr;
> + *seg_out = head_seg;
> + return 0;
> +}
> +
> +/* Return the section-header table. */
> +
> +Elf_Shdr *
> +find_shdrtab (Elf_Ehdr *ehdr)
> +{
> + return (Elf_Shdr *) (((char *) ehdr) + ehdr->e_shoff);
> +}
> +
> +/* Return the string table of the section headers. */
> +
> +const char *
> +find_shstrtab (Elf_Ehdr *ehdr, unsigned *size)
> +{
> + const Elf_Shdr *shdr = find_shdrtab (ehdr);
> +
> + if (ehdr->e_shnum <= ehdr->e_shstrndx)
> + {
> + printf ("The index of the string table is corrupt.");
> + return NULL;
> + }
> + const Elf_Shdr *shstr = &shdr[ehdr->e_shstrndx];
> + *size = shstr->sh_size;
> + return ((const char *) ehdr) + shstr->sh_offset;
> +}
> +
> +/* Return the string table named SECTION. */
> +
> +const char *
> +find_strtab (Elf_Ehdr *ehdr,
> + const char *shstrtab, unsigned shstrtab_size,
> + const char *section, unsigned *strtab_size)
> +{
> + const Elf_Shdr *shdr = find_shdrtab (ehdr);
> + unsigned i = 0;
> + for (i = 0; i < ehdr->e_shnum; i++)
> + {
> + Elf_Word name = shdr[i].sh_name;
> + if (shdr[i].sh_type == SHT_STRTAB && name <= shstrtab_size
> + && strcmp ((const char *) &shstrtab[name], section) == 0)
> + {
> + *strtab_size = shdr[i].sh_size;
> + return ((const char *) ehdr) + shdr[i].sh_offset;
> + }
> +
> + }
> + return NULL;
> +}
> +
> +/* Return the section header named SECTION. */
> +
> +Elf_Shdr *
> +find_shdr (Elf_Ehdr *ehdr, const char *section,
> + const char *shstrtab, unsigned shstrtab_size)
> +{
> + Elf_Shdr *shdr = find_shdrtab (ehdr);
> + unsigned i = 0;
> + for (i = 0; i < ehdr->e_shnum; i++)
> + {
> + Elf_Word name = shdr[i].sh_name;
> + if (name <= shstrtab_size)
> + {
> + if (strcmp ((const char *) &shstrtab[name], section) == 0)
> + return &shdr[i];
> + }
> +
> + }
> + return NULL;
> +}
> +
> +/* Return the symbol table. */
> +
> +Elf_Sym *
> +find_symtab (Elf_Ehdr *ehdr, unsigned *symtab_size)
> +{
> + unsigned i = 0;
> + const Elf_Shdr *shdr = find_shdrtab (ehdr);
> + for (i = 0; i < ehdr->e_shnum; i++)
> + {
> + if (shdr[i].sh_type == SHT_SYMTAB)
> + {
> + *symtab_size = shdr[i].sh_size / sizeof (Elf_Sym);
> + return (Elf_Sym *) (((const char *) ehdr) + shdr[i].sh_offset);
> + }
> + }
> + return NULL;
> +}
> +
> +/* Lookup the offset of FUNC. */
> +
> +int
> +lookup_function (const char *func,
> + Elf_Sym *symtab, unsigned symtab_size,
> + const char *strtab, unsigned strtab_size, unsigned *offset)
> +{
> + unsigned i = 0;
> + for (i = 0; i < symtab_size; i++)
> + {
> + Elf_Sym *sym = &symtab[i];
> +
> + if (elf_st_type (sym->st_info) != STT_FUNC)
> + continue;
> +
> + if (sym->st_name < strtab_size)
> + {
> + const char *name = &strtab[sym->st_name];
> + if (strcmp (name, func) == 0)
> + {
> + *offset = (unsigned) sym->st_value;
> + return 0;
> + }
> + }
> + }
> +
> + return -1;
> +}
> +
> +/* Load a shared library without relying on the standard
> + loader to test GDB's commands for adding and removing
> + symbol files at runtime. */
> +
> +int
> +main (int argc, const char *argv[])
> +{
> + const char *file = SHLIB_NAME;
> +
> + Elf_Ehdr *ehdr = 0;
> + struct segment *head_seg = 0;
> +
> + if (load_shlib (file, &ehdr, &head_seg) != 0)
> + return -1;
> +
> + /* Get the string table for the section headers. */
> + unsigned shstrtab_size = 0;
> + const char *shstrtab = find_shstrtab (ehdr, &shstrtab_size);
> +
> + if (shstrtab == NULL)
> + return -1;
> +
> + /* Get the text section. */
> + Elf_Shdr *text = find_shdr (ehdr, ".text", shstrtab, shstrtab_size);
> + if (text == NULL)
> + return -1;
> +
> + char *base_addr = head_seg->mapped_addr + text->sh_offset;
> +
> + /* Notify GDB to add the symbol file. */
> + gdb_add_symbol_file (base_addr, file);
> +
> + /* Get the string table for the symbols. */
> + unsigned strtab_size = 0;
> + const char *strtab = find_strtab (ehdr, shstrtab, shstrtab_size,
> + ".strtab", &strtab_size);
> + if (strtab == NULL)
> + {
> + printf (".strtab not found.");
> + return -1;
> + }
> +
> + /* Get the symbol table. */
> + unsigned symtab_size = 0;
> + Elf_Sym *symtab = find_symtab (ehdr, &symtab_size);
> + if (symtab == NULL)
> + {
> + printf ("symtab not found.");
> + return -1;
> + }
> +
> + /* Call bar from SHLIB_NAME. */
> + unsigned bar_offset = 0;
> + if (lookup_function ("bar",
> + symtab, symtab_size,
> + strtab, strtab_size, &bar_offset) != 0)
> + return -1;
> +
> + int (*pbar) () = (int (*)()) (head_seg->mapped_addr + bar_offset);
> +
> + int value1 = (*pbar) ();
> +
> + /* Call foo from SHLIB_NAME. */
> + unsigned foo_offset = 0;
> + if (lookup_function ("foo",
> + symtab, symtab_size,
> + strtab, strtab_size, &foo_offset) != 0)
> + return -1;
> +
> + int (*pfoo) (int) = (int (*)(int)) (head_seg->mapped_addr + foo_offset);
> +
> + int value2 = (*pfoo) (2);
> +
> + /* Notify GDB to remove the symbol file. */
> + gdb_remove_symbol_file (base_addr);
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/sym-file.exp b/gdb/testsuite/gdb.base/sym-file.exp
> new file mode 100644
> index 0000000..873e488
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/sym-file.exp
> @@ -0,0 +1,143 @@
> +# 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/>. */
> +
> +# Test adding and removing a symbol file dynamically:
> +# 1) Run to gdb_add_symbol_file in $srcfile.
> +# 2) Set a pending breakpoint at bar in $libsrc.
> +# 3) Load $shlib_name using 'add-symbol-file'.
> +# 4) Continue to bar in $libsrc.
> +# 5) Set a breakpoint at foo in $librc.
> +# 6) Continue to foo in $libsrc.
> +# 7) Set a breakpoint at gdb_remove_symbol_file.
> +# 8) Continue to gdb_remove_symbol_file in $srcfile.
> +# 9) Remove $shlib_name using 'remove-symbol-file'.
> +# 10) Check that the breakpoints at foo and bar are pending.
> +# 11) Check that the execution can continue without error.
> +
While we could leave it to the compilation failing to signify this
isn't an elf target,
I think you should add a test for appropriate targets here.
gdb.base/dup-sect.exp has this:
# This test can only be run on targets which support ELF and use gas.
# For now pick a sampling of likely targets.
if {![istarget *-*-linux*]
&& ![istarget *-*-gnu*]
&& ![istarget *-*-elf*]
&& ![istarget arm*-*-eabi*]
&& ![istarget arm*-*-symbianelf*]
&& ![istarget powerpc-*-eabi*]} {
return 0
}
Something along those lines would be good.
[IWBN to add is_elf_target to lib/gdb.exp, but I'm not sure if that'll
be problematic.]
> +if [skip_shlib_tests] {
> + return 0
> +}
> +
> +if [is_remote target] {
> + return 0
> +}
> +
> +set target_size TARGET_UNKNOWN
> +if [is_lp64_target] {
> + set target_size TARGET_LP64
> +} elseif [is_ilp32_target] {
> + set target_size TARGET_ILP32
> +} else {
> + return 0
> +}
> +
> +set testfile sym-file.exp
> +set mainfile sym-file-main
> +set libfile sym-file-lib
> +set srcfile ${mainfile}.c
> +set libsrc "${srcdir}/${subdir}/${libfile}.c"
> +set libname "${libfile}.so"
> +set shlib_name "${objdir}/${subdir}/${libname}"
> +set libobj "${objdir}/${subdir}/${libname}"
> +set exec_opts [list debug "additional_flags=-D$target_size\
> + -DSHLIB_NAME\\=\"$shlib_name\""]
> +
> +if [get_compiler_info] {
> + return -1
> +}
> +
> +if { [gdb_compile_shlib $libsrc $libobj {debug}] != "" } {
> + untested ${testfile}
> + return
> +}
> +
> +if { [prepare_for_testing ${testfile} ${mainfile} ${srcfile} $exec_opts] } {
> + return
> +}
> +
> +# 1) Run to GDB_ADD_SYMBOl_FILE in $srcfile for adding
> +# $shlib_name.
> +set result [runto gdb_add_symbol_file]
> +if { !$result } then {
> + return
> +}
> +
> +# 2) Set a pending breakpoint at bar in $libsrc.
> +set result [gdb_breakpoint bar allow-pending]
> +if { !$result } then {
> + return
> +}
> +
> +# 3) Add $shlib_name using 'add-symbol-file'.
> +set result [gdb_test "add-symbol-file ${shlib_name} addr" \
> + "Reading symbols from .*${shlib_name}\\.\\.\\.done\\." \
> + "add-symbol-file ${shlib_name} addr" \
> + "add symbol table from file \".*${shlib_name}\"\
> + at.*\\(y or n\\) " \
> + "y"]
> +if { $result != 0 } then {
> + return
> +}
> +
> +# 4) Continue to bar in $libsrc to ensure that the breakpoint
> +# was bound correctly after adding $shilb_name.
> +set lnum_bar [gdb_get_line_number "break at bar" ${libfile}.c]
> +gdb_continue_to_breakpoint bar ".*$libfile\\.c:$lnum_bar.*"
> +
> +# 5) Set a breakpoint at foo in $libsrc.
> +set result [gdb_breakpoint foo]
> +if { !$result } then {
> + return
> +}
> +
> +# 6) Continue to foo in $libsrc to ensure that the breakpoint
> +# was bound correctly.
> +set lnum_foo [gdb_get_line_number "break at foo" ${libfile}.c]
> +gdb_continue_to_breakpoint foo ".*$libfile\\.c:$lnum_foo.*"
> +
> +# 7) Set a breakpoint at gdb_remove_symbol_file in $srcfile for
> +# removing $shlib_name.
> +set result [gdb_breakpoint gdb_remove_symbol_file]
> +if { !$result } then {
> + return
> +}
> +
> +# 8) Continue to gdb_remove_symbol_file in $srcfile.
> +gdb_continue_to_breakpoint gdb_remove_symbol_file
> +
> +# 9) Remove $shlib_name using 'remove-symbol-file'.
> +set result [gdb_test "remove-symbol-file -a addr" \
> + ""\
> + "remove-symbol-file -a addr" \
> + "Remove symbol table from file \".*${shlib_name}\"\\?\
> +.*\\(y or n\\) " \
> + "y"]
> +if { $result != 0 } then {
> + return
> +}
> +
> +# 10) Check that the breakpoints at foo and bar are pending after removing
> +# $shlib_name.
> +gdb_test "info breakpoints 2" \
> + ".*PENDING.*" \
> + "check if Breakpoint 2 is pending."
> +
> +gdb_test "info breakpoints 3" \
> + ".*PENDING.*" \
> + "check if Breakpoint 3 is pending."
> +
> +# 11) Check that the execution can continue without error.
> +gdb_continue_to_end
> +
> --
> 1.7.1
>