[PATCH 0/2] Improved variable object invalidation in GDB

[PATCH 0/2] Improved variable object invalidation in GDB

[Raza, Saqlain]

Hi,
This patch series improves variable object invalidation in GDB.

This is a followup to the patch series submission made in https://sourceware.org/ml/gdb-patches/2015-04/msg00598.html . This problem still holds in the latest GDB master.

Raza, Saqlain (2):
  Fix varobj updation after symbol removal
  Testsuite for varobj updation after symbol removal

 gdb/ChangeLog                              |  13 ++
 gdb/objfiles.c                             |  19 ++
 gdb/testsuite/ChangeLog                    |  11 +
 gdb/testsuite/gdb.mi/mi-var-invalidate.exp |  68 ++++++
 gdb/testsuite/gdb.mi/sym-file-lib.c        |  28 +++
 gdb/testsuite/gdb.mi/sym-file-loader.c     | 355 +++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/sym-file-loader.h     | 101 ++++++++
 gdb/testsuite/gdb.mi/sym-file-main.c       |  86 +++++++
 gdb/varobj.c                               |  35 +++
 gdb/varobj.h                               |   4 +
 10 files changed, 720 insertions(+)
 create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c
 create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c
 create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h
 create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c

-- 
2.8.1

[PATCH 2/2] Testsuite for varobj updation after symbol removal

[Raza, Saqlain]

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 and arm-none-linux-gnueabi targets
using both simulator and real boards.

2019-10-17  Saqlain Raza  <saqlain_raza@mentor.com>
            Taimoor Mirza  <taimoor_mirza@mentor.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.

Signed-off-by: Raza, Saqlain <Saqlain_Raza@mentor.com>
---
 gdb/testsuite/ChangeLog                    |  11 +
 gdb/testsuite/gdb.mi/mi-var-invalidate.exp |  68 ++++++
 gdb/testsuite/gdb.mi/sym-file-lib.c        |  28 +++
 gdb/testsuite/gdb.mi/sym-file-loader.c     | 355 +++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/sym-file-loader.h     | 101 ++++++++
 gdb/testsuite/gdb.mi/sym-file-main.c       |  86 +++++++
 6 files changed, 649 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/ChangeLog b/gdb/testsuite/ChangeLog
index 395a3cc..82c3a57 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,14 @@
+2019-10-17  Saqlain Raza  <saqlain_raza@mentor.com>
+	    Taimoor Mirza  <taimoor_mirza@mentor.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.
+
 2019-10-16  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.fortran/module.exp: Extend with 'info variables' test.
diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp
index e390968..b06e713 100644
--- a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp
@@ -120,6 +120,74 @@ mi_gdb_test "-var-update global_simple" \
 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
 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..86c719c
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/sym-file-lib.c
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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..0a3cd32
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/sym-file-loader.c
@@ -0,0 +1,355 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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..d6ebb39
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/sym-file-loader.h
@@ -0,0 +1,101 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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..ecdef4e
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/sym-file-main.c
@@ -0,0 +1,86 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 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;
+}
-- 
2.8.1

[PATCH 1/2] Fix varobj updation after symbol removal

[Raza, Saqlain]

This problem was observed while loading and unloading symbols using 
add-symbol-file and remove-symbol-file. When remove-symbol-file 
command is invoked, it calls clear_symtab_users that calls varobj_invalidate 
to invalidate variable objects. This function invalidates the varobjs 
that are tied to locals and re-create the ones that are defined on 
globals. During this re-creation of globals, variable objects are 
re-evaluated that can result in new value. But this change is not recorded
and because of this, -var-update for such modified variable objects
gives empty change list.

Proposed Fix:
=============
GDB has mechanism of marking varobj's updated if they are set via 
varobj_set_value operation. This allows any next -var-update to report 
this change. Same approach should be used during varobj invalidation.
If value of newly created varobj is different from previous one, mark it 
updated so that -var-update can get this change.

Variable object invalidation code is cleaned up to avoid using pointers 
whose target has been already freed.

