* [PATCH 0/2] Improved variable object invalidation in GDB
@ 2014-06-27 10:14 Taimoor Mirza
2014-06-27 10:14 ` [PATCH 2/2] Testsuite for varobj updation after symbol removal Taimoor Mirza
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Taimoor Mirza @ 2014-06-27 10:14 UTC (permalink / raw)
To: gdb-patches; +Cc: Taimoor Mirza
Hi,
This patch series improves variable object invalidation in GDB.
This patch has been regression tested on ppc-gnu-linux
and i686 targets using both simulator and real boards.
Taimoor Mirza (2):
Fix varobj updation after symbol removal
Testsuite for varobj updation after symbol removal
gdb/objfiles.c | 19 ++
gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++
gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++
gdb/testsuite/gdb.mi/sym-file-loader.c | 353 ++++++++++++++++++++++++++++
gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++
gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++
gdb/varobj.c | 37 +++
gdb/varobj.h | 3 +
8 files changed, 688 insertions(+)
create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c
create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c
create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h
create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c
--
1.7.9.5
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 2/2] Testsuite for varobj updation after symbol removal 2014-06-27 10:14 [PATCH 0/2] Improved variable object invalidation in GDB Taimoor Mirza @ 2014-06-27 10:14 ` Taimoor Mirza 2014-07-13 16:11 ` Taimoor 2014-08-05 11:01 ` [PING][PATCH " Taimoor 2014-06-27 10:14 ` [PATCH 1/2] Fix " Taimoor Mirza ` (2 subsequent siblings) 3 siblings, 2 replies; 14+ messages in thread From: Taimoor Mirza @ 2014-06-27 10:14 UTC (permalink / raw) To: gdb-patches; +Cc: Taimoor Mirza This patch provides testcases for variable object updation after removing symbols. Test programs are same as used for testing remove-symbol-file command in gdb.base. sym-file-main.c is modified to just add a global variable for which varible object is created in testsuite. Testing: ======= This is tested for x86 target and also on ppc-linux-gnu and mips-gnu-linux using both simulator and real boards. 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> * gdb.mi/mi-var-invalidate.exp: Add tests to check global variable object change list is correct when its value is updated before removing symbols. * gdb.mi/sym-file-loader.c: New file. * gdb.mi/sym-file-loader.h: New file. * gdb.mi/sym-file-main.c: New file. * gdb.mi/sym-file-lib.c: New file. --- gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ gdb/testsuite/gdb.mi/sym-file-loader.c | 353 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ 5 files changed, 629 insertions(+) create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp index e6ba392..e3bf953 100644 --- a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp @@ -121,5 +121,72 @@ mi_gdb_test "-var-info-type global_simple" \ "\\^done,type=\"\"" \ "no type for invalid variable global_simple" +# Test varobj updation after removing symbols. + +if {[skip_shlib_tests]} { + 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 main_basename sym-file-main +set loader_basename sym-file-loader +set lib_basename sym-file-lib + +standard_testfile $main_basename.c $loader_basename.c $lib_basename.c + +set libsrc "${srcdir}/${subdir}/${srcfile3}" +set test_bin_name "sym-test-file" +set test_bin [standard_output_file ${test_bin_name}] +set shlib_name [standard_output_file ${lib_basename}.so] +set exec_opts [list debug "additional_flags= -I$srcdir/../../include/ \ +-D$target_size -DSHLIB_NAME\\=\"$shlib_name\""] + +if [get_compiler_info] { + return -1 +} + +if {[gdb_compile_shlib $libsrc $shlib_name {debug}] != ""} { + untested ${testfile} + return -1 +} + +if {[build_executable $testfile $test_bin "$srcfile $srcfile2" $exec_opts]} { + return -1 +} + +mi_delete_breakpoints +mi_gdb_load ${test_bin} + +# Create varobj for count variable. +mi_create_varobj var_count count "Create global varobj for count" + +# Run to GDB_ADD_SYMBOl_FILE in $srcfile for adding +# $shlib_name. +mi_runto gdb_add_symbol_file + +# Add $shlib_name using 'add-symbol-file'. +mi_gdb_test "-interpreter-exec console \"add-symbol-file ${shlib_name} addr\"" \ + "~\"add symbol table from file .*so.*at.*= $hex.*\\^done" \ + "add-symbol-file ${shlib_name}" + +# Continue to gdb_remove_symbol_file in $srcfile. +mi_runto gdb_remove_symbol_file + +# Remove $shlib_name using 'remove-symbol-file'. +mi_gdb_test "-interpreter-exec console \"remove-symbol-file -a addr\"" \ + ".*\\^done"\ + "remove-symbol-file test" + +# Check var_count varobj changelist is not empty. +mi_varobj_update var_count {var_count} "Update var_count" + mi_gdb_exit return 0 diff --git a/gdb/testsuite/gdb.mi/sym-file-lib.c b/gdb/testsuite/gdb.mi/sym-file-lib.c new file mode 100644 index 0000000..dae5188 --- /dev/null +++ b/gdb/testsuite/gdb.mi/sym-file-lib.c @@ -0,0 +1,26 @@ +/* Copyright 2013-2014 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.mi/sym-file-loader.c b/gdb/testsuite/gdb.mi/sym-file-loader.c new file mode 100644 index 0000000..c72a1d1 --- /dev/null +++ b/gdb/testsuite/gdb.mi/sym-file-loader.c @@ -0,0 +1,353 @@ +/* Copyright 2013-2014 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 <unistd.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/mman.h> + +#include "sym-file-loader.h" + +#ifdef TARGET_LP64 + +uint8_t +elf_st_type (uint8_t st_info) +{ + return ELF64_ST_TYPE (st_info); +} + +#elif defined TARGET_ILP32 + +uint8_t +elf_st_type (uint8_t st_info) +{ + return ELF32_ST_TYPE (st_info); +} + +#endif + +/* Load a program segment. */ + +static struct segment * +load (uint8_t *addr, Elf_External_Phdr *phdr, struct segment *tail_seg) +{ + struct segment *seg = NULL; + uint8_t *mapped_addr = NULL; + void *from = NULL; + void *to = NULL; + + /* For the sake of simplicity all operations are permitted. */ + unsigned perm = PROT_READ | PROT_WRITE | PROT_EXEC; + + mapped_addr = (uint8_t *) mmap ((void *) GETADDR (phdr, p_vaddr), + GET (phdr, p_memsz), perm, + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + + from = (void *) (addr + GET (phdr, p_offset)); + to = (void *) mapped_addr; + + memcpy (to, from, GET (phdr, p_filesz)); + + seg = (struct segment *) malloc (sizeof (struct segment)); + + 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; +} + +/* Mini shared library loader. No reallocation + is performed for the sake of simplicity. */ + +int +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, + struct segment **seg_out) +{ + uint64_t i; + int fd; + off_t fsize; + uint8_t *addr; + Elf_External_Ehdr *ehdr; + Elf_External_Phdr *phdr; + struct segment *head_seg = NULL; + struct segment *tail_seg = NULL; + + /* Map the lib in memory for reading. */ + fd = open (file, O_RDONLY); + if (fd < 0) + { + perror ("fopen failed."); + return -1; + } + + fsize = lseek (fd, 0, SEEK_END); + + if (fsize < 0) + { + perror ("lseek failed."); + return -1; + } + + addr = (uint8_t *) mmap (NULL, fsize, PROT_READ, MAP_PRIVATE, fd, 0); + if (addr == (uint8_t *) -1) + { + perror ("mmap failed."); + return -1; + } + + /* Check if the lib is an ELF file. */ + ehdr = (Elf_External_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 (void *) != 4) + { + printf ("Architecture mismatch."); + return -1; + } + } + else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) + { + if (sizeof (void *) != 8) + { + printf ("Architecture mismatch."); + return -1; + } + } + + /* Load the program segments. For the sake of simplicity + assume that no reallocation is needed. */ + phdr = (Elf_External_Phdr *) (addr + GET (ehdr, e_phoff)); + for (i = 0; i < GET (ehdr, e_phnum); i++, phdr++) + { + if (GET (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_External_Shdr * +find_shdrtab (Elf_External_Ehdr *ehdr) +{ + return (Elf_External_Shdr *) (((uint8_t *) ehdr) + GET (ehdr, e_shoff)); +} + +/* Return the string table of the section headers. */ + +const char * +find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size) +{ + const Elf_External_Shdr *shdr; + const Elf_External_Shdr *shstr; + + if (GET (ehdr, e_shnum) <= GET (ehdr, e_shstrndx)) + { + printf ("The index of the string table is corrupt."); + return NULL; + } + + shdr = find_shdrtab (ehdr); + + shstr = &shdr[GET (ehdr, e_shstrndx)]; + *size = GET (shstr, sh_size); + return ((const char *) ehdr) + GET (shstr, sh_offset); +} + +/* Return the string table named SECTION. */ + +const char * +find_strtab (Elf_External_Ehdr *ehdr, + const char *section, uint64_t *strtab_size) +{ + uint64_t shstrtab_size = 0; + const char *shstrtab; + uint64_t i; + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); + + /* Get the string table of the section headers. */ + shstrtab = find_shstrtab (ehdr, &shstrtab_size); + if (shstrtab == NULL) + return NULL; + + for (i = 0; i < GET (ehdr, e_shnum); i++) + { + uint64_t name = GET (shdr + i, sh_name); + if (GET (shdr + i, sh_type) == SHT_STRTAB && name <= shstrtab_size + && strcmp ((const char *) &shstrtab[name], section) == 0) + { + *strtab_size = GET (shdr + i, sh_size); + return ((const char *) ehdr) + GET (shdr + i, sh_offset); + } + + } + return NULL; +} + +/* Return the section header named SECTION. */ + +Elf_External_Shdr * +find_shdr (Elf_External_Ehdr *ehdr, const char *section) +{ + uint64_t shstrtab_size = 0; + const char *shstrtab; + uint64_t i; + + /* Get the string table of the section headers. */ + shstrtab = find_shstrtab (ehdr, &shstrtab_size); + if (shstrtab == NULL) + return NULL; + + Elf_External_Shdr *shdr = find_shdrtab (ehdr); + for (i = 0; i < GET (ehdr, e_shnum); i++) + { + uint64_t name = GET (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_External_Sym * +find_symtab (Elf_External_Ehdr *ehdr, uint64_t *symtab_size) +{ + uint64_t i; + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); + + for (i = 0; i < GET (ehdr, e_shnum); i++) + { + if (GET (shdr + i, sh_type) == SHT_SYMTAB) + { + *symtab_size = GET (shdr + i, sh_size) / sizeof (Elf_External_Sym); + return (Elf_External_Sym *) (((const char *) ehdr) + + GET (shdr + i, sh_offset)); + } + } + return NULL; +} + +/* Translate a file offset to an address in a loaded segment. */ + +int +translate_offset (uint64_t file_offset, struct segment *seg, void **addr) +{ + while (seg) + { + uint64_t p_from, p_to; + + Elf_External_Phdr *phdr = seg->phdr; + + if (phdr == NULL) + { + seg = seg->next; + continue; + } + + p_from = GET (phdr, p_offset); + p_to = p_from + GET (phdr, p_filesz); + + if (p_from <= file_offset && file_offset < p_to) + { + *addr = (void *) (seg->mapped_addr + (file_offset - p_from)); + return 0; + } + seg = seg->next; + } + + return -1; +} + +/* Lookup the address of FUNC. */ + +int +lookup_function (const char *func, + Elf_External_Ehdr *ehdr, struct segment *seg, void **addr) +{ + const char *strtab; + uint64_t strtab_size = 0; + Elf_External_Sym *symtab; + uint64_t symtab_size = 0; + uint64_t i; + + /* Get the string table for the symbols. */ + strtab = find_strtab (ehdr, ".strtab", &strtab_size); + if (strtab == NULL) + { + printf (".strtab not found."); + return -1; + } + + /* Get the symbol table. */ + symtab = find_symtab (ehdr, &symtab_size); + if (symtab == NULL) + { + printf ("symbol table not found."); + return -1; + } + + for (i = 0; i < symtab_size; i++) + { + Elf_External_Sym *sym = &symtab[i]; + + if (elf_st_type (GET (sym, st_info)) != STT_FUNC) + continue; + + if (GET (sym, st_name) < strtab_size) + { + const char *name = &strtab[GET (sym, st_name)]; + if (strcmp (name, func) == 0) + { + + uint64_t offset = GET (sym, st_value); + return translate_offset (offset, seg, addr); + } + } + } + + return -1; +} diff --git a/gdb/testsuite/gdb.mi/sym-file-loader.h b/gdb/testsuite/gdb.mi/sym-file-loader.h new file mode 100644 index 0000000..03dc7e1 --- /dev/null +++ b/gdb/testsuite/gdb.mi/sym-file-loader.h @@ -0,0 +1,99 @@ +/* Copyright 2013-2014 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/>. +*/ + +#ifndef __SYM_FILE_LOADER__ +#define __SYM_FILE_LOADER__ + +#include <inttypes.h> +#include <ansidecl.h> +#include <elf/common.h> +#include <elf/external.h> + +#ifdef TARGET_LP64 + +typedef Elf64_External_Phdr Elf_External_Phdr; +typedef Elf64_External_Ehdr Elf_External_Ehdr; +typedef Elf64_External_Shdr Elf_External_Shdr; +typedef Elf64_External_Sym Elf_External_Sym; +typedef uint64_t Elf_Addr; + +#elif defined TARGET_ILP32 + +typedef Elf32_External_Phdr Elf_External_Phdr; +typedef Elf32_External_Ehdr Elf_External_Ehdr; +typedef Elf32_External_Shdr Elf_External_Shdr; +typedef Elf32_External_Sym Elf_External_Sym; +typedef uint32_t Elf_Addr; + +#endif + +#define GET(hdr, field) (\ +sizeof ((hdr)->field) == 1 ? (uint64_t) (hdr)->field[0] : \ +sizeof ((hdr)->field) == 2 ? (uint64_t) *(uint16_t *) (hdr)->field : \ +sizeof ((hdr)->field) == 4 ? (uint64_t) *(uint32_t *) (hdr)->field : \ +sizeof ((hdr)->field) == 8 ? *(uint64_t *) (hdr)->field : \ +*(uint64_t *) NULL) + +#define GETADDR(hdr, field) (\ +sizeof ((hdr)->field) == sizeof (Elf_Addr) ? *(Elf_Addr *) (hdr)->field : \ +*(Elf_Addr *) NULL) + +struct segment +{ + uint8_t *mapped_addr; + Elf_External_Phdr *phdr; + struct segment *next; +}; + +/* Mini shared library loader. No reallocation is performed + for the sake of simplicity. */ + +int +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, + struct segment **seg_out); + +/* Return the section-header table. */ + +Elf_External_Shdr *find_shdrtab (Elf_External_Ehdr *ehdr); + +/* Return the string table of the section headers. */ + +const char *find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size); + +/* Return the string table named SECTION. */ + +const char *find_strtab (Elf_External_Ehdr *ehdr, + const char *section, uint64_t *strtab_size); + +/* Return the section header named SECTION. */ + +Elf_External_Shdr *find_shdr (Elf_External_Ehdr *ehdr, const char *section); + +/* Return the symbol table. */ + +Elf_External_Sym *find_symtab (Elf_External_Ehdr *ehdr, + uint64_t *symtab_size); + +/* Translate a file offset to an address in a loaded segment. */ + +int translate_offset (uint64_t file_offset, struct segment *seg, void **addr); + +/* Lookup the address of FUNC. */ + +int +lookup_function (const char *func, Elf_External_Ehdr* ehdr, + struct segment *seg, void **addr); + +#endif diff --git a/gdb/testsuite/gdb.mi/sym-file-main.c b/gdb/testsuite/gdb.mi/sym-file-main.c new file mode 100644 index 0000000..8260824 --- /dev/null +++ b/gdb/testsuite/gdb.mi/sym-file-main.c @@ -0,0 +1,84 @@ +/* Copyright 2013-2014 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 <stdlib.h> +#include <stdio.h> + +#include "sym-file-loader.h" + +// Global variable +int count = 0; + +void +gdb_add_symbol_file (void *addr, const char *file) +{ + return; +} + +void +gdb_remove_symbol_file (void *addr) +{ + return; +} + +/* 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_External_Ehdr *ehdr = NULL; + struct segment *head_seg = NULL; + Elf_External_Shdr *text; + char *text_addr = NULL; + int (*pbar) () = NULL; + int (*pfoo) (int) = NULL; + + if (load_shlib (file, &ehdr, &head_seg) != 0) + return -1; + + /* Get the text section. */ + text = find_shdr (ehdr, ".text"); + if (text == NULL) + return -1; + + /* Notify GDB to add the symbol file. */ + if (translate_offset (GET (text, sh_offset), head_seg, (void **) &text_addr) + != 0) + return -1; + + gdb_add_symbol_file (text_addr, file); + + /* Call bar from SHLIB_NAME. */ + if (lookup_function ("bar", ehdr, head_seg, (void *) &pbar) != 0) + return -1; + + (*pbar) (); + + /* Call foo from SHLIB_NAME. */ + if (lookup_function ("foo", ehdr, head_seg, (void *) &pfoo) != 0) + return -1; + + (*pfoo) (2); + + count++; + + /* Notify GDB to remove the symbol file. */ + gdb_remove_symbol_file (text_addr); + + return 0; +} -- 1.7.9.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Testsuite for varobj updation after symbol removal 2014-06-27 10:14 ` [PATCH 2/2] Testsuite for varobj updation after symbol removal Taimoor Mirza @ 2014-07-13 16:11 ` Taimoor 2014-08-05 11:01 ` [PING][PATCH " Taimoor 1 sibling, 0 replies; 14+ messages in thread From: Taimoor @ 2014-07-13 16:11 UTC (permalink / raw) To: gdb-patches ping. -Taimoor On 06/27/2014 03:13 PM, Taimoor Mirza wrote: > This patch provides testcases for variable object updation after removing > symbols. Test programs are same as used for testing remove-symbol-file > command in gdb.base. sym-file-main.c is modified to just add a global > variable for which varible object is created in testsuite. > > Testing: > ======= > This is tested for x86 target and also on ppc-linux-gnu and mips-gnu-linux > using both simulator and real boards. > > 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> > > * gdb.mi/mi-var-invalidate.exp: Add tests to check global > variable object change list is correct when its value is > updated before removing symbols. > * gdb.mi/sym-file-loader.c: New file. > * gdb.mi/sym-file-loader.h: New file. > * gdb.mi/sym-file-main.c: New file. > * gdb.mi/sym-file-lib.c: New file. > > --- > gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ > gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ > gdb/testsuite/gdb.mi/sym-file-loader.c | 353 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ > gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ > 5 files changed, 629 insertions(+) > create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h > create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c > > diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > index e6ba392..e3bf953 100644 > --- a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > @@ -121,5 +121,72 @@ mi_gdb_test "-var-info-type global_simple" \ > "\\^done,type=\"\"" \ > "no type for invalid variable global_simple" > > +# Test varobj updation after removing symbols. > + > +if {[skip_shlib_tests]} { > + 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 main_basename sym-file-main > +set loader_basename sym-file-loader > +set lib_basename sym-file-lib > + > +standard_testfile $main_basename.c $loader_basename.c $lib_basename.c > + > +set libsrc "${srcdir}/${subdir}/${srcfile3}" > +set test_bin_name "sym-test-file" > +set test_bin [standard_output_file ${test_bin_name}] > +set shlib_name [standard_output_file ${lib_basename}.so] > +set exec_opts [list debug "additional_flags= -I$srcdir/../../include/ \ > +-D$target_size -DSHLIB_NAME\\=\"$shlib_name\""] > + > +if [get_compiler_info] { > + return -1 > +} > + > +if {[gdb_compile_shlib $libsrc $shlib_name {debug}] != ""} { > + untested ${testfile} > + return -1 > +} > + > +if {[build_executable $testfile $test_bin "$srcfile $srcfile2" $exec_opts]} { > + return -1 > +} > + > +mi_delete_breakpoints > +mi_gdb_load ${test_bin} > + > +# Create varobj for count variable. > +mi_create_varobj var_count count "Create global varobj for count" > + > +# Run to GDB_ADD_SYMBOl_FILE in $srcfile for adding > +# $shlib_name. > +mi_runto gdb_add_symbol_file > + > +# Add $shlib_name using 'add-symbol-file'. > +mi_gdb_test "-interpreter-exec console \"add-symbol-file ${shlib_name} addr\"" \ > + "~\"add symbol table from file .*so.*at.*= $hex.*\\^done" \ > + "add-symbol-file ${shlib_name}" > + > +# Continue to gdb_remove_symbol_file in $srcfile. > +mi_runto gdb_remove_symbol_file > + > +# Remove $shlib_name using 'remove-symbol-file'. > +mi_gdb_test "-interpreter-exec console \"remove-symbol-file -a addr\"" \ > + ".*\\^done"\ > + "remove-symbol-file test" > + > +# Check var_count varobj changelist is not empty. > +mi_varobj_update var_count {var_count} "Update var_count" > + > mi_gdb_exit > return 0 > diff --git a/gdb/testsuite/gdb.mi/sym-file-lib.c b/gdb/testsuite/gdb.mi/sym-file-lib.c > new file mode 100644 > index 0000000..dae5188 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-lib.c > @@ -0,0 +1,26 @@ > +/* Copyright 2013-2014 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.mi/sym-file-loader.c b/gdb/testsuite/gdb.mi/sym-file-loader.c > new file mode 100644 > index 0000000..c72a1d1 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-loader.c > @@ -0,0 +1,353 @@ > +/* Copyright 2013-2014 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 <unistd.h> > +#include <fcntl.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/mman.h> > + > +#include "sym-file-loader.h" > + > +#ifdef TARGET_LP64 > + > +uint8_t > +elf_st_type (uint8_t st_info) > +{ > + return ELF64_ST_TYPE (st_info); > +} > + > +#elif defined TARGET_ILP32 > + > +uint8_t > +elf_st_type (uint8_t st_info) > +{ > + return ELF32_ST_TYPE (st_info); > +} > + > +#endif > + > +/* Load a program segment. */ > + > +static struct segment * > +load (uint8_t *addr, Elf_External_Phdr *phdr, struct segment *tail_seg) > +{ > + struct segment *seg = NULL; > + uint8_t *mapped_addr = NULL; > + void *from = NULL; > + void *to = NULL; > + > + /* For the sake of simplicity all operations are permitted. */ > + unsigned perm = PROT_READ | PROT_WRITE | PROT_EXEC; > + > + mapped_addr = (uint8_t *) mmap ((void *) GETADDR (phdr, p_vaddr), > + GET (phdr, p_memsz), perm, > + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); > + > + from = (void *) (addr + GET (phdr, p_offset)); > + to = (void *) mapped_addr; > + > + memcpy (to, from, GET (phdr, p_filesz)); > + > + seg = (struct segment *) malloc (sizeof (struct segment)); > + > + 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; > +} > + > +/* Mini shared library loader. No reallocation > + is performed for the sake of simplicity. */ > + > +int > +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, > + struct segment **seg_out) > +{ > + uint64_t i; > + int fd; > + off_t fsize; > + uint8_t *addr; > + Elf_External_Ehdr *ehdr; > + Elf_External_Phdr *phdr; > + struct segment *head_seg = NULL; > + struct segment *tail_seg = NULL; > + > + /* Map the lib in memory for reading. */ > + fd = open (file, O_RDONLY); > + if (fd < 0) > + { > + perror ("fopen failed."); > + return -1; > + } > + > + fsize = lseek (fd, 0, SEEK_END); > + > + if (fsize < 0) > + { > + perror ("lseek failed."); > + return -1; > + } > + > + addr = (uint8_t *) mmap (NULL, fsize, PROT_READ, MAP_PRIVATE, fd, 0); > + if (addr == (uint8_t *) -1) > + { > + perror ("mmap failed."); > + return -1; > + } > + > + /* Check if the lib is an ELF file. */ > + ehdr = (Elf_External_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 (void *) != 4) > + { > + printf ("Architecture mismatch."); > + return -1; > + } > + } > + else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) > + { > + if (sizeof (void *) != 8) > + { > + printf ("Architecture mismatch."); > + return -1; > + } > + } > + > + /* Load the program segments. For the sake of simplicity > + assume that no reallocation is needed. */ > + phdr = (Elf_External_Phdr *) (addr + GET (ehdr, e_phoff)); > + for (i = 0; i < GET (ehdr, e_phnum); i++, phdr++) > + { > + if (GET (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_External_Shdr * > +find_shdrtab (Elf_External_Ehdr *ehdr) > +{ > + return (Elf_External_Shdr *) (((uint8_t *) ehdr) + GET (ehdr, e_shoff)); > +} > + > +/* Return the string table of the section headers. */ > + > +const char * > +find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size) > +{ > + const Elf_External_Shdr *shdr; > + const Elf_External_Shdr *shstr; > + > + if (GET (ehdr, e_shnum) <= GET (ehdr, e_shstrndx)) > + { > + printf ("The index of the string table is corrupt."); > + return NULL; > + } > + > + shdr = find_shdrtab (ehdr); > + > + shstr = &shdr[GET (ehdr, e_shstrndx)]; > + *size = GET (shstr, sh_size); > + return ((const char *) ehdr) + GET (shstr, sh_offset); > +} > + > +/* Return the string table named SECTION. */ > + > +const char * > +find_strtab (Elf_External_Ehdr *ehdr, > + const char *section, uint64_t *strtab_size) > +{ > + uint64_t shstrtab_size = 0; > + const char *shstrtab; > + uint64_t i; > + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); > + > + /* Get the string table of the section headers. */ > + shstrtab = find_shstrtab (ehdr, &shstrtab_size); > + if (shstrtab == NULL) > + return NULL; > + > + for (i = 0; i < GET (ehdr, e_shnum); i++) > + { > + uint64_t name = GET (shdr + i, sh_name); > + if (GET (shdr + i, sh_type) == SHT_STRTAB && name <= shstrtab_size > + && strcmp ((const char *) &shstrtab[name], section) == 0) > + { > + *strtab_size = GET (shdr + i, sh_size); > + return ((const char *) ehdr) + GET (shdr + i, sh_offset); > + } > + > + } > + return NULL; > +} > + > +/* Return the section header named SECTION. */ > + > +Elf_External_Shdr * > +find_shdr (Elf_External_Ehdr *ehdr, const char *section) > +{ > + uint64_t shstrtab_size = 0; > + const char *shstrtab; > + uint64_t i; > + > + /* Get the string table of the section headers. */ > + shstrtab = find_shstrtab (ehdr, &shstrtab_size); > + if (shstrtab == NULL) > + return NULL; > + > + Elf_External_Shdr *shdr = find_shdrtab (ehdr); > + for (i = 0; i < GET (ehdr, e_shnum); i++) > + { > + uint64_t name = GET (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_External_Sym * > +find_symtab (Elf_External_Ehdr *ehdr, uint64_t *symtab_size) > +{ > + uint64_t i; > + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); > + > + for (i = 0; i < GET (ehdr, e_shnum); i++) > + { > + if (GET (shdr + i, sh_type) == SHT_SYMTAB) > + { > + *symtab_size = GET (shdr + i, sh_size) / sizeof (Elf_External_Sym); > + return (Elf_External_Sym *) (((const char *) ehdr) + > + GET (shdr + i, sh_offset)); > + } > + } > + return NULL; > +} > + > +/* Translate a file offset to an address in a loaded segment. */ > + > +int > +translate_offset (uint64_t file_offset, struct segment *seg, void **addr) > +{ > + while (seg) > + { > + uint64_t p_from, p_to; > + > + Elf_External_Phdr *phdr = seg->phdr; > + > + if (phdr == NULL) > + { > + seg = seg->next; > + continue; > + } > + > + p_from = GET (phdr, p_offset); > + p_to = p_from + GET (phdr, p_filesz); > + > + if (p_from <= file_offset && file_offset < p_to) > + { > + *addr = (void *) (seg->mapped_addr + (file_offset - p_from)); > + return 0; > + } > + seg = seg->next; > + } > + > + return -1; > +} > + > +/* Lookup the address of FUNC. */ > + > +int > +lookup_function (const char *func, > + Elf_External_Ehdr *ehdr, struct segment *seg, void **addr) > +{ > + const char *strtab; > + uint64_t strtab_size = 0; > + Elf_External_Sym *symtab; > + uint64_t symtab_size = 0; > + uint64_t i; > + > + /* Get the string table for the symbols. */ > + strtab = find_strtab (ehdr, ".strtab", &strtab_size); > + if (strtab == NULL) > + { > + printf (".strtab not found."); > + return -1; > + } > + > + /* Get the symbol table. */ > + symtab = find_symtab (ehdr, &symtab_size); > + if (symtab == NULL) > + { > + printf ("symbol table not found."); > + return -1; > + } > + > + for (i = 0; i < symtab_size; i++) > + { > + Elf_External_Sym *sym = &symtab[i]; > + > + if (elf_st_type (GET (sym, st_info)) != STT_FUNC) > + continue; > + > + if (GET (sym, st_name) < strtab_size) > + { > + const char *name = &strtab[GET (sym, st_name)]; > + if (strcmp (name, func) == 0) > + { > + > + uint64_t offset = GET (sym, st_value); > + return translate_offset (offset, seg, addr); > + } > + } > + } > + > + return -1; > +} > diff --git a/gdb/testsuite/gdb.mi/sym-file-loader.h b/gdb/testsuite/gdb.mi/sym-file-loader.h > new file mode 100644 > index 0000000..03dc7e1 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-loader.h > @@ -0,0 +1,99 @@ > +/* Copyright 2013-2014 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/>. > +*/ > + > +#ifndef __SYM_FILE_LOADER__ > +#define __SYM_FILE_LOADER__ > + > +#include <inttypes.h> > +#include <ansidecl.h> > +#include <elf/common.h> > +#include <elf/external.h> > + > +#ifdef TARGET_LP64 > + > +typedef Elf64_External_Phdr Elf_External_Phdr; > +typedef Elf64_External_Ehdr Elf_External_Ehdr; > +typedef Elf64_External_Shdr Elf_External_Shdr; > +typedef Elf64_External_Sym Elf_External_Sym; > +typedef uint64_t Elf_Addr; > + > +#elif defined TARGET_ILP32 > + > +typedef Elf32_External_Phdr Elf_External_Phdr; > +typedef Elf32_External_Ehdr Elf_External_Ehdr; > +typedef Elf32_External_Shdr Elf_External_Shdr; > +typedef Elf32_External_Sym Elf_External_Sym; > +typedef uint32_t Elf_Addr; > + > +#endif > + > +#define GET(hdr, field) (\ > +sizeof ((hdr)->field) == 1 ? (uint64_t) (hdr)->field[0] : \ > +sizeof ((hdr)->field) == 2 ? (uint64_t) *(uint16_t *) (hdr)->field : \ > +sizeof ((hdr)->field) == 4 ? (uint64_t) *(uint32_t *) (hdr)->field : \ > +sizeof ((hdr)->field) == 8 ? *(uint64_t *) (hdr)->field : \ > +*(uint64_t *) NULL) > + > +#define GETADDR(hdr, field) (\ > +sizeof ((hdr)->field) == sizeof (Elf_Addr) ? *(Elf_Addr *) (hdr)->field : \ > +*(Elf_Addr *) NULL) > + > +struct segment > +{ > + uint8_t *mapped_addr; > + Elf_External_Phdr *phdr; > + struct segment *next; > +}; > + > +/* Mini shared library loader. No reallocation is performed > + for the sake of simplicity. */ > + > +int > +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, > + struct segment **seg_out); > + > +/* Return the section-header table. */ > + > +Elf_External_Shdr *find_shdrtab (Elf_External_Ehdr *ehdr); > + > +/* Return the string table of the section headers. */ > + > +const char *find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size); > + > +/* Return the string table named SECTION. */ > + > +const char *find_strtab (Elf_External_Ehdr *ehdr, > + const char *section, uint64_t *strtab_size); > + > +/* Return the section header named SECTION. */ > + > +Elf_External_Shdr *find_shdr (Elf_External_Ehdr *ehdr, const char *section); > + > +/* Return the symbol table. */ > + > +Elf_External_Sym *find_symtab (Elf_External_Ehdr *ehdr, > + uint64_t *symtab_size); > + > +/* Translate a file offset to an address in a loaded segment. */ > + > +int translate_offset (uint64_t file_offset, struct segment *seg, void **addr); > + > +/* Lookup the address of FUNC. */ > + > +int > +lookup_function (const char *func, Elf_External_Ehdr* ehdr, > + struct segment *seg, void **addr); > + > +#endif > diff --git a/gdb/testsuite/gdb.mi/sym-file-main.c b/gdb/testsuite/gdb.mi/sym-file-main.c > new file mode 100644 > index 0000000..8260824 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-main.c > @@ -0,0 +1,84 @@ > +/* Copyright 2013-2014 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 <stdlib.h> > +#include <stdio.h> > + > +#include "sym-file-loader.h" > + > +// Global variable > +int count = 0; > + > +void > +gdb_add_symbol_file (void *addr, const char *file) > +{ > + return; > +} > + > +void > +gdb_remove_symbol_file (void *addr) > +{ > + return; > +} > + > +/* 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_External_Ehdr *ehdr = NULL; > + struct segment *head_seg = NULL; > + Elf_External_Shdr *text; > + char *text_addr = NULL; > + int (*pbar) () = NULL; > + int (*pfoo) (int) = NULL; > + > + if (load_shlib (file, &ehdr, &head_seg) != 0) > + return -1; > + > + /* Get the text section. */ > + text = find_shdr (ehdr, ".text"); > + if (text == NULL) > + return -1; > + > + /* Notify GDB to add the symbol file. */ > + if (translate_offset (GET (text, sh_offset), head_seg, (void **) &text_addr) > + != 0) > + return -1; > + > + gdb_add_symbol_file (text_addr, file); > + > + /* Call bar from SHLIB_NAME. */ > + if (lookup_function ("bar", ehdr, head_seg, (void *) &pbar) != 0) > + return -1; > + > + (*pbar) (); > + > + /* Call foo from SHLIB_NAME. */ > + if (lookup_function ("foo", ehdr, head_seg, (void *) &pfoo) != 0) > + return -1; > + > + (*pfoo) (2); > + > + count++; > + > + /* Notify GDB to remove the symbol file. */ > + gdb_remove_symbol_file (text_addr); > + > + return 0; > +} > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PING][PATCH 2/2] Testsuite for varobj updation after symbol removal 2014-06-27 10:14 ` [PATCH 2/2] Testsuite for varobj updation after symbol removal Taimoor Mirza 2014-07-13 16:11 ` Taimoor @ 2014-08-05 11:01 ` Taimoor 2014-09-01 7:29 ` Taimoor 1 sibling, 1 reply; 14+ messages in thread From: Taimoor @ 2014-08-05 11:01 UTC (permalink / raw) To: gdb-patches Ping. On 06/27/2014 03:13 PM, Taimoor Mirza wrote: > This patch provides testcases for variable object updation after removing > symbols. Test programs are same as used for testing remove-symbol-file > command in gdb.base. sym-file-main.c is modified to just add a global > variable for which varible object is created in testsuite. > > Testing: > ======= > This is tested for x86 target and also on ppc-linux-gnu and mips-gnu-linux > using both simulator and real boards. > > 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> > > * gdb.mi/mi-var-invalidate.exp: Add tests to check global > variable object change list is correct when its value is > updated before removing symbols. > * gdb.mi/sym-file-loader.c: New file. > * gdb.mi/sym-file-loader.h: New file. > * gdb.mi/sym-file-main.c: New file. > * gdb.mi/sym-file-lib.c: New file. > > --- > gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ > gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ > gdb/testsuite/gdb.mi/sym-file-loader.c | 353 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ > gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ > 5 files changed, 629 insertions(+) > create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h > create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c > > diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > index e6ba392..e3bf953 100644 > --- a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > @@ -121,5 +121,72 @@ mi_gdb_test "-var-info-type global_simple" \ > "\\^done,type=\"\"" \ > "no type for invalid variable global_simple" > > +# Test varobj updation after removing symbols. > + > +if {[skip_shlib_tests]} { > + 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 main_basename sym-file-main > +set loader_basename sym-file-loader > +set lib_basename sym-file-lib > + > +standard_testfile $main_basename.c $loader_basename.c $lib_basename.c > + > +set libsrc "${srcdir}/${subdir}/${srcfile3}" > +set test_bin_name "sym-test-file" > +set test_bin [standard_output_file ${test_bin_name}] > +set shlib_name [standard_output_file ${lib_basename}.so] > +set exec_opts [list debug "additional_flags= -I$srcdir/../../include/ \ > +-D$target_size -DSHLIB_NAME\\=\"$shlib_name\""] > + > +if [get_compiler_info] { > + return -1 > +} > + > +if {[gdb_compile_shlib $libsrc $shlib_name {debug}] != ""} { > + untested ${testfile} > + return -1 > +} > + > +if {[build_executable $testfile $test_bin "$srcfile $srcfile2" $exec_opts]} { > + return -1 > +} > + > +mi_delete_breakpoints > +mi_gdb_load ${test_bin} > + > +# Create varobj for count variable. > +mi_create_varobj var_count count "Create global varobj for count" > + > +# Run to GDB_ADD_SYMBOl_FILE in $srcfile for adding > +# $shlib_name. > +mi_runto gdb_add_symbol_file > + > +# Add $shlib_name using 'add-symbol-file'. > +mi_gdb_test "-interpreter-exec console \"add-symbol-file ${shlib_name} addr\"" \ > + "~\"add symbol table from file .*so.*at.*= $hex.*\\^done" \ > + "add-symbol-file ${shlib_name}" > + > +# Continue to gdb_remove_symbol_file in $srcfile. > +mi_runto gdb_remove_symbol_file > + > +# Remove $shlib_name using 'remove-symbol-file'. > +mi_gdb_test "-interpreter-exec console \"remove-symbol-file -a addr\"" \ > + ".*\\^done"\ > + "remove-symbol-file test" > + > +# Check var_count varobj changelist is not empty. > +mi_varobj_update var_count {var_count} "Update var_count" > + > mi_gdb_exit > return 0 > diff --git a/gdb/testsuite/gdb.mi/sym-file-lib.c b/gdb/testsuite/gdb.mi/sym-file-lib.c > new file mode 100644 > index 0000000..dae5188 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-lib.c > @@ -0,0 +1,26 @@ > +/* Copyright 2013-2014 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.mi/sym-file-loader.c b/gdb/testsuite/gdb.mi/sym-file-loader.c > new file mode 100644 > index 0000000..c72a1d1 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-loader.c > @@ -0,0 +1,353 @@ > +/* Copyright 2013-2014 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 <unistd.h> > +#include <fcntl.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/mman.h> > + > +#include "sym-file-loader.h" > + > +#ifdef TARGET_LP64 > + > +uint8_t > +elf_st_type (uint8_t st_info) > +{ > + return ELF64_ST_TYPE (st_info); > +} > + > +#elif defined TARGET_ILP32 > + > +uint8_t > +elf_st_type (uint8_t st_info) > +{ > + return ELF32_ST_TYPE (st_info); > +} > + > +#endif > + > +/* Load a program segment. */ > + > +static struct segment * > +load (uint8_t *addr, Elf_External_Phdr *phdr, struct segment *tail_seg) > +{ > + struct segment *seg = NULL; > + uint8_t *mapped_addr = NULL; > + void *from = NULL; > + void *to = NULL; > + > + /* For the sake of simplicity all operations are permitted. */ > + unsigned perm = PROT_READ | PROT_WRITE | PROT_EXEC; > + > + mapped_addr = (uint8_t *) mmap ((void *) GETADDR (phdr, p_vaddr), > + GET (phdr, p_memsz), perm, > + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); > + > + from = (void *) (addr + GET (phdr, p_offset)); > + to = (void *) mapped_addr; > + > + memcpy (to, from, GET (phdr, p_filesz)); > + > + seg = (struct segment *) malloc (sizeof (struct segment)); > + > + 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; > +} > + > +/* Mini shared library loader. No reallocation > + is performed for the sake of simplicity. */ > + > +int > +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, > + struct segment **seg_out) > +{ > + uint64_t i; > + int fd; > + off_t fsize; > + uint8_t *addr; > + Elf_External_Ehdr *ehdr; > + Elf_External_Phdr *phdr; > + struct segment *head_seg = NULL; > + struct segment *tail_seg = NULL; > + > + /* Map the lib in memory for reading. */ > + fd = open (file, O_RDONLY); > + if (fd < 0) > + { > + perror ("fopen failed."); > + return -1; > + } > + > + fsize = lseek (fd, 0, SEEK_END); > + > + if (fsize < 0) > + { > + perror ("lseek failed."); > + return -1; > + } > + > + addr = (uint8_t *) mmap (NULL, fsize, PROT_READ, MAP_PRIVATE, fd, 0); > + if (addr == (uint8_t *) -1) > + { > + perror ("mmap failed."); > + return -1; > + } > + > + /* Check if the lib is an ELF file. */ > + ehdr = (Elf_External_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 (void *) != 4) > + { > + printf ("Architecture mismatch."); > + return -1; > + } > + } > + else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) > + { > + if (sizeof (void *) != 8) > + { > + printf ("Architecture mismatch."); > + return -1; > + } > + } > + > + /* Load the program segments. For the sake of simplicity > + assume that no reallocation is needed. */ > + phdr = (Elf_External_Phdr *) (addr + GET (ehdr, e_phoff)); > + for (i = 0; i < GET (ehdr, e_phnum); i++, phdr++) > + { > + if (GET (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_External_Shdr * > +find_shdrtab (Elf_External_Ehdr *ehdr) > +{ > + return (Elf_External_Shdr *) (((uint8_t *) ehdr) + GET (ehdr, e_shoff)); > +} > + > +/* Return the string table of the section headers. */ > + > +const char * > +find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size) > +{ > + const Elf_External_Shdr *shdr; > + const Elf_External_Shdr *shstr; > + > + if (GET (ehdr, e_shnum) <= GET (ehdr, e_shstrndx)) > + { > + printf ("The index of the string table is corrupt."); > + return NULL; > + } > + > + shdr = find_shdrtab (ehdr); > + > + shstr = &shdr[GET (ehdr, e_shstrndx)]; > + *size = GET (shstr, sh_size); > + return ((const char *) ehdr) + GET (shstr, sh_offset); > +} > + > +/* Return the string table named SECTION. */ > + > +const char * > +find_strtab (Elf_External_Ehdr *ehdr, > + const char *section, uint64_t *strtab_size) > +{ > + uint64_t shstrtab_size = 0; > + const char *shstrtab; > + uint64_t i; > + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); > + > + /* Get the string table of the section headers. */ > + shstrtab = find_shstrtab (ehdr, &shstrtab_size); > + if (shstrtab == NULL) > + return NULL; > + > + for (i = 0; i < GET (ehdr, e_shnum); i++) > + { > + uint64_t name = GET (shdr + i, sh_name); > + if (GET (shdr + i, sh_type) == SHT_STRTAB && name <= shstrtab_size > + && strcmp ((const char *) &shstrtab[name], section) == 0) > + { > + *strtab_size = GET (shdr + i, sh_size); > + return ((const char *) ehdr) + GET (shdr + i, sh_offset); > + } > + > + } > + return NULL; > +} > + > +/* Return the section header named SECTION. */ > + > +Elf_External_Shdr * > +find_shdr (Elf_External_Ehdr *ehdr, const char *section) > +{ > + uint64_t shstrtab_size = 0; > + const char *shstrtab; > + uint64_t i; > + > + /* Get the string table of the section headers. */ > + shstrtab = find_shstrtab (ehdr, &shstrtab_size); > + if (shstrtab == NULL) > + return NULL; > + > + Elf_External_Shdr *shdr = find_shdrtab (ehdr); > + for (i = 0; i < GET (ehdr, e_shnum); i++) > + { > + uint64_t name = GET (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_External_Sym * > +find_symtab (Elf_External_Ehdr *ehdr, uint64_t *symtab_size) > +{ > + uint64_t i; > + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); > + > + for (i = 0; i < GET (ehdr, e_shnum); i++) > + { > + if (GET (shdr + i, sh_type) == SHT_SYMTAB) > + { > + *symtab_size = GET (shdr + i, sh_size) / sizeof (Elf_External_Sym); > + return (Elf_External_Sym *) (((const char *) ehdr) + > + GET (shdr + i, sh_offset)); > + } > + } > + return NULL; > +} > + > +/* Translate a file offset to an address in a loaded segment. */ > + > +int > +translate_offset (uint64_t file_offset, struct segment *seg, void **addr) > +{ > + while (seg) > + { > + uint64_t p_from, p_to; > + > + Elf_External_Phdr *phdr = seg->phdr; > + > + if (phdr == NULL) > + { > + seg = seg->next; > + continue; > + } > + > + p_from = GET (phdr, p_offset); > + p_to = p_from + GET (phdr, p_filesz); > + > + if (p_from <= file_offset && file_offset < p_to) > + { > + *addr = (void *) (seg->mapped_addr + (file_offset - p_from)); > + return 0; > + } > + seg = seg->next; > + } > + > + return -1; > +} > + > +/* Lookup the address of FUNC. */ > + > +int > +lookup_function (const char *func, > + Elf_External_Ehdr *ehdr, struct segment *seg, void **addr) > +{ > + const char *strtab; > + uint64_t strtab_size = 0; > + Elf_External_Sym *symtab; > + uint64_t symtab_size = 0; > + uint64_t i; > + > + /* Get the string table for the symbols. */ > + strtab = find_strtab (ehdr, ".strtab", &strtab_size); > + if (strtab == NULL) > + { > + printf (".strtab not found."); > + return -1; > + } > + > + /* Get the symbol table. */ > + symtab = find_symtab (ehdr, &symtab_size); > + if (symtab == NULL) > + { > + printf ("symbol table not found."); > + return -1; > + } > + > + for (i = 0; i < symtab_size; i++) > + { > + Elf_External_Sym *sym = &symtab[i]; > + > + if (elf_st_type (GET (sym, st_info)) != STT_FUNC) > + continue; > + > + if (GET (sym, st_name) < strtab_size) > + { > + const char *name = &strtab[GET (sym, st_name)]; > + if (strcmp (name, func) == 0) > + { > + > + uint64_t offset = GET (sym, st_value); > + return translate_offset (offset, seg, addr); > + } > + } > + } > + > + return -1; > +} > diff --git a/gdb/testsuite/gdb.mi/sym-file-loader.h b/gdb/testsuite/gdb.mi/sym-file-loader.h > new file mode 100644 > index 0000000..03dc7e1 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-loader.h > @@ -0,0 +1,99 @@ > +/* Copyright 2013-2014 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/>. > +*/ > + > +#ifndef __SYM_FILE_LOADER__ > +#define __SYM_FILE_LOADER__ > + > +#include <inttypes.h> > +#include <ansidecl.h> > +#include <elf/common.h> > +#include <elf/external.h> > + > +#ifdef TARGET_LP64 > + > +typedef Elf64_External_Phdr Elf_External_Phdr; > +typedef Elf64_External_Ehdr Elf_External_Ehdr; > +typedef Elf64_External_Shdr Elf_External_Shdr; > +typedef Elf64_External_Sym Elf_External_Sym; > +typedef uint64_t Elf_Addr; > + > +#elif defined TARGET_ILP32 > + > +typedef Elf32_External_Phdr Elf_External_Phdr; > +typedef Elf32_External_Ehdr Elf_External_Ehdr; > +typedef Elf32_External_Shdr Elf_External_Shdr; > +typedef Elf32_External_Sym Elf_External_Sym; > +typedef uint32_t Elf_Addr; > + > +#endif > + > +#define GET(hdr, field) (\ > +sizeof ((hdr)->field) == 1 ? (uint64_t) (hdr)->field[0] : \ > +sizeof ((hdr)->field) == 2 ? (uint64_t) *(uint16_t *) (hdr)->field : \ > +sizeof ((hdr)->field) == 4 ? (uint64_t) *(uint32_t *) (hdr)->field : \ > +sizeof ((hdr)->field) == 8 ? *(uint64_t *) (hdr)->field : \ > +*(uint64_t *) NULL) > + > +#define GETADDR(hdr, field) (\ > +sizeof ((hdr)->field) == sizeof (Elf_Addr) ? *(Elf_Addr *) (hdr)->field : \ > +*(Elf_Addr *) NULL) > + > +struct segment > +{ > + uint8_t *mapped_addr; > + Elf_External_Phdr *phdr; > + struct segment *next; > +}; > + > +/* Mini shared library loader. No reallocation is performed > + for the sake of simplicity. */ > + > +int > +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, > + struct segment **seg_out); > + > +/* Return the section-header table. */ > + > +Elf_External_Shdr *find_shdrtab (Elf_External_Ehdr *ehdr); > + > +/* Return the string table of the section headers. */ > + > +const char *find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size); > + > +/* Return the string table named SECTION. */ > + > +const char *find_strtab (Elf_External_Ehdr *ehdr, > + const char *section, uint64_t *strtab_size); > + > +/* Return the section header named SECTION. */ > + > +Elf_External_Shdr *find_shdr (Elf_External_Ehdr *ehdr, const char *section); > + > +/* Return the symbol table. */ > + > +Elf_External_Sym *find_symtab (Elf_External_Ehdr *ehdr, > + uint64_t *symtab_size); > + > +/* Translate a file offset to an address in a loaded segment. */ > + > +int translate_offset (uint64_t file_offset, struct segment *seg, void **addr); > + > +/* Lookup the address of FUNC. */ > + > +int > +lookup_function (const char *func, Elf_External_Ehdr* ehdr, > + struct segment *seg, void **addr); > + > +#endif > diff --git a/gdb/testsuite/gdb.mi/sym-file-main.c b/gdb/testsuite/gdb.mi/sym-file-main.c > new file mode 100644 > index 0000000..8260824 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/sym-file-main.c > @@ -0,0 +1,84 @@ > +/* Copyright 2013-2014 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 <stdlib.h> > +#include <stdio.h> > + > +#include "sym-file-loader.h" > + > +// Global variable > +int count = 0; > + > +void > +gdb_add_symbol_file (void *addr, const char *file) > +{ > + return; > +} > + > +void > +gdb_remove_symbol_file (void *addr) > +{ > + return; > +} > + > +/* 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_External_Ehdr *ehdr = NULL; > + struct segment *head_seg = NULL; > + Elf_External_Shdr *text; > + char *text_addr = NULL; > + int (*pbar) () = NULL; > + int (*pfoo) (int) = NULL; > + > + if (load_shlib (file, &ehdr, &head_seg) != 0) > + return -1; > + > + /* Get the text section. */ > + text = find_shdr (ehdr, ".text"); > + if (text == NULL) > + return -1; > + > + /* Notify GDB to add the symbol file. */ > + if (translate_offset (GET (text, sh_offset), head_seg, (void **) &text_addr) > + != 0) > + return -1; > + > + gdb_add_symbol_file (text_addr, file); > + > + /* Call bar from SHLIB_NAME. */ > + if (lookup_function ("bar", ehdr, head_seg, (void *) &pbar) != 0) > + return -1; > + > + (*pbar) (); > + > + /* Call foo from SHLIB_NAME. */ > + if (lookup_function ("foo", ehdr, head_seg, (void *) &pfoo) != 0) > + return -1; > + > + (*pfoo) (2); > + > + count++; > + > + /* Notify GDB to remove the symbol file. */ > + gdb_remove_symbol_file (text_addr); > + > + return 0; > +} > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PING][PATCH 2/2] Testsuite for varobj updation after symbol removal 2014-08-05 11:01 ` [PING][PATCH " Taimoor @ 2014-09-01 7:29 ` Taimoor 0 siblings, 0 replies; 14+ messages in thread From: Taimoor @ 2014-09-01 7:29 UTC (permalink / raw) To: gdb-patches Ping Regards, Taimoor On 08/05/2014 04:01 PM, Taimoor wrote: > Ping. > > On 06/27/2014 03:13 PM, Taimoor Mirza wrote: >> This patch provides testcases for variable object updation after removing >> symbols. Test programs are same as used for testing remove-symbol-file >> command in gdb.base. sym-file-main.c is modified to just add a global >> variable for which varible object is created in testsuite. >> >> Testing: >> ======= >> This is tested for x86 target and also on ppc-linux-gnu and >> mips-gnu-linux >> using both simulator and real boards. >> >> 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> >> >> * gdb.mi/mi-var-invalidate.exp: Add tests to check global >> variable object change list is correct when its value is >> updated before removing symbols. >> * gdb.mi/sym-file-loader.c: New file. >> * gdb.mi/sym-file-loader.h: New file. >> * gdb.mi/sym-file-main.c: New file. >> * gdb.mi/sym-file-lib.c: New file. >> >> --- >> gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ >> gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ >> gdb/testsuite/gdb.mi/sym-file-loader.c | 353 >> ++++++++++++++++++++++++++++ >> gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ >> gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ >> 5 files changed, 629 insertions(+) >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c >> >> diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp >> b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp >> index e6ba392..e3bf953 100644 >> --- a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp >> +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp >> @@ -121,5 +121,72 @@ mi_gdb_test "-var-info-type global_simple" \ >> "\\^done,type=\"\"" \ >> "no type for invalid variable global_simple" >> >> +# Test varobj updation after removing symbols. >> + >> +if {[skip_shlib_tests]} { >> + 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 main_basename sym-file-main >> +set loader_basename sym-file-loader >> +set lib_basename sym-file-lib >> + >> +standard_testfile $main_basename.c $loader_basename.c $lib_basename.c >> + >> +set libsrc "${srcdir}/${subdir}/${srcfile3}" >> +set test_bin_name "sym-test-file" >> +set test_bin [standard_output_file ${test_bin_name}] >> +set shlib_name [standard_output_file ${lib_basename}.so] >> +set exec_opts [list debug "additional_flags= -I$srcdir/../../include/ \ >> +-D$target_size -DSHLIB_NAME\\=\"$shlib_name\""] >> + >> +if [get_compiler_info] { >> + return -1 >> +} >> + >> +if {[gdb_compile_shlib $libsrc $shlib_name {debug}] != ""} { >> + untested ${testfile} >> + return -1 >> +} >> + >> +if {[build_executable $testfile $test_bin "$srcfile $srcfile2" >> $exec_opts]} { >> + return -1 >> +} >> + >> +mi_delete_breakpoints >> +mi_gdb_load ${test_bin} >> + >> +# Create varobj for count variable. >> +mi_create_varobj var_count count "Create global varobj for count" >> + >> +# Run to GDB_ADD_SYMBOl_FILE in $srcfile for adding >> +# $shlib_name. >> +mi_runto gdb_add_symbol_file >> + >> +# Add $shlib_name using 'add-symbol-file'. >> +mi_gdb_test "-interpreter-exec console \"add-symbol-file >> ${shlib_name} addr\"" \ >> + "~\"add symbol table from file .*so.*at.*= $hex.*\\^done" \ >> + "add-symbol-file ${shlib_name}" >> + >> +# Continue to gdb_remove_symbol_file in $srcfile. >> +mi_runto gdb_remove_symbol_file >> + >> +# Remove $shlib_name using 'remove-symbol-file'. >> +mi_gdb_test "-interpreter-exec console \"remove-symbol-file -a addr\"" \ >> + ".*\\^done"\ >> + "remove-symbol-file test" >> + >> +# Check var_count varobj changelist is not empty. >> +mi_varobj_update var_count {var_count} "Update var_count" >> + >> mi_gdb_exit >> return 0 >> diff --git a/gdb/testsuite/gdb.mi/sym-file-lib.c >> b/gdb/testsuite/gdb.mi/sym-file-lib.c >> new file mode 100644 >> index 0000000..dae5188 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/sym-file-lib.c >> @@ -0,0 +1,26 @@ >> +/* Copyright 2013-2014 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.mi/sym-file-loader.c >> b/gdb/testsuite/gdb.mi/sym-file-loader.c >> new file mode 100644 >> index 0000000..c72a1d1 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/sym-file-loader.c >> @@ -0,0 +1,353 @@ >> +/* Copyright 2013-2014 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 <unistd.h> >> +#include <fcntl.h> >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <string.h> >> +#include <sys/mman.h> >> + >> +#include "sym-file-loader.h" >> + >> +#ifdef TARGET_LP64 >> + >> +uint8_t >> +elf_st_type (uint8_t st_info) >> +{ >> + return ELF64_ST_TYPE (st_info); >> +} >> + >> +#elif defined TARGET_ILP32 >> + >> +uint8_t >> +elf_st_type (uint8_t st_info) >> +{ >> + return ELF32_ST_TYPE (st_info); >> +} >> + >> +#endif >> + >> +/* Load a program segment. */ >> + >> +static struct segment * >> +load (uint8_t *addr, Elf_External_Phdr *phdr, struct segment *tail_seg) >> +{ >> + struct segment *seg = NULL; >> + uint8_t *mapped_addr = NULL; >> + void *from = NULL; >> + void *to = NULL; >> + >> + /* For the sake of simplicity all operations are permitted. */ >> + unsigned perm = PROT_READ | PROT_WRITE | PROT_EXEC; >> + >> + mapped_addr = (uint8_t *) mmap ((void *) GETADDR (phdr, p_vaddr), >> + GET (phdr, p_memsz), perm, >> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); >> + >> + from = (void *) (addr + GET (phdr, p_offset)); >> + to = (void *) mapped_addr; >> + >> + memcpy (to, from, GET (phdr, p_filesz)); >> + >> + seg = (struct segment *) malloc (sizeof (struct segment)); >> + >> + 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; >> +} >> + >> +/* Mini shared library loader. No reallocation >> + is performed for the sake of simplicity. */ >> + >> +int >> +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, >> + struct segment **seg_out) >> +{ >> + uint64_t i; >> + int fd; >> + off_t fsize; >> + uint8_t *addr; >> + Elf_External_Ehdr *ehdr; >> + Elf_External_Phdr *phdr; >> + struct segment *head_seg = NULL; >> + struct segment *tail_seg = NULL; >> + >> + /* Map the lib in memory for reading. */ >> + fd = open (file, O_RDONLY); >> + if (fd < 0) >> + { >> + perror ("fopen failed."); >> + return -1; >> + } >> + >> + fsize = lseek (fd, 0, SEEK_END); >> + >> + if (fsize < 0) >> + { >> + perror ("lseek failed."); >> + return -1; >> + } >> + >> + addr = (uint8_t *) mmap (NULL, fsize, PROT_READ, MAP_PRIVATE, fd, 0); >> + if (addr == (uint8_t *) -1) >> + { >> + perror ("mmap failed."); >> + return -1; >> + } >> + >> + /* Check if the lib is an ELF file. */ >> + ehdr = (Elf_External_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 (void *) != 4) >> + { >> + printf ("Architecture mismatch."); >> + return -1; >> + } >> + } >> + else if (ehdr->e_ident[EI_CLASS] == ELFCLASS64) >> + { >> + if (sizeof (void *) != 8) >> + { >> + printf ("Architecture mismatch."); >> + return -1; >> + } >> + } >> + >> + /* Load the program segments. For the sake of simplicity >> + assume that no reallocation is needed. */ >> + phdr = (Elf_External_Phdr *) (addr + GET (ehdr, e_phoff)); >> + for (i = 0; i < GET (ehdr, e_phnum); i++, phdr++) >> + { >> + if (GET (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_External_Shdr * >> +find_shdrtab (Elf_External_Ehdr *ehdr) >> +{ >> + return (Elf_External_Shdr *) (((uint8_t *) ehdr) + GET (ehdr, >> e_shoff)); >> +} >> + >> +/* Return the string table of the section headers. */ >> + >> +const char * >> +find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size) >> +{ >> + const Elf_External_Shdr *shdr; >> + const Elf_External_Shdr *shstr; >> + >> + if (GET (ehdr, e_shnum) <= GET (ehdr, e_shstrndx)) >> + { >> + printf ("The index of the string table is corrupt."); >> + return NULL; >> + } >> + >> + shdr = find_shdrtab (ehdr); >> + >> + shstr = &shdr[GET (ehdr, e_shstrndx)]; >> + *size = GET (shstr, sh_size); >> + return ((const char *) ehdr) + GET (shstr, sh_offset); >> +} >> + >> +/* Return the string table named SECTION. */ >> + >> +const char * >> +find_strtab (Elf_External_Ehdr *ehdr, >> + const char *section, uint64_t *strtab_size) >> +{ >> + uint64_t shstrtab_size = 0; >> + const char *shstrtab; >> + uint64_t i; >> + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); >> + >> + /* Get the string table of the section headers. */ >> + shstrtab = find_shstrtab (ehdr, &shstrtab_size); >> + if (shstrtab == NULL) >> + return NULL; >> + >> + for (i = 0; i < GET (ehdr, e_shnum); i++) >> + { >> + uint64_t name = GET (shdr + i, sh_name); >> + if (GET (shdr + i, sh_type) == SHT_STRTAB && name <= shstrtab_size >> + && strcmp ((const char *) &shstrtab[name], section) == 0) >> + { >> + *strtab_size = GET (shdr + i, sh_size); >> + return ((const char *) ehdr) + GET (shdr + i, sh_offset); >> + } >> + >> + } >> + return NULL; >> +} >> + >> +/* Return the section header named SECTION. */ >> + >> +Elf_External_Shdr * >> +find_shdr (Elf_External_Ehdr *ehdr, const char *section) >> +{ >> + uint64_t shstrtab_size = 0; >> + const char *shstrtab; >> + uint64_t i; >> + >> + /* Get the string table of the section headers. */ >> + shstrtab = find_shstrtab (ehdr, &shstrtab_size); >> + if (shstrtab == NULL) >> + return NULL; >> + >> + Elf_External_Shdr *shdr = find_shdrtab (ehdr); >> + for (i = 0; i < GET (ehdr, e_shnum); i++) >> + { >> + uint64_t name = GET (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_External_Sym * >> +find_symtab (Elf_External_Ehdr *ehdr, uint64_t *symtab_size) >> +{ >> + uint64_t i; >> + const Elf_External_Shdr *shdr = find_shdrtab (ehdr); >> + >> + for (i = 0; i < GET (ehdr, e_shnum); i++) >> + { >> + if (GET (shdr + i, sh_type) == SHT_SYMTAB) >> + { >> + *symtab_size = GET (shdr + i, sh_size) / sizeof >> (Elf_External_Sym); >> + return (Elf_External_Sym *) (((const char *) ehdr) + >> + GET (shdr + i, sh_offset)); >> + } >> + } >> + return NULL; >> +} >> + >> +/* Translate a file offset to an address in a loaded segment. */ >> + >> +int >> +translate_offset (uint64_t file_offset, struct segment *seg, void >> **addr) >> +{ >> + while (seg) >> + { >> + uint64_t p_from, p_to; >> + >> + Elf_External_Phdr *phdr = seg->phdr; >> + >> + if (phdr == NULL) >> + { >> + seg = seg->next; >> + continue; >> + } >> + >> + p_from = GET (phdr, p_offset); >> + p_to = p_from + GET (phdr, p_filesz); >> + >> + if (p_from <= file_offset && file_offset < p_to) >> + { >> + *addr = (void *) (seg->mapped_addr + (file_offset - p_from)); >> + return 0; >> + } >> + seg = seg->next; >> + } >> + >> + return -1; >> +} >> + >> +/* Lookup the address of FUNC. */ >> + >> +int >> +lookup_function (const char *func, >> + Elf_External_Ehdr *ehdr, struct segment *seg, void **addr) >> +{ >> + const char *strtab; >> + uint64_t strtab_size = 0; >> + Elf_External_Sym *symtab; >> + uint64_t symtab_size = 0; >> + uint64_t i; >> + >> + /* Get the string table for the symbols. */ >> + strtab = find_strtab (ehdr, ".strtab", &strtab_size); >> + if (strtab == NULL) >> + { >> + printf (".strtab not found."); >> + return -1; >> + } >> + >> + /* Get the symbol table. */ >> + symtab = find_symtab (ehdr, &symtab_size); >> + if (symtab == NULL) >> + { >> + printf ("symbol table not found."); >> + return -1; >> + } >> + >> + for (i = 0; i < symtab_size; i++) >> + { >> + Elf_External_Sym *sym = &symtab[i]; >> + >> + if (elf_st_type (GET (sym, st_info)) != STT_FUNC) >> + continue; >> + >> + if (GET (sym, st_name) < strtab_size) >> + { >> + const char *name = &strtab[GET (sym, st_name)]; >> + if (strcmp (name, func) == 0) >> + { >> + >> + uint64_t offset = GET (sym, st_value); >> + return translate_offset (offset, seg, addr); >> + } >> + } >> + } >> + >> + return -1; >> +} >> diff --git a/gdb/testsuite/gdb.mi/sym-file-loader.h >> b/gdb/testsuite/gdb.mi/sym-file-loader.h >> new file mode 100644 >> index 0000000..03dc7e1 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/sym-file-loader.h >> @@ -0,0 +1,99 @@ >> +/* Copyright 2013-2014 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/>. >> +*/ >> + >> +#ifndef __SYM_FILE_LOADER__ >> +#define __SYM_FILE_LOADER__ >> + >> +#include <inttypes.h> >> +#include <ansidecl.h> >> +#include <elf/common.h> >> +#include <elf/external.h> >> + >> +#ifdef TARGET_LP64 >> + >> +typedef Elf64_External_Phdr Elf_External_Phdr; >> +typedef Elf64_External_Ehdr Elf_External_Ehdr; >> +typedef Elf64_External_Shdr Elf_External_Shdr; >> +typedef Elf64_External_Sym Elf_External_Sym; >> +typedef uint64_t Elf_Addr; >> + >> +#elif defined TARGET_ILP32 >> + >> +typedef Elf32_External_Phdr Elf_External_Phdr; >> +typedef Elf32_External_Ehdr Elf_External_Ehdr; >> +typedef Elf32_External_Shdr Elf_External_Shdr; >> +typedef Elf32_External_Sym Elf_External_Sym; >> +typedef uint32_t Elf_Addr; >> + >> +#endif >> + >> +#define GET(hdr, field) (\ >> +sizeof ((hdr)->field) == 1 ? (uint64_t) (hdr)->field[0] : \ >> +sizeof ((hdr)->field) == 2 ? (uint64_t) *(uint16_t *) (hdr)->field : \ >> +sizeof ((hdr)->field) == 4 ? (uint64_t) *(uint32_t *) (hdr)->field : \ >> +sizeof ((hdr)->field) == 8 ? *(uint64_t *) (hdr)->field : \ >> +*(uint64_t *) NULL) >> + >> +#define GETADDR(hdr, field) (\ >> +sizeof ((hdr)->field) == sizeof (Elf_Addr) ? *(Elf_Addr *) >> (hdr)->field : \ >> +*(Elf_Addr *) NULL) >> + >> +struct segment >> +{ >> + uint8_t *mapped_addr; >> + Elf_External_Phdr *phdr; >> + struct segment *next; >> +}; >> + >> +/* Mini shared library loader. No reallocation is performed >> + for the sake of simplicity. */ >> + >> +int >> +load_shlib (const char *file, Elf_External_Ehdr **ehdr_out, >> + struct segment **seg_out); >> + >> +/* Return the section-header table. */ >> + >> +Elf_External_Shdr *find_shdrtab (Elf_External_Ehdr *ehdr); >> + >> +/* Return the string table of the section headers. */ >> + >> +const char *find_shstrtab (Elf_External_Ehdr *ehdr, uint64_t *size); >> + >> +/* Return the string table named SECTION. */ >> + >> +const char *find_strtab (Elf_External_Ehdr *ehdr, >> + const char *section, uint64_t *strtab_size); >> + >> +/* Return the section header named SECTION. */ >> + >> +Elf_External_Shdr *find_shdr (Elf_External_Ehdr *ehdr, const char >> *section); >> + >> +/* Return the symbol table. */ >> + >> +Elf_External_Sym *find_symtab (Elf_External_Ehdr *ehdr, >> + uint64_t *symtab_size); >> + >> +/* Translate a file offset to an address in a loaded segment. */ >> + >> +int translate_offset (uint64_t file_offset, struct segment *seg, void >> **addr); >> + >> +/* Lookup the address of FUNC. */ >> + >> +int >> +lookup_function (const char *func, Elf_External_Ehdr* ehdr, >> + struct segment *seg, void **addr); >> + >> +#endif >> diff --git a/gdb/testsuite/gdb.mi/sym-file-main.c >> b/gdb/testsuite/gdb.mi/sym-file-main.c >> new file mode 100644 >> index 0000000..8260824 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/sym-file-main.c >> @@ -0,0 +1,84 @@ >> +/* Copyright 2013-2014 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 <stdlib.h> >> +#include <stdio.h> >> + >> +#include "sym-file-loader.h" >> + >> +// Global variable >> +int count = 0; >> + >> +void >> +gdb_add_symbol_file (void *addr, const char *file) >> +{ >> + return; >> +} >> + >> +void >> +gdb_remove_symbol_file (void *addr) >> +{ >> + return; >> +} >> + >> +/* 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_External_Ehdr *ehdr = NULL; >> + struct segment *head_seg = NULL; >> + Elf_External_Shdr *text; >> + char *text_addr = NULL; >> + int (*pbar) () = NULL; >> + int (*pfoo) (int) = NULL; >> + >> + if (load_shlib (file, &ehdr, &head_seg) != 0) >> + return -1; >> + >> + /* Get the text section. */ >> + text = find_shdr (ehdr, ".text"); >> + if (text == NULL) >> + return -1; >> + >> + /* Notify GDB to add the symbol file. */ >> + if (translate_offset (GET (text, sh_offset), head_seg, (void **) >> &text_addr) >> + != 0) >> + return -1; >> + >> + gdb_add_symbol_file (text_addr, file); >> + >> + /* Call bar from SHLIB_NAME. */ >> + if (lookup_function ("bar", ehdr, head_seg, (void *) &pbar) != 0) >> + return -1; >> + >> + (*pbar) (); >> + >> + /* Call foo from SHLIB_NAME. */ >> + if (lookup_function ("foo", ehdr, head_seg, (void *) &pfoo) != 0) >> + return -1; >> + >> + (*pfoo) (2); >> + >> + count++; >> + >> + /* Notify GDB to remove the symbol file. */ >> + gdb_remove_symbol_file (text_addr); >> + >> + return 0; >> +} >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] Fix varobj updation after symbol removal 2014-06-27 10:14 [PATCH 0/2] Improved variable object invalidation in GDB Taimoor Mirza 2014-06-27 10:14 ` [PATCH 2/2] Testsuite for varobj updation after symbol removal Taimoor Mirza @ 2014-06-27 10:14 ` Taimoor Mirza 2014-07-13 7:50 ` Taimoor 2014-08-05 11:00 ` [PING][PATCH " Taimoor 2014-07-13 7:49 ` [PATCH 0/2] Improved variable object invalidation in GDB Taimoor 2014-08-05 11:00 ` [PING][PATCH " Taimoor 3 siblings, 2 replies; 14+ messages in thread From: Taimoor Mirza @ 2014-06-27 10:14 UTC (permalink / raw) To: gdb-patches; +Cc: Taimoor Mirza This problem was observed while loading and unloading symbols using add-symbol-file and remove-symbol-file. When remove-symbol-file command is invoked, it calls clear_symtab_users that calls varobj_invalidate to invalidate variable objects. This function invalidates the varobjs that are tied to locals and re-create the ones that are defined on globals. During this re-creation of globals, variable objects are re-evaluated that can result in new value. But this change is not recorded and because of this, -var-update for such modified variable objects gives empty change list. Proposed Fix: ============= GDB has mechanism of marking varobj's updated if they are set via varobj_set_value operation. This allows any next -var-update to report this change. Same approach should be used during varobj invalidation. If value of newly created varobj is different from previous one, mark it updated so that -var-update can get this change. Variable object invalidation code is cleaned up to avoid using pointers whose target has been already freed. Fix Testing: =========== This fix has been regression tested on both simulator and real boards. 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> Maciej W. Rozycki <macro@codesourcery.com> gdb/ * varobj.h (varobj_is_valid_p, varobj_set_invalid): New prototypes. * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions. (varobj_invalidate_iter): Mark re-created global object updated if its value is different from previous value. * objfiles.c (invalidate_objfile_varobj_type_iter): New function. (free_objfile): Call it. --- gdb/objfiles.c | 19 +++++++++++++++++++ gdb/varobj.c | 37 +++++++++++++++++++++++++++++++++++++ gdb/varobj.h | 3 +++ 3 files changed, 59 insertions(+) diff --git a/gdb/objfiles.c b/gdb/objfiles.c index 0a0b1cb..03559a3 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -32,6 +32,7 @@ #include "bcache.h" #include "expression.h" #include "parser-defs.h" +#include "varobj.h" #include "gdb_assert.h" #include <sys/types.h> @@ -538,6 +539,21 @@ free_objfile_separate_debug (struct objfile *objfile) } } +/* Mark the variable object VAR invalid if built upon a type coming from + the objfile requested, passed as DATA. Also clear the type reference. */ + +static void +invalidate_objfile_varobj_type_iter (struct varobj *var, void *data) +{ + struct objfile *objfile = data; + + if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile) + { + varobj_set_invalid (var); + var->type = NULL; + } +} + /* Destroy an objfile and all the symtabs and psymtabs under it. */ void @@ -584,6 +600,9 @@ free_objfile (struct objfile *objfile) lists. */ preserve_values (objfile); + /* Varobj may refer to types stored in objfile's obstack. */ + all_root_varobjs (invalidate_objfile_varobj_type_iter, objfile); + /* It still may reference data modules have associated with the objfile and the symbol file data. */ forget_cached_source_info_for_objfile (objfile); diff --git a/gdb/varobj.c b/gdb/varobj.c index 7446f10..2a563af 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -2683,6 +2683,22 @@ varobj_floating_p (struct varobj *var) return var->root->floating; } +/* Get the valid flag of varobj VAR. */ + +int +varobj_is_valid_p (struct varobj *var) +{ + return var->root->is_valid; +} + +/* Clear the valid flag on varobj VAR. */ + +void +varobj_set_invalid (struct varobj *var) +{ + var->root->is_valid = 0; +} + /* Implement the "value_is_changeable_p" varobj callback for most languages. */ @@ -2762,6 +2778,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused) if (var->root->floating || var->root->valid_block == NULL) { struct varobj *tmp_var; + char *tmp_var_value, *var_value; /* Try to create a varobj with same expression. If we succeed replace the old varobj, otherwise invalidate it. */ @@ -2770,6 +2787,26 @@ varobj_invalidate_iter (struct varobj *var, void *unused) if (tmp_var != NULL) { tmp_var->obj_name = xstrdup (var->obj_name); + tmp_var_value = varobj_get_value (tmp_var); + var_value = varobj_get_value (var); + + /* As varobjs are re-evaluated during creation so there is a + chance that new value is different from old one. Compare + value of old varobj and newly created varobj and mark + varobj updated If new value is different. */ + if (var_value == NULL && tmp_var_value == NULL) + ; /* Equal. */ + else if (var_value == NULL || tmp_var_value == NULL) + tmp_var->updated = 1; + else + { + /* Mark tmp_var updated if new value is different. */ + if (strcmp (tmp_var_value, var_value) != 0) + tmp_var->updated = 1; + } + + xfree (tmp_var_value); + xfree (var_value); varobj_delete (var, NULL, 0); install_variable (tmp_var); } diff --git a/gdb/varobj.h b/gdb/varobj.h index 74d41cf..7439a94 100644 --- a/gdb/varobj.h +++ b/gdb/varobj.h @@ -299,6 +299,9 @@ extern int varobj_editable_p (struct varobj *var); extern int varobj_floating_p (struct varobj *var); +extern int varobj_is_valid_p (struct varobj *var); +extern void varobj_set_invalid (struct varobj *var); + extern void varobj_set_visualizer (struct varobj *var, const char *visualizer); -- 1.7.9.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] Fix varobj updation after symbol removal 2014-06-27 10:14 ` [PATCH 1/2] Fix " Taimoor Mirza @ 2014-07-13 7:50 ` Taimoor 2014-08-05 11:00 ` [PING][PATCH " Taimoor 1 sibling, 0 replies; 14+ messages in thread From: Taimoor @ 2014-07-13 7:50 UTC (permalink / raw) To: gdb-patches ping. -Taimoor On 06/27/2014 03:13 PM, Taimoor Mirza wrote: > This problem was observed while loading and unloading symbols using > add-symbol-file and remove-symbol-file. When remove-symbol-file > command is invoked, it calls clear_symtab_users that calls varobj_invalidate > to invalidate variable objects. This function invalidates the varobjs > that are tied to locals and re-create the ones that are defined on > globals. During this re-creation of globals, variable objects are > re-evaluated that can result in new value. But this change is not recorded > and because of this, -var-update for such modified variable objects > gives empty change list. > > Proposed Fix: > ============= > GDB has mechanism of marking varobj's updated if they are set via > varobj_set_value operation. This allows any next -var-update to report > this change. Same approach should be used during varobj invalidation. > If value of newly created varobj is different from previous one, mark it > updated so that -var-update can get this change. > > Variable object invalidation code is cleaned up to avoid using pointers > whose target has been already freed. > > Fix Testing: > =========== > This fix has been regression tested on both simulator and real boards. > > 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> > Maciej W. Rozycki <macro@codesourcery.com> > > gdb/ > * varobj.h (varobj_is_valid_p, varobj_set_invalid): New > prototypes. > * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions. > (varobj_invalidate_iter): Mark re-created global object updated > if its value is different from previous value. > * objfiles.c (invalidate_objfile_varobj_type_iter): New function. > (free_objfile): Call it. > > --- > gdb/objfiles.c | 19 +++++++++++++++++++ > gdb/varobj.c | 37 +++++++++++++++++++++++++++++++++++++ > gdb/varobj.h | 3 +++ > 3 files changed, 59 insertions(+) > > diff --git a/gdb/objfiles.c b/gdb/objfiles.c > index 0a0b1cb..03559a3 100644 > --- a/gdb/objfiles.c > +++ b/gdb/objfiles.c > @@ -32,6 +32,7 @@ > #include "bcache.h" > #include "expression.h" > #include "parser-defs.h" > +#include "varobj.h" > > #include "gdb_assert.h" > #include <sys/types.h> > @@ -538,6 +539,21 @@ free_objfile_separate_debug (struct objfile *objfile) > } > } > > +/* Mark the variable object VAR invalid if built upon a type coming from > + the objfile requested, passed as DATA. Also clear the type reference. */ > + > +static void > +invalidate_objfile_varobj_type_iter (struct varobj *var, void *data) > +{ > + struct objfile *objfile = data; > + > + if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile) > + { > + varobj_set_invalid (var); > + var->type = NULL; > + } > +} > + > /* Destroy an objfile and all the symtabs and psymtabs under it. */ > > void > @@ -584,6 +600,9 @@ free_objfile (struct objfile *objfile) > lists. */ > preserve_values (objfile); > > + /* Varobj may refer to types stored in objfile's obstack. */ > + all_root_varobjs (invalidate_objfile_varobj_type_iter, objfile); > + > /* It still may reference data modules have associated with the objfile and > the symbol file data. */ > forget_cached_source_info_for_objfile (objfile); > diff --git a/gdb/varobj.c b/gdb/varobj.c > index 7446f10..2a563af 100644 > --- a/gdb/varobj.c > +++ b/gdb/varobj.c > @@ -2683,6 +2683,22 @@ varobj_floating_p (struct varobj *var) > return var->root->floating; > } > > +/* Get the valid flag of varobj VAR. */ > + > +int > +varobj_is_valid_p (struct varobj *var) > +{ > + return var->root->is_valid; > +} > + > +/* Clear the valid flag on varobj VAR. */ > + > +void > +varobj_set_invalid (struct varobj *var) > +{ > + var->root->is_valid = 0; > +} > + > /* Implement the "value_is_changeable_p" varobj callback for most > languages. */ > > @@ -2762,6 +2778,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused) > if (var->root->floating || var->root->valid_block == NULL) > { > struct varobj *tmp_var; > + char *tmp_var_value, *var_value; > > /* Try to create a varobj with same expression. If we succeed > replace the old varobj, otherwise invalidate it. */ > @@ -2770,6 +2787,26 @@ varobj_invalidate_iter (struct varobj *var, void *unused) > if (tmp_var != NULL) > { > tmp_var->obj_name = xstrdup (var->obj_name); > + tmp_var_value = varobj_get_value (tmp_var); > + var_value = varobj_get_value (var); > + > + /* As varobjs are re-evaluated during creation so there is a > + chance that new value is different from old one. Compare > + value of old varobj and newly created varobj and mark > + varobj updated If new value is different. */ > + if (var_value == NULL && tmp_var_value == NULL) > + ; /* Equal. */ > + else if (var_value == NULL || tmp_var_value == NULL) > + tmp_var->updated = 1; > + else > + { > + /* Mark tmp_var updated if new value is different. */ > + if (strcmp (tmp_var_value, var_value) != 0) > + tmp_var->updated = 1; > + } > + > + xfree (tmp_var_value); > + xfree (var_value); > varobj_delete (var, NULL, 0); > install_variable (tmp_var); > } > diff --git a/gdb/varobj.h b/gdb/varobj.h > index 74d41cf..7439a94 100644 > --- a/gdb/varobj.h > +++ b/gdb/varobj.h > @@ -299,6 +299,9 @@ extern int varobj_editable_p (struct varobj *var); > > extern int varobj_floating_p (struct varobj *var); > > +extern int varobj_is_valid_p (struct varobj *var); > +extern void varobj_set_invalid (struct varobj *var); > + > extern void varobj_set_visualizer (struct varobj *var, > const char *visualizer); > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PING][PATCH 1/2] Fix varobj updation after symbol removal 2014-06-27 10:14 ` [PATCH 1/2] Fix " Taimoor Mirza 2014-07-13 7:50 ` Taimoor @ 2014-08-05 11:00 ` Taimoor 2014-09-01 7:28 ` Taimoor 1 sibling, 1 reply; 14+ messages in thread From: Taimoor @ 2014-08-05 11:00 UTC (permalink / raw) To: gdb-patches Ping. On 06/27/2014 03:13 PM, Taimoor Mirza wrote: > This problem was observed while loading and unloading symbols using > add-symbol-file and remove-symbol-file. When remove-symbol-file > command is invoked, it calls clear_symtab_users that calls varobj_invalidate > to invalidate variable objects. This function invalidates the varobjs > that are tied to locals and re-create the ones that are defined on > globals. During this re-creation of globals, variable objects are > re-evaluated that can result in new value. But this change is not recorded > and because of this, -var-update for such modified variable objects > gives empty change list. > > Proposed Fix: > ============= > GDB has mechanism of marking varobj's updated if they are set via > varobj_set_value operation. This allows any next -var-update to report > this change. Same approach should be used during varobj invalidation. > If value of newly created varobj is different from previous one, mark it > updated so that -var-update can get this change. > > Variable object invalidation code is cleaned up to avoid using pointers > whose target has been already freed. > > Fix Testing: > =========== > This fix has been regression tested on both simulator and real boards. > > 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> > Maciej W. Rozycki <macro@codesourcery.com> > > gdb/ > * varobj.h (varobj_is_valid_p, varobj_set_invalid): New > prototypes. > * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions. > (varobj_invalidate_iter): Mark re-created global object updated > if its value is different from previous value. > * objfiles.c (invalidate_objfile_varobj_type_iter): New function. > (free_objfile): Call it. > > --- > gdb/objfiles.c | 19 +++++++++++++++++++ > gdb/varobj.c | 37 +++++++++++++++++++++++++++++++++++++ > gdb/varobj.h | 3 +++ > 3 files changed, 59 insertions(+) > > diff --git a/gdb/objfiles.c b/gdb/objfiles.c > index 0a0b1cb..03559a3 100644 > --- a/gdb/objfiles.c > +++ b/gdb/objfiles.c > @@ -32,6 +32,7 @@ > #include "bcache.h" > #include "expression.h" > #include "parser-defs.h" > +#include "varobj.h" > > #include "gdb_assert.h" > #include <sys/types.h> > @@ -538,6 +539,21 @@ free_objfile_separate_debug (struct objfile *objfile) > } > } > > +/* Mark the variable object VAR invalid if built upon a type coming from > + the objfile requested, passed as DATA. Also clear the type reference. */ > + > +static void > +invalidate_objfile_varobj_type_iter (struct varobj *var, void *data) > +{ > + struct objfile *objfile = data; > + > + if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile) > + { > + varobj_set_invalid (var); > + var->type = NULL; > + } > +} > + > /* Destroy an objfile and all the symtabs and psymtabs under it. */ > > void > @@ -584,6 +600,9 @@ free_objfile (struct objfile *objfile) > lists. */ > preserve_values (objfile); > > + /* Varobj may refer to types stored in objfile's obstack. */ > + all_root_varobjs (invalidate_objfile_varobj_type_iter, objfile); > + > /* It still may reference data modules have associated with the objfile and > the symbol file data. */ > forget_cached_source_info_for_objfile (objfile); > diff --git a/gdb/varobj.c b/gdb/varobj.c > index 7446f10..2a563af 100644 > --- a/gdb/varobj.c > +++ b/gdb/varobj.c > @@ -2683,6 +2683,22 @@ varobj_floating_p (struct varobj *var) > return var->root->floating; > } > > +/* Get the valid flag of varobj VAR. */ > + > +int > +varobj_is_valid_p (struct varobj *var) > +{ > + return var->root->is_valid; > +} > + > +/* Clear the valid flag on varobj VAR. */ > + > +void > +varobj_set_invalid (struct varobj *var) > +{ > + var->root->is_valid = 0; > +} > + > /* Implement the "value_is_changeable_p" varobj callback for most > languages. */ > > @@ -2762,6 +2778,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused) > if (var->root->floating || var->root->valid_block == NULL) > { > struct varobj *tmp_var; > + char *tmp_var_value, *var_value; > > /* Try to create a varobj with same expression. If we succeed > replace the old varobj, otherwise invalidate it. */ > @@ -2770,6 +2787,26 @@ varobj_invalidate_iter (struct varobj *var, void *unused) > if (tmp_var != NULL) > { > tmp_var->obj_name = xstrdup (var->obj_name); > + tmp_var_value = varobj_get_value (tmp_var); > + var_value = varobj_get_value (var); > + > + /* As varobjs are re-evaluated during creation so there is a > + chance that new value is different from old one. Compare > + value of old varobj and newly created varobj and mark > + varobj updated If new value is different. */ > + if (var_value == NULL && tmp_var_value == NULL) > + ; /* Equal. */ > + else if (var_value == NULL || tmp_var_value == NULL) > + tmp_var->updated = 1; > + else > + { > + /* Mark tmp_var updated if new value is different. */ > + if (strcmp (tmp_var_value, var_value) != 0) > + tmp_var->updated = 1; > + } > + > + xfree (tmp_var_value); > + xfree (var_value); > varobj_delete (var, NULL, 0); > install_variable (tmp_var); > } > diff --git a/gdb/varobj.h b/gdb/varobj.h > index 74d41cf..7439a94 100644 > --- a/gdb/varobj.h > +++ b/gdb/varobj.h > @@ -299,6 +299,9 @@ extern int varobj_editable_p (struct varobj *var); > > extern int varobj_floating_p (struct varobj *var); > > +extern int varobj_is_valid_p (struct varobj *var); > +extern void varobj_set_invalid (struct varobj *var); > + > extern void varobj_set_visualizer (struct varobj *var, > const char *visualizer); > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PING][PATCH 1/2] Fix varobj updation after symbol removal 2014-08-05 11:00 ` [PING][PATCH " Taimoor @ 2014-09-01 7:28 ` Taimoor 0 siblings, 0 replies; 14+ messages in thread From: Taimoor @ 2014-09-01 7:28 UTC (permalink / raw) To: gdb-patches Ping On 08/05/2014 04:00 PM, Taimoor wrote: > > Ping. > > On 06/27/2014 03:13 PM, Taimoor Mirza wrote: >> This problem was observed while loading and unloading symbols using >> add-symbol-file and remove-symbol-file. When remove-symbol-file >> command is invoked, it calls clear_symtab_users that calls >> varobj_invalidate >> to invalidate variable objects. This function invalidates the varobjs >> that are tied to locals and re-create the ones that are defined on >> globals. During this re-creation of globals, variable objects are >> re-evaluated that can result in new value. But this change is not >> recorded >> and because of this, -var-update for such modified variable objects >> gives empty change list. >> >> Proposed Fix: >> ============= >> GDB has mechanism of marking varobj's updated if they are set via >> varobj_set_value operation. This allows any next -var-update to report >> this change. Same approach should be used during varobj invalidation. >> If value of newly created varobj is different from previous one, mark it >> updated so that -var-update can get this change. >> >> Variable object invalidation code is cleaned up to avoid using pointers >> whose target has been already freed. >> >> Fix Testing: >> =========== >> This fix has been regression tested on both simulator and real boards. >> >> 2014-06-27 Taimoor Mirza <tmirza@codesourcery.com> >> Maciej W. Rozycki <macro@codesourcery.com> >> >> gdb/ >> * varobj.h (varobj_is_valid_p, varobj_set_invalid): New >> prototypes. >> * varobj.c (varobj_is_valid_p, varobj_set_invalid): New >> functions. >> (varobj_invalidate_iter): Mark re-created global object updated >> if its value is different from previous value. >> * objfiles.c (invalidate_objfile_varobj_type_iter): New >> function. >> (free_objfile): Call it. >> >> --- >> gdb/objfiles.c | 19 +++++++++++++++++++ >> gdb/varobj.c | 37 +++++++++++++++++++++++++++++++++++++ >> gdb/varobj.h | 3 +++ >> 3 files changed, 59 insertions(+) >> >> diff --git a/gdb/objfiles.c b/gdb/objfiles.c >> index 0a0b1cb..03559a3 100644 >> --- a/gdb/objfiles.c >> +++ b/gdb/objfiles.c >> @@ -32,6 +32,7 @@ >> #include "bcache.h" >> #include "expression.h" >> #include "parser-defs.h" >> +#include "varobj.h" >> >> #include "gdb_assert.h" >> #include <sys/types.h> >> @@ -538,6 +539,21 @@ free_objfile_separate_debug (struct objfile >> *objfile) >> } >> } >> >> +/* Mark the variable object VAR invalid if built upon a type coming from >> + the objfile requested, passed as DATA. Also clear the type >> reference. */ >> + >> +static void >> +invalidate_objfile_varobj_type_iter (struct varobj *var, void *data) >> +{ >> + struct objfile *objfile = data; >> + >> + if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile) >> + { >> + varobj_set_invalid (var); >> + var->type = NULL; >> + } >> +} >> + >> /* Destroy an objfile and all the symtabs and psymtabs under it. */ >> >> void >> @@ -584,6 +600,9 @@ free_objfile (struct objfile *objfile) >> lists. */ >> preserve_values (objfile); >> >> + /* Varobj may refer to types stored in objfile's obstack. */ >> + all_root_varobjs (invalidate_objfile_varobj_type_iter, objfile); >> + >> /* It still may reference data modules have associated with the >> objfile and >> the symbol file data. */ >> forget_cached_source_info_for_objfile (objfile); >> diff --git a/gdb/varobj.c b/gdb/varobj.c >> index 7446f10..2a563af 100644 >> --- a/gdb/varobj.c >> +++ b/gdb/varobj.c >> @@ -2683,6 +2683,22 @@ varobj_floating_p (struct varobj *var) >> return var->root->floating; >> } >> >> +/* Get the valid flag of varobj VAR. */ >> + >> +int >> +varobj_is_valid_p (struct varobj *var) >> +{ >> + return var->root->is_valid; >> +} >> + >> +/* Clear the valid flag on varobj VAR. */ >> + >> +void >> +varobj_set_invalid (struct varobj *var) >> +{ >> + var->root->is_valid = 0; >> +} >> + >> /* Implement the "value_is_changeable_p" varobj callback for most >> languages. */ >> >> @@ -2762,6 +2778,7 @@ varobj_invalidate_iter (struct varobj *var, void >> *unused) >> if (var->root->floating || var->root->valid_block == NULL) >> { >> struct varobj *tmp_var; >> + char *tmp_var_value, *var_value; >> >> /* Try to create a varobj with same expression. If we succeed >> replace the old varobj, otherwise invalidate it. */ >> @@ -2770,6 +2787,26 @@ varobj_invalidate_iter (struct varobj *var, >> void *unused) >> if (tmp_var != NULL) >> { >> tmp_var->obj_name = xstrdup (var->obj_name); >> + tmp_var_value = varobj_get_value (tmp_var); >> + var_value = varobj_get_value (var); >> + >> + /* As varobjs are re-evaluated during creation so there is a >> + chance that new value is different from old one. Compare >> + value of old varobj and newly created varobj and mark >> + varobj updated If new value is different. */ >> + if (var_value == NULL && tmp_var_value == NULL) >> + ; /* Equal. */ >> + else if (var_value == NULL || tmp_var_value == NULL) >> + tmp_var->updated = 1; >> + else >> + { >> + /* Mark tmp_var updated if new value is different. */ >> + if (strcmp (tmp_var_value, var_value) != 0) >> + tmp_var->updated = 1; >> + } >> + >> + xfree (tmp_var_value); >> + xfree (var_value); >> varobj_delete (var, NULL, 0); >> install_variable (tmp_var); >> } >> diff --git a/gdb/varobj.h b/gdb/varobj.h >> index 74d41cf..7439a94 100644 >> --- a/gdb/varobj.h >> +++ b/gdb/varobj.h >> @@ -299,6 +299,9 @@ extern int varobj_editable_p (struct varobj *var); >> >> extern int varobj_floating_p (struct varobj *var); >> >> +extern int varobj_is_valid_p (struct varobj *var); >> +extern void varobj_set_invalid (struct varobj *var); >> + >> extern void varobj_set_visualizer (struct varobj *var, >> const char *visualizer); >> >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] Improved variable object invalidation in GDB 2014-06-27 10:14 [PATCH 0/2] Improved variable object invalidation in GDB Taimoor Mirza 2014-06-27 10:14 ` [PATCH 2/2] Testsuite for varobj updation after symbol removal Taimoor Mirza 2014-06-27 10:14 ` [PATCH 1/2] Fix " Taimoor Mirza @ 2014-07-13 7:49 ` Taimoor 2014-08-05 11:00 ` [PING][PATCH " Taimoor 3 siblings, 0 replies; 14+ messages in thread From: Taimoor @ 2014-07-13 7:49 UTC (permalink / raw) To: gdb-patches ping. Can anyone kindly review this patch series. Thanks, Taimoor On 06/27/2014 03:13 PM, Taimoor Mirza wrote: > Hi, > This patch series improves variable object invalidation in GDB. > This patch has been regression tested on ppc-gnu-linux > and i686 targets using both simulator and real boards. > > > Taimoor Mirza (2): > Fix varobj updation after symbol removal > Testsuite for varobj updation after symbol removal > > gdb/objfiles.c | 19 ++ > gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ > gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ > gdb/testsuite/gdb.mi/sym-file-loader.c | 353 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ > gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ > gdb/varobj.c | 37 +++ > gdb/varobj.h | 3 + > 8 files changed, 688 insertions(+) > create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h > create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PING][PATCH 0/2] Improved variable object invalidation in GDB 2014-06-27 10:14 [PATCH 0/2] Improved variable object invalidation in GDB Taimoor Mirza ` (2 preceding siblings ...) 2014-07-13 7:49 ` [PATCH 0/2] Improved variable object invalidation in GDB Taimoor @ 2014-08-05 11:00 ` Taimoor 2014-09-01 7:28 ` Taimoor 3 siblings, 1 reply; 14+ messages in thread From: Taimoor @ 2014-08-05 11:00 UTC (permalink / raw) To: gdb-patches Ping. On 06/27/2014 03:13 PM, Taimoor Mirza wrote: > Hi, > This patch series improves variable object invalidation in GDB. > This patch has been regression tested on ppc-gnu-linux > and i686 targets using both simulator and real boards. > > > Taimoor Mirza (2): > Fix varobj updation after symbol removal > Testsuite for varobj updation after symbol removal > > gdb/objfiles.c | 19 ++ > gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ > gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ > gdb/testsuite/gdb.mi/sym-file-loader.c | 353 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ > gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ > gdb/varobj.c | 37 +++ > gdb/varobj.h | 3 + > 8 files changed, 688 insertions(+) > create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c > create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h > create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PING][PATCH 0/2] Improved variable object invalidation in GDB 2014-08-05 11:00 ` [PING][PATCH " Taimoor @ 2014-09-01 7:28 ` Taimoor 0 siblings, 0 replies; 14+ messages in thread From: Taimoor @ 2014-09-01 7:28 UTC (permalink / raw) To: gdb-patches Hi, Ping. Regards, Taimoor On 08/05/2014 03:59 PM, Taimoor wrote: > > Ping. > > On 06/27/2014 03:13 PM, Taimoor Mirza wrote: >> Hi, >> This patch series improves variable object invalidation in GDB. >> This patch has been regression tested on ppc-gnu-linux >> and i686 targets using both simulator and real boards. >> >> >> Taimoor Mirza (2): >> Fix varobj updation after symbol removal >> Testsuite for varobj updation after symbol removal >> >> gdb/objfiles.c | 19 ++ >> gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ >> gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ >> gdb/testsuite/gdb.mi/sym-file-loader.c | 353 >> ++++++++++++++++++++++++++++ >> gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ >> gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ >> gdb/varobj.c | 37 +++ >> gdb/varobj.h | 3 + >> 8 files changed, 688 insertions(+) >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h >> create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] Improved variable object invalidation in GDB @ 2015-04-16 3:33 Taimoor Mirza 2015-04-16 3:33 ` [PATCH 1/2] Fix varobj updation after symbol removal Taimoor Mirza 0 siblings, 1 reply; 14+ messages in thread From: Taimoor Mirza @ 2015-04-16 3:33 UTC (permalink / raw) To: gdb-patches; +Cc: Taimoor Mirza Hi, This patch series improves variable object invalidation in GDB. This patch has been regression tested on ppc-gnu-linux and i686 targets using both simulator and real boards. Taimoor Mirza (2): Fix varobj updation after symbol removal Testsuite for varobj updation after symbol removal gdb/objfiles.c | 19 ++ gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 67 ++++++ gdb/testsuite/gdb.mi/sym-file-lib.c | 26 ++ gdb/testsuite/gdb.mi/sym-file-loader.c | 353 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/sym-file-loader.h | 99 ++++++++ gdb/testsuite/gdb.mi/sym-file-main.c | 84 +++++++ gdb/varobj.c | 37 +++ gdb/varobj.h | 3 + 8 files changed, 688 insertions(+) create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c -- 1.7.9.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] Fix varobj updation after symbol removal 2015-04-16 3:33 [PATCH " Taimoor Mirza @ 2015-04-16 3:33 ` Taimoor Mirza 0 siblings, 0 replies; 14+ messages in thread From: Taimoor Mirza @ 2015-04-16 3:33 UTC (permalink / raw) To: gdb-patches; +Cc: Taimoor Mirza This problem was observed while loading and unloading symbols using add-symbol-file and remove-symbol-file. When remove-symbol-file command is invoked, it calls clear_symtab_users that calls varobj_invalidate to invalidate variable objects. This function invalidates the varobjs that are tied to locals and re-create the ones that are defined on globals. During this re-creation of globals, variable objects are re-evaluated that can result in new value. But this change is not recorded and because of this, -var-update for such modified variable objects gives empty change list. Proposed Fix: ============= GDB has mechanism of marking varobj's updated if they are set via varobj_set_value operation. This allows any next -var-update to report this change. Same approach should be used during varobj invalidation. If value of newly created varobj is different from previous one, mark it updated so that -var-update can get this change. Variable object invalidation code is cleaned up to avoid using pointers whose target has been already freed. Fix Testing: =========== This fix has been regression tested on both simulator and real boards. 2015-04-15 Taimoor Mirza <tmirza@codesourcery.com> Maciej W. Rozycki <macro@codesourcery.com> gdb/ * varobj.h (varobj_is_valid_p, varobj_set_invalid): New prototypes. * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions. (varobj_invalidate_iter): Mark re-created global object updated if its value is different from previous value. * objfiles.c (invalidate_objfile_varobj_type_iter): New function. (free_objfile): Call it. Signed-off-by: Taimoor Mirza <tmirza@codesourcery.com> --- gdb/objfiles.c | 19 +++++++++++++++++++ gdb/varobj.c | 37 +++++++++++++++++++++++++++++++++++++ gdb/varobj.h | 3 +++ 3 files changed, 59 insertions(+) diff --git a/gdb/objfiles.c b/gdb/objfiles.c index ff20bc8..5905998 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -32,6 +32,7 @@ #include "bcache.h" #include "expression.h" #include "parser-defs.h" +#include "varobj.h" #include <sys/types.h> #include <sys/stat.h> @@ -533,6 +534,21 @@ free_objfile_separate_debug (struct objfile *objfile) } } +/* Mark the variable object VAR invalid if built upon a type coming from + the objfile requested, passed as DATA. Also clear the type reference. */ + +static void +invalidate_objfile_varobj_type_iter (struct varobj *var, void *data) +{ + struct objfile *objfile = data; + + if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile) + { + varobj_set_invalid (var); + var->type = NULL; + } +} + /* Destroy an objfile and all the symtabs and psymtabs under it. */ void @@ -579,6 +595,9 @@ free_objfile (struct objfile *objfile) lists. */ preserve_values (objfile); + /* Varobj may refer to types stored in objfile's obstack. */ + all_root_varobjs (invalidate_objfile_varobj_type_iter, objfile); + /* It still may reference data modules have associated with the objfile and the symbol file data. */ forget_cached_source_info_for_objfile (objfile); diff --git a/gdb/varobj.c b/gdb/varobj.c index b220fd8..4860a37 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -2699,6 +2699,22 @@ varobj_floating_p (const struct varobj *var) return var->root->floating; } +/* Get the valid flag of varobj VAR. */ + +int +varobj_is_valid_p (struct varobj *var) +{ + return var->root->is_valid; +} + +/* Clear the valid flag on varobj VAR. */ + +void +varobj_set_invalid (struct varobj *var) +{ + var->root->is_valid = 0; +} + /* Implement the "value_is_changeable_p" varobj callback for most languages. */ @@ -2760,6 +2776,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused) if (var->root->floating || var->root->valid_block == NULL) { struct varobj *tmp_var; + char *tmp_var_value, *var_value; /* Try to create a varobj with same expression. If we succeed replace the old varobj, otherwise invalidate it. */ @@ -2768,6 +2785,26 @@ varobj_invalidate_iter (struct varobj *var, void *unused) if (tmp_var != NULL) { tmp_var->obj_name = xstrdup (var->obj_name); + tmp_var_value = varobj_get_value (tmp_var); + var_value = varobj_get_value (var); + + /* As varobjs are re-evaluated during creation so there is a + chance that new value is different from old one. Compare + value of old varobj and newly created varobj and mark + varobj updated If new value is different. */ + if (var_value == NULL && tmp_var_value == NULL) + ; /* Equal. */ + else if (var_value == NULL || tmp_var_value == NULL) + tmp_var->updated = 1; + else + { + /* Mark tmp_var updated if new value is different. */ + if (strcmp (tmp_var_value, var_value) != 0) + tmp_var->updated = 1; + } + + xfree (tmp_var_value); + xfree (var_value); varobj_delete (var, NULL, 0); install_variable (tmp_var); } diff --git a/gdb/varobj.h b/gdb/varobj.h index 8860526..4afb9e8 100644 --- a/gdb/varobj.h +++ b/gdb/varobj.h @@ -311,6 +311,9 @@ extern int varobj_editable_p (const struct varobj *var); extern int varobj_floating_p (const struct varobj *var); +extern int varobj_is_valid_p (struct varobj *var); +extern void varobj_set_invalid (struct varobj *var); + extern void varobj_set_visualizer (struct varobj *var, const char *visualizer); -- 1.7.9.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/2] Improved variable object invalidation in GDB @ 2019-10-17 10:03 Raza, Saqlain 2019-10-17 10:03 ` [PATCH 1/2] Fix varobj updation after symbol removal Raza, Saqlain 0 siblings, 1 reply; 14+ messages in thread From: Raza, Saqlain @ 2019-10-17 10:03 UTC (permalink / raw) To: gdb-patches; +Cc: taimoor_mirza, Raza, Saqlain Hi, This patch series improves variable object invalidation in GDB. This is a followup to the patch series submission made in https://sourceware.org/ml/gdb-patches/2015-04/msg00598.html . This problem still holds in the latest GDB master. Raza, Saqlain (2): Fix varobj updation after symbol removal Testsuite for varobj updation after symbol removal gdb/ChangeLog | 13 ++ gdb/objfiles.c | 19 ++ gdb/testsuite/ChangeLog | 11 + gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 68 ++++++ gdb/testsuite/gdb.mi/sym-file-lib.c | 28 +++ gdb/testsuite/gdb.mi/sym-file-loader.c | 355 +++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/sym-file-loader.h | 101 ++++++++ gdb/testsuite/gdb.mi/sym-file-main.c | 86 +++++++ gdb/varobj.c | 35 +++ gdb/varobj.h | 4 + 10 files changed, 720 insertions(+) create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c -- 2.8.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] Fix varobj updation after symbol removal 2019-10-17 10:03 [PATCH 0/2] Improved variable object invalidation in GDB Raza, Saqlain @ 2019-10-17 10:03 ` Raza, Saqlain 0 siblings, 0 replies; 14+ messages in thread From: Raza, Saqlain @ 2019-10-17 10:03 UTC (permalink / raw) To: gdb-patches; +Cc: taimoor_mirza, Raza, Saqlain, Raza This problem was observed while loading and unloading symbols using add-symbol-file and remove-symbol-file. When remove-symbol-file command is invoked, it calls clear_symtab_users that calls varobj_invalidate to invalidate variable objects. This function invalidates the varobjs that are tied to locals and re-create the ones that are defined on globals. During this re-creation of globals, variable objects are re-evaluated that can result in new value. But this change is not recorded and because of this, -var-update for such modified variable objects gives empty change list. Proposed Fix: ============= GDB has mechanism of marking varobj's updated if they are set via varobj_set_value operation. This allows any next -var-update to report this change. Same approach should be used during varobj invalidation. If value of newly created varobj is different from previous one, mark it updated so that -var-update can get this change. Variable object invalidation code is cleaned up to avoid using pointers whose target has been already freed. Fix Testing: =========== This fix has been regression tested on both simulator and real boards for x86_64 and arm-none-linux-gnueabi targets. 2019-10-17 Saqlain Raza <saqlain_raza@mentor.com> Taimoor Mirza <taimoor_mirza@mentor.com> Maciej W. Rozycki <macro@codesourcery.com> gdb/ * objfiles.c: Include varobj.h. (invalidate_objfile_varobj_type_iter): New function. (free_objfile): Call it. * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions. (varobj_invalidate_iter): Mark re-created global object updated if its value is different from previous value. * varobj.h (varobj_is_valid_p, varobj_set_invalid): New prototypes. Signed-off-by: Raza, Saqlain <Saqlain_Raza@mentor.com> --- gdb/ChangeLog | 13 +++++++++++++ gdb/objfiles.c | 19 +++++++++++++++++++ gdb/varobj.c | 35 +++++++++++++++++++++++++++++++++++ gdb/varobj.h | 4 ++++ 4 files changed, 71 insertions(+) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ced508f..b92da1b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2019-10-17 Saqlain Raza <saqlain_raza@mentor.com> + Taimoor Mirza <taimoor_mirza@mentor.com> + Maciej W. Rozycki <macro@codesourcery.com> + + * objfiles.c: Include varobj.h. + (invalidate_objfile_varobj_type_iter): New function. + (free_objfile): Call it. + * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions. + (varobj_invalidate_iter): Mark re-created global object updated + if its value is different from previous value. + * varobj.h (varobj_is_valid_p, varobj_set_invalid): New + prototypes. + 2019-10-16 Tom Tromey <tom@tromey.com> * objfiles.h (struct objfile) <original_name>: Now const. diff --git a/gdb/objfiles.c b/gdb/objfiles.c index f9e7d20..8704814 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -32,6 +32,7 @@ #include "bcache.h" #include "expression.h" #include "parser-defs.h" +#include "varobj.h" #include <sys/types.h> #include <sys/stat.h> @@ -568,6 +569,21 @@ free_objfile_separate_debug (struct objfile *objfile) } } +/* Mark the variable object VAR invalid if built upon a type coming from + the objfile requested, passed as DATA. Also clear the type reference. */ + +static void +invalidate_objfile_varobj_type_iter (struct varobj *var, void *data) +{ + struct objfile *objfile = (struct objfile*)data; + + if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile) + { + varobj_set_invalid (var); + var->type = NULL; + } +} + /* Destroy an objfile and all the symtabs and psymtabs under it. */ objfile::~objfile () @@ -613,6 +629,9 @@ objfile::~objfile () lists. */ preserve_values (this); + /* Varobj may refer to types stored in objfile's obstack. */ + all_root_varobjs (invalidate_objfile_varobj_type_iter, this); + /* It still may reference data modules have associated with the objfile and the symbol file data. */ forget_cached_source_info_for_objfile (this); diff --git a/gdb/varobj.c b/gdb/varobj.c index 37a522b..f7696e7 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -2431,6 +2431,22 @@ varobj_floating_p (const struct varobj *var) return var->root->floating; } +/* Get the valid flag of varobj VAR. */ + +int +varobj_is_valid_p (struct varobj *var) +{ + return var->root->is_valid; +} + +/* Clear the valid flag on varobj VAR. */ + +void +varobj_set_invalid (struct varobj *var) +{ + var->root->is_valid = 0; +} + /* Implement the "value_is_changeable_p" varobj callback for most languages. */ @@ -2492,6 +2508,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused) if (var->root->floating || var->root->valid_block == NULL) { struct varobj *tmp_var; + std::string tmp_var_value, var_value; /* Try to create a varobj with same expression. If we succeed replace the old varobj, otherwise invalidate it. */ @@ -2500,6 +2517,24 @@ varobj_invalidate_iter (struct varobj *var, void *unused) if (tmp_var != NULL) { tmp_var->obj_name = var->obj_name; + tmp_var_value = varobj_get_value (tmp_var); + var_value = varobj_get_value (var); + + /* Since varobjs are re-evaluated during creation, there is a + chance the new value is different from old one. Compare + old varobj and the newly created varobj and mark varobj + updated ff new value is different. */ + if (var_value.empty() && tmp_var_value.empty()) + ; /* Equal. */ + else if (var_value.empty() || tmp_var_value.empty()) + tmp_var->updated = 1; + else + { + /* Mark tmp_var updated if the new value is different. */ + if (tmp_var_value != var_value) + tmp_var->updated = 1; + } + varobj_delete (var, 0); install_variable (tmp_var); } diff --git a/gdb/varobj.h b/gdb/varobj.h index 66db780..0885f70 100644 --- a/gdb/varobj.h +++ b/gdb/varobj.h @@ -323,6 +323,10 @@ extern bool varobj_editable_p (const struct varobj *var); extern bool varobj_floating_p (const struct varobj *var); +extern int varobj_is_valid_p (struct varobj *var); + +extern void varobj_set_invalid (struct varobj *var); + extern void varobj_set_visualizer (struct varobj *var, const char *visualizer); -- 2.8.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-10-17 10:03 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-27 10:14 [PATCH 0/2] Improved variable object invalidation in GDB Taimoor Mirza 2014-06-27 10:14 ` [PATCH 2/2] Testsuite for varobj updation after symbol removal Taimoor Mirza 2014-07-13 16:11 ` Taimoor 2014-08-05 11:01 ` [PING][PATCH " Taimoor 2014-09-01 7:29 ` Taimoor 2014-06-27 10:14 ` [PATCH 1/2] Fix " Taimoor Mirza 2014-07-13 7:50 ` Taimoor 2014-08-05 11:00 ` [PING][PATCH " Taimoor 2014-09-01 7:28 ` Taimoor 2014-07-13 7:49 ` [PATCH 0/2] Improved variable object invalidation in GDB Taimoor 2014-08-05 11:00 ` [PING][PATCH " Taimoor 2014-09-01 7:28 ` Taimoor 2015-04-16 3:33 [PATCH " Taimoor Mirza 2015-04-16 3:33 ` [PATCH 1/2] Fix varobj updation after symbol removal Taimoor Mirza 2019-10-17 10:03 [PATCH 0/2] Improved variable object invalidation in GDB Raza, Saqlain 2019-10-17 10:03 ` [PATCH 1/2] Fix varobj updation after symbol removal Raza, Saqlain
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox