* [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 ` [PATCH 1/2] Fix varobj updation after symbol removal Taimoor Mirza
@ 2014-06-27 10:14 ` Taimoor Mirza
2014-07-13 16:11 ` Taimoor
2014-08-05 11:01 ` [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; 13+ 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] 13+ messages in thread
* [PATCH 0/2] Improved variable object invalidation in GDB
@ 2014-06-27 10:14 Taimoor Mirza
2014-06-27 10:14 ` [PATCH 1/2] Fix varobj updation after symbol removal Taimoor Mirza
` (3 more replies)
0 siblings, 4 replies; 13+ 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] 13+ 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 ` Taimoor Mirza
2014-07-13 7:50 ` Taimoor
2014-08-05 11:00 ` [PING][PATCH " Taimoor
2014-06-27 10:14 ` [PATCH 2/2] Testsuite for " Taimoor Mirza
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ 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] 13+ 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 1/2] Fix varobj updation after symbol removal Taimoor Mirza
2014-06-27 10:14 ` [PATCH 2/2] Testsuite for " Taimoor Mirza
@ 2014-07-13 7:49 ` Taimoor
2014-08-05 11:00 ` [PING][PATCH " Taimoor
3 siblings, 0 replies; 13+ 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] 13+ messages in thread
* Re: [PATCH 1/2] Fix varobj updation after symbol removal
2014-06-27 10:14 ` [PATCH 1/2] Fix varobj updation after symbol removal Taimoor Mirza
@ 2014-07-13 7:50 ` Taimoor
2014-08-05 11:00 ` [PING][PATCH " Taimoor
1 sibling, 0 replies; 13+ 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] 13+ messages in thread
* Re: [PATCH 2/2] Testsuite for varobj updation after symbol removal
2014-06-27 10:14 ` [PATCH 2/2] Testsuite for " Taimoor Mirza
@ 2014-07-13 16:11 ` Taimoor
2014-08-05 11:01 ` [PING][PATCH " Taimoor
1 sibling, 0 replies; 13+ 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] 13+ messages in thread
* [PING][PATCH 1/2] Fix varobj updation after symbol removal
2014-06-27 10:14 ` [PATCH 1/2] Fix varobj updation after symbol removal Taimoor Mirza
2014-07-13 7:50 ` Taimoor
@ 2014-08-05 11:00 ` Taimoor
2014-09-01 7:28 ` Taimoor
1 sibling, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ messages in thread
* [PING][PATCH 2/2] Testsuite for varobj updation after symbol removal
2014-06-27 10:14 ` [PATCH 2/2] Testsuite for " Taimoor Mirza
2014-07-13 16:11 ` Taimoor
@ 2014-08-05 11:01 ` Taimoor
2014-09-01 7:29 ` Taimoor
1 sibling, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ messages in thread
* Re: [PING][PATCH 1/2] Fix varobj updation after symbol removal
2019-10-17 10:03 ` [PATCH 1/2] Fix varobj updation after symbol removal Raza, Saqlain
@ 2019-10-22 6:46 ` Saqlain Raza
0 siblings, 0 replies; 13+ messages in thread
From: Saqlain Raza @ 2019-10-22 6:46 UTC (permalink / raw)
To: gdb-patches; +Cc: taimoor_mirza, Raza
Ping !
Thanks,
Saqlain
On 10/17/19 3:03 PM, Raza, Saqlain 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
> 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);
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-22 6:46 UTC | newest]
Thread overview: 13+ 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 1/2] Fix varobj updation after symbol removal Taimoor Mirza
2014-07-13 7:50 ` Taimoor
2014-08-05 11:00 ` [PING][PATCH " Taimoor
2014-09-01 7:28 ` Taimoor
2014-06-27 10:14 ` [PATCH 2/2] Testsuite for " Taimoor Mirza
2014-07-13 16:11 ` Taimoor
2014-08-05 11:01 ` [PING][PATCH " Taimoor
2014-09-01 7:29 ` 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
2019-10-17 10:03 [PATCH " Raza, Saqlain
2019-10-17 10:03 ` [PATCH 1/2] Fix varobj updation after symbol removal Raza, Saqlain
2019-10-22 6:46 ` [PING][PATCH " Saqlain Raza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).