Fix Testing:
===========
This fix has been regression tested on both simulator and real boards
for x86_64 and arm-none-linux-gnueabi targets.

2019-10-17  Saqlain Raza  <saqlain_raza@mentor.com>
            Taimoor Mirza  <taimoor_mirza@mentor.com>
            Maciej W. Rozycki  <macro@codesourcery.com>

        gdb/
        * objfiles.c: Include varobj.h.
        (invalidate_objfile_varobj_type_iter): New function.
        (free_objfile): Call it.
        * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions.
        (varobj_invalidate_iter): Mark re-created global object updated
        if its value is different from previous value.
        * varobj.h (varobj_is_valid_p, varobj_set_invalid): New
        prototypes.

Signed-off-by: Raza, Saqlain <Saqlain_Raza@mentor.com>
---
 gdb/ChangeLog  | 13 +++++++++++++
 gdb/objfiles.c | 19 +++++++++++++++++++
 gdb/varobj.c   | 35 +++++++++++++++++++++++++++++++++++
 gdb/varobj.h   |  4 ++++
 4 files changed, 71 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ced508f..b92da1b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2019-10-17  Saqlain Raza  <saqlain_raza@mentor.com>
+	    Taimoor Mirza  <taimoor_mirza@mentor.com>
+	    Maciej W. Rozycki  <macro@codesourcery.com>
+
+	* objfiles.c: Include varobj.h.
+	(invalidate_objfile_varobj_type_iter): New function.
+	(free_objfile): Call it.
+	* varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions.
+	(varobj_invalidate_iter): Mark re-created global object updated
+	if its value is different from previous value.
+	* varobj.h (varobj_is_valid_p, varobj_set_invalid): New
+	prototypes.
+
 2019-10-16  Tom Tromey  <tom@tromey.com>
 
 	* objfiles.h (struct objfile) <original_name>: Now const.
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index f9e7d20..8704814 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -32,6 +32,7 @@
 #include "bcache.h"
 #include "expression.h"
 #include "parser-defs.h"
+#include "varobj.h"
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -568,6 +569,21 @@ free_objfile_separate_debug (struct objfile *objfile)
     }
 }
 
+/* Mark the variable object VAR invalid if built upon a type coming from
+   the objfile requested, passed as DATA.  Also clear the type reference.  */
+
+static void
+invalidate_objfile_varobj_type_iter (struct varobj *var, void *data)
+{
+  struct objfile *objfile = (struct objfile*)data;
+
+  if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile)
+    {
+      varobj_set_invalid (var);
+      var->type = NULL;
+    }
+}
+
 /* Destroy an objfile and all the symtabs and psymtabs under it.  */
 
 objfile::~objfile ()
@@ -613,6 +629,9 @@ objfile::~objfile ()
      lists.  */
   preserve_values (this);
 
+  /* Varobj may refer to types stored in objfile's obstack.  */
+  all_root_varobjs (invalidate_objfile_varobj_type_iter, this);
+
   /* It still may reference data modules have associated with the objfile and
      the symbol file data.  */
   forget_cached_source_info_for_objfile (this);
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 37a522b..f7696e7 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2431,6 +2431,22 @@ varobj_floating_p (const struct varobj *var)
   return var->root->floating;
 }
 
+/* Get the valid flag of varobj VAR.  */
+
+int
+varobj_is_valid_p (struct varobj *var)
+{
+  return var->root->is_valid;
+}
+
+/* Clear the valid flag on varobj VAR.  */
+
+void
+varobj_set_invalid (struct varobj *var)
+{
+  var->root->is_valid = 0;
+}
+
 /* Implement the "value_is_changeable_p" varobj callback for most
    languages.  */
 
@@ -2492,6 +2508,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
   if (var->root->floating || var->root->valid_block == NULL)
     {
       struct varobj *tmp_var;
+      std::string tmp_var_value, var_value;
 
       /* Try to create a varobj with same expression.  If we succeed
 	 replace the old varobj, otherwise invalidate it.  */
@@ -2500,6 +2517,24 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
       if (tmp_var != NULL) 
 	{ 
 	  tmp_var->obj_name = var->obj_name;
+         tmp_var_value = varobj_get_value (tmp_var);
+         var_value = varobj_get_value (var);
+
+         /* Since varobjs are re-evaluated during creation, there is a
+            chance the new value is different from old one.  Compare
+            old varobj and the newly created varobj and mark varobj
+            updated ff new value is different.  */
+         if (var_value.empty() && tmp_var_value.empty())
+           ; /* Equal.  */
+         else if (var_value.empty() || tmp_var_value.empty())
+           tmp_var->updated = 1;
+         else
+           {
+             /* Mark tmp_var updated if the new value is different.  */
+             if (tmp_var_value != var_value)
+               tmp_var->updated = 1;
+           }
+
 	  varobj_delete (var, 0);
 	  install_variable (tmp_var);
 	}
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 66db780..0885f70 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -323,6 +323,10 @@ extern bool varobj_editable_p (const struct varobj *var);
 
 extern bool varobj_floating_p (const struct varobj *var);
 
+extern int varobj_is_valid_p (struct varobj *var);
+
+extern void varobj_set_invalid (struct varobj *var);
+
 extern void varobj_set_visualizer (struct varobj *var,
 				   const char *visualizer);
 
-- 
2.8.1

Re: [PING][PATCH 0/2] Improved variable object invalidation in GDB

[Saqlain Raza]

Ping !

Thanks,
Saqlain

On 10/17/19 3:03 PM, Raza, Saqlain wrote:
> Hi,
> This patch series improves variable object invalidation in GDB.
>
> This is a followup to the patch series submission made in https://sourceware.org/ml/gdb-patches/2015-04/msg00598.html . This problem still holds in the latest GDB master.
>
> Raza, Saqlain (2):
>    Fix varobj updation after symbol removal
>    Testsuite for varobj updation after symbol removal
>
>   gdb/ChangeLog                              |  13 ++
>   gdb/objfiles.c                             |  19 ++
>   gdb/testsuite/ChangeLog                    |  11 +
>   gdb/testsuite/gdb.mi/mi-var-invalidate.exp |  68 ++++++
>   gdb/testsuite/gdb.mi/sym-file-lib.c        |  28 +++
>   gdb/testsuite/gdb.mi/sym-file-loader.c     | 355 +++++++++++++++++++++++++++++
>   gdb/testsuite/gdb.mi/sym-file-loader.h     | 101 ++++++++
>   gdb/testsuite/gdb.mi/sym-file-main.c       |  86 +++++++
>   gdb/varobj.c                               |  35 +++
>   gdb/varobj.h                               |   4 +
>   10 files changed, 720 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c
>

Re: [PING][PATCH 1/2] Fix varobj updation after symbol removal

[Saqlain 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);
>   

Re: [PING][PATCH 2/2] Testsuite for varobj updation after symbol removal

[Saqlain Raza]

Ping !

Thanks,
Saqlain

On 10/17/19 3:03 PM, Raza, Saqlain 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 and arm-none-linux-gnueabi targets
> using both simulator and real boards.
>
> 2019-10-17  Saqlain Raza  <saqlain_raza@mentor.com>
>              Taimoor Mirza  <taimoor_mirza@mentor.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.
>
> Signed-off-by: Raza, Saqlain <Saqlain_Raza@mentor.com>
> ---
>   gdb/testsuite/ChangeLog                    |  11 +
>   gdb/testsuite/gdb.mi/mi-var-invalidate.exp |  68 ++++++
>   gdb/testsuite/gdb.mi/sym-file-lib.c        |  28 +++
>   gdb/testsuite/gdb.mi/sym-file-loader.c     | 355 +++++++++++++++++++++++++++++
>   gdb/testsuite/gdb.mi/sym-file-loader.h     | 101 ++++++++
>   gdb/testsuite/gdb.mi/sym-file-main.c       |  86 +++++++
>   6 files changed, 649 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/ChangeLog b/gdb/testsuite/ChangeLog
> index 395a3cc..82c3a57 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,14 @@
> +2019-10-17  Saqlain Raza  <saqlain_raza@mentor.com>
> +	    Taimoor Mirza  <taimoor_mirza@mentor.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.
> +
>   2019-10-16  Andrew Burgess  <andrew.burgess@embecosm.com>
>   
>   	* gdb.fortran/module.exp: Extend with 'info variables' test.
> diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp
> index e390968..b06e713 100644
> --- a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp
> +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp
> @@ -120,6 +120,74 @@ mi_gdb_test "-var-update global_simple" \
>   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
>   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..86c719c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/sym-file-lib.c
> @@ -0,0 +1,28 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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..0a3cd32
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/sym-file-loader.c
> @@ -0,0 +1,355 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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..d6ebb39
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/sym-file-loader.h
> @@ -0,0 +1,101 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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..ecdef4e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/sym-file-main.c
> @@ -0,0 +1,86 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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;
> +}

Re: [PATCH 0/2] Improved variable object invalidation in GDB

[Luis Machado]

Hi,

Before reviewing the patch itself, could you please expand, with more 
detail, what the use case is for this particular fix?

It seems to be the same patch Taimoor sent a while ago, so in order to 
improve its chances of getting accepted, it would be nice to have a bit 
more background.

In particular, adding varobj bits to objfiles.[c] is a bit strange. 
Varobj access seems to be restricted to core varobj implementations and 
language support only.

It may be a sign that something more fundamental is missing, like an 
interface of some kind, an observer or a notification.

Better understanding the use case will allow us to determine where/how 
exactly this should be fixed.

Luis

On 10/17/19 7:03 AM, Raza, Saqlain wrote:
> Hi,
> This patch series improves variable object invalidation in GDB.
> 
> This is a followup to the patch series submission made in https://sourceware.org/ml/gdb-patches/2015-04/msg00598.html . This problem still holds in the latest GDB master.
> 
> Raza, Saqlain (2):
>    Fix varobj updation after symbol removal
>    Testsuite for varobj updation after symbol removal
> 
>   gdb/ChangeLog                              |  13 ++
>   gdb/objfiles.c                             |  19 ++
>   gdb/testsuite/ChangeLog                    |  11 +
>   gdb/testsuite/gdb.mi/mi-var-invalidate.exp |  68 ++++++
>   gdb/testsuite/gdb.mi/sym-file-lib.c        |  28 +++
>   gdb/testsuite/gdb.mi/sym-file-loader.c     | 355 +++++++++++++++++++++++++++++
>   gdb/testsuite/gdb.mi/sym-file-loader.h     | 101 ++++++++
>   gdb/testsuite/gdb.mi/sym-file-main.c       |  86 +++++++
>   gdb/varobj.c                               |  35 +++
>   gdb/varobj.h                               |   4 +
>   10 files changed, 720 insertions(+)
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h
>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c
> 

Re: [PATCH 0/2] Improved variable object invalidation in GDB

[Saqlain Raza]

Hi Luis,

Thanks you very much for taking a look and sorry for the delay in response.

> Before reviewing the patch itself, could you please expand, with more 
> detail, what the use case is for this particular fix? 
Can you please review the test-case submitted in 
https://sourceware.org/ml/gdb-patches/2019-10/msg00558.html if that 
somewhat clarifies the use-case ?

For examining, changing or updating the values of expression, a GDB/MI 
Variable object of the expression using "-var-create" is made and later 
is checked for changes using "-var-update". Now, the problem is that the 
memory contents of a global symbol (being monitored via the expression) 
are changed but in response to "-var-update", it replies with the empty 
change list "changelist=[]" where as changes are now expected in change 
list (due to changed memory contents). This happens after a symbol file 
removal takes place.

GDB traces for Variable object creation and update before symbol file is 
removed:

565,916 104-var-create --thread-group i1 - * ((NODE*)0xbc834)->next
565,924 %"Sending packet: $mbc838,4#35..."
565,924 %"Ack\n"
565,924 %"Packet received: 4cca0b00\n"
565,925 104^done,name="var14",numchild="3",value="0xbca4c 
<Control_Array+536>",type="struct NODE_\
STRUCT *",has_more="0"

573,325 141-var-update 1 var14
573,334 %"Sending packet: $mbc838,4#35..."
573,334 %"Ack\n"
573,334 %"Packet received: 4cca0b00\n"
573,334 141^done,changelist=[]

GDB traces generated when symbol file has been removed:

589,422 184-var-update 1 var14
589,431 %"Sending packet: $mbc838,4#35..."
589,431 %"Ack\n"
589,431 %"Packet received: 64cc0b00\n" <----- Memory contents did change.
589,431 184^done,changelist=[]  <------- Changelist still empty.

Thanks,
Saqlain

On 10/22/19 5:53 PM, Luis Machado wrote:
> Hi,
>
> Before reviewing the patch itself, could you please expand, with more 
> detail, what the use case is for this particular fix?
>
> It seems to be the same patch Taimoor sent a while ago, so in order to 
> improve its chances of getting accepted, it would be nice to have a 
> bit more background.
>
> In particular, adding varobj bits to objfiles.[c] is a bit strange. 
> Varobj access seems to be restricted to core varobj implementations 
> and language support only.
>
> It may be a sign that something more fundamental is missing, like an 
> interface of some kind, an observer or a notification.
>
> Better understanding the use case will allow us to determine where/how 
> exactly this should be fixed.
>
> Luis
>
> On 10/17/19 7:03 AM, Raza, Saqlain wrote:
>> Hi,
>> This patch series improves variable object invalidation in GDB.
>>
>> This is a followup to the patch series submission made in 
>> https://sourceware.org/ml/gdb-patches/2015-04/msg00598.html . This 
>> problem still holds in the latest GDB master.
>>
>> Raza, Saqlain (2):
>>    Fix varobj updation after symbol removal
>>    Testsuite for varobj updation after symbol removal
>>
>>   gdb/ChangeLog                              |  13 ++
>>   gdb/objfiles.c                             |  19 ++
>>   gdb/testsuite/ChangeLog                    |  11 +
>>   gdb/testsuite/gdb.mi/mi-var-invalidate.exp |  68 ++++++
>>   gdb/testsuite/gdb.mi/sym-file-lib.c        |  28 +++
>>   gdb/testsuite/gdb.mi/sym-file-loader.c     | 355 
>> +++++++++++++++++++++++++++++
>>   gdb/testsuite/gdb.mi/sym-file-loader.h     | 101 ++++++++
>>   gdb/testsuite/gdb.mi/sym-file-main.c       |  86 +++++++
>>   gdb/varobj.c                               |  35 +++
>>   gdb/varobj.h                               |   4 +
>>   10 files changed, 720 insertions(+)
>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c
>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c
>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h
>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c
>>

Re: [PING][PATCH 0/2] Improved variable object invalidation in GDB

[Saqlain Raza]

Ping !

For reference, the patch is available in:

https://sourceware.org/ml/gdb-patches/2019-10/msg00557.html

Testcase:

https://sourceware.org/ml/gdb-patches/2019-10/msg00558.html

Thanks,
Saqlain

On 11/7/19 12:50 PM, Saqlain Raza wrote:
> Hi Luis,
>
> Thanks you very much for taking a look and sorry for the delay in 
> response.
>
>> Before reviewing the patch itself, could you please expand, with more 
>> detail, what the use case is for this particular fix? 
> Can you please review the test-case submitted in 
> https://sourceware.org/ml/gdb-patches/2019-10/msg00558.html if that 
> somewhat clarifies the use-case ?
>
> For examining, changing or updating the values of expression, a GDB/MI 
> Variable object of the expression using "-var-create" is made and 
> later is checked for changes using "-var-update". Now, the problem is 
> that the memory contents of a global symbol (being monitored via the 
> expression) are changed but in response to "-var-update", it replies 
> with the empty change list "changelist=[]" where as changes are now 
> expected in change list (due to changed memory contents). This happens 
> after a symbol file removal takes place.
>
> GDB traces for Variable object creation and update before symbol file 
> is removed:
>
> 565,916 104-var-create --thread-group i1 - * ((NODE*)0xbc834)->next
> 565,924 %"Sending packet: $mbc838,4#35..."
> 565,924 %"Ack\n"
> 565,924 %"Packet received: 4cca0b00\n"
> 565,925 104^done,name="var14",numchild="3",value="0xbca4c 
> <Control_Array+536>",type="struct NODE_\
> STRUCT *",has_more="0"
>
> 573,325 141-var-update 1 var14
> 573,334 %"Sending packet: $mbc838,4#35..."
> 573,334 %"Ack\n"
> 573,334 %"Packet received: 4cca0b00\n"
> 573,334 141^done,changelist=[]
>
> GDB traces generated when symbol file has been removed:
>
> 589,422 184-var-update 1 var14
> 589,431 %"Sending packet: $mbc838,4#35..."
> 589,431 %"Ack\n"
> 589,431 %"Packet received: 64cc0b00\n" <----- Memory contents did change.
> 589,431 184^done,changelist=[]  <------- Changelist still empty.
>
> Thanks,
> Saqlain
>
> On 10/22/19 5:53 PM, Luis Machado wrote:
>> Hi,
>>
>> Before reviewing the patch itself, could you please expand, with more 
>> detail, what the use case is for this particular fix?
>>
>> It seems to be the same patch Taimoor sent a while ago, so in order 
>> to improve its chances of getting accepted, it would be nice to have 
>> a bit more background.
>>
>> In particular, adding varobj bits to objfiles.[c] is a bit strange. 
>> Varobj access seems to be restricted to core varobj implementations 
>> and language support only.
>>
>> It may be a sign that something more fundamental is missing, like an 
>> interface of some kind, an observer or a notification.
>>
>> Better understanding the use case will allow us to determine 
>> where/how exactly this should be fixed.
>>
>> Luis
>>
>> On 10/17/19 7:03 AM, Raza, Saqlain wrote:
>>> Hi,
>>> This patch series improves variable object invalidation in GDB.
>>>
>>> This is a followup to the patch series submission made in 
>>> https://sourceware.org/ml/gdb-patches/2015-04/msg00598.html . This 
>>> problem still holds in the latest GDB master.
>>>
>>> Raza, Saqlain (2):
>>>    Fix varobj updation after symbol removal
>>>    Testsuite for varobj updation after symbol removal
>>>
>>>   gdb/ChangeLog                              |  13 ++
>>>   gdb/objfiles.c                             |  19 ++
>>>   gdb/testsuite/ChangeLog                    |  11 +
>>>   gdb/testsuite/gdb.mi/mi-var-invalidate.exp |  68 ++++++
>>>   gdb/testsuite/gdb.mi/sym-file-lib.c        |  28 +++
>>>   gdb/testsuite/gdb.mi/sym-file-loader.c     | 355 
>>> +++++++++++++++++++++++++++++
>>>   gdb/testsuite/gdb.mi/sym-file-loader.h     | 101 ++++++++
>>>   gdb/testsuite/gdb.mi/sym-file-main.c       |  86 +++++++
>>>   gdb/varobj.c                               |  35 +++
>>>   gdb/varobj.h                               |   4 +
>>>   10 files changed, 720 insertions(+)
>>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-lib.c
>>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.c
>>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-loader.h
>>>   create mode 100644 gdb/testsuite/gdb.mi/sym-file-main.c
>>>

Re: [PING][PATCH 0/2] Improved variable object invalidation in GDB

[Taimoor]

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
>>

[PING][PATCH 0/2] Improved variable object invalidation in GDB

[Taimoor]

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
>