[patchv3 0/8] Validate binary before use

Jan Kratochvil jan.kratochvil@redhat.com
Thu Feb 27 21:33:00 GMT 2014


Hi,

git://sourceware.org/git/archer.git
jankratochvil/gdbserverbuildid
 - although it is not properly separated into 1..8 there as in this series

the is a follow-up to never checked in series:
	Message-ID: <1366127096-5744-1-git-send-email-ARistovski@qnx.com>
	https://sourceware.org/ml/gdb-patches/2013-04/msg00472.html

The changes [attached] are:

 * Implemented the review comments I made.
   (Except for comments for the testcase.)

 * Removed fetching build-id in solib-svr4.c for NAT run:
   /* There is no way to safely fetch build-id from running inferior without OS
      specific code.  The code from get_hex_build_id from gdbserver/linux-low.c
      could be used for GNU/Linux NAT target.  */

   As former code regressed break-interp.exp with 32-bit targets for:
   egrep '(BINprelinkNO.*pieATTACH.*relinkYES|BINprelinkYES.*pieATTACH.*relinkNO).*attach main bt'

   Implementing new feature into gdbserver and not linux-nat seems correct to
   me according to:
   https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity
   I will list there this local/remote difference after check-in but that
   should be OK after the final switch to gdbserver.

I left there authorship intact except for added mine to [patchv3 7/8].
I do not mind to change it any way.


Jan
-------------- next part --------------
diff --git a/gdb/common/common-target.c b/gdb/common/common-target.c
index 4bd2731..d6e5d60 100644
--- a/gdb/common/common-target.c
+++ b/gdb/common/common-target.c
@@ -1,6 +1,6 @@
 /* Utility target functions for GDB, and GDBserver.
 
-   Copyright (C) 2013 Free Software Foundation, Inc.
+   Copyright (C) 2014 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
diff --git a/gdb/common/common-target.h b/gdb/common/common-target.h
index b252c00..9aedc12 100644
--- a/gdb/common/common-target.h
+++ b/gdb/common/common-target.h
@@ -1,6 +1,6 @@
 /* Utility target functions for GDB, and GDBserver.
 
-   Copyright (C) 2013 Free Software Foundation, Inc.
+   Copyright (C) 2014 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
index 6432b8f..bb587df 100644
--- a/gdb/common/linux-maps.c
+++ b/gdb/common/linux-maps.c
@@ -1,5 +1,5 @@
 /* Linux-specific memory maps manipulation routines.
-   Copyright (C) 2013 Free Software Foundation, Inc.
+   Copyright (C) 2014 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
diff --git a/gdb/common/linux-maps.h b/gdb/common/linux-maps.h
index e989376..03b14d3 100644
--- a/gdb/common/linux-maps.h
+++ b/gdb/common/linux-maps.h
@@ -1,5 +1,5 @@
 /* Linux-specific memory maps manipulation routines.
-   Copyright (C) 2013 Free Software Foundation, Inc.
+   Copyright (C) 2014 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d14b215..6f05bd4 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -163,10 +163,7 @@ typedef union ElfXX_Nhdr
 #define ELFXX_SIZEOF(elf64, hdr) ((elf64) ? sizeof ((hdr)._64) \
 					  : sizeof ((hdr)._32))
 /* Round up to next 4 byte boundary.  */
-#define ELFXX_ROUNDUP_4(elf64, what) ((elf64) ? (((what) + 3)		\
-						 & ~((Elf64_Word)3U))	\
-					      : (((what) + 3)		\
-						 & ~((Elf32_Word) 3U)))
+#define ELFXX_ROUNDUP_4(elf64, what) (((what) + 3) & ~(ULONGEST) 3)
 #define BUILD_ID_INVALID "?"
 
 /* A list of all unknown processes which receive stop signals.  Some
@@ -5764,7 +5761,7 @@ free_mapping_entry_vec (VEC (mapping_entry_s) *lst)
 static int
 compare_mapping_entry_range (const void *const k, const void *const b)
 {
-  const ULONGEST key = *(CORE_ADDR *) k;
+  const ULONGEST key = *(const CORE_ADDR *) k;
   const mapping_entry_s *const p = b;
 
   if (key < p->vaddr)
@@ -5812,7 +5809,8 @@ read_build_id (struct find_memory_region_callback_data *const p,
 	  || e_phentsize != ELFXX_SIZEOF (is_elf64, *phdr))
 	{
 	  /* Basic sanity check failed.  */
-	  warning ("Could not read program header.");
+	  warning (_("Could not identify program header at %s."),
+		   paddress (load_addr));
 	  return;
 	}
 
@@ -5823,7 +5821,8 @@ read_build_id (struct find_memory_region_callback_data *const p,
 			     ELFXX_FLD (is_elf64, ehdr, e_phnum) * e_phentsize)
 	  != 0)
 	{
-	  warning ("Could not read program header.");
+	  warning (_("Could not read program header at %s."),
+		   paddress (load_addr));
 	  return;
 	}
 
@@ -5847,14 +5846,14 @@ read_build_id (struct find_memory_region_callback_data *const p,
 				 ELFXX_FLD (is_elf64, *phdr, p_memsz)) != 0)
 	    {
 	      xfree (pt_note);
-	      warning ("Could not read note at address 0x%s",
+	      warning (_("Could not read note at address 0x%s"),
 		       paddress (note_addr));
 	      break;
 	    }
 
 	  pt_end = pt_note + ELFXX_FLD (is_elf64, *phdr, p_memsz);
 	  nhdr = (void *) pt_note;
-	  while ((gdb_byte *) nhdr < pt_end)
+	  while ((const gdb_byte *) nhdr < pt_end)
 	    {
 	      const size_t namesz
 		= ELFXX_ROUNDUP_4 (is_elf64, ELFXX_FLD (is_elf64, *nhdr,
@@ -5862,14 +5861,14 @@ read_build_id (struct find_memory_region_callback_data *const p,
 	      const size_t descsz
 		= ELFXX_ROUNDUP_4 (is_elf64, ELFXX_FLD (is_elf64, *nhdr,
 							n_descsz));
-	      const size_t note_sz = ELFXX_SIZEOF (is_elf64, *nhdr) + namesz
-				     + descsz;
+	      const size_t note_sz = (ELFXX_SIZEOF (is_elf64, *nhdr) + namesz
+				      + descsz);
 
-	      if (((gdb_byte *) nhdr + note_sz) > pt_end || note_sz == 0
+	      if (((const gdb_byte *) nhdr + note_sz) > pt_end || note_sz == 0
 		  || descsz == 0)
 		{
-		  warning ("Malformed PT_NOTE at address 0x%s\n",
-			   paddress ((CORE_ADDR) nhdr));
+		  warning (_("Malformed PT_NOTE at address 0x%s\n"),
+			   paddress (note_addr + (gdb_byte *) nhdr - pt_note));
 		  break;
 		}
 	      if (ELFXX_FLD (is_elf64, *nhdr, n_type) == NT_GNU_BUILD_ID
@@ -5981,9 +5980,10 @@ get_hex_build_id (const CORE_ADDR l_addr, const CORE_ADDR l_ld,
 	{
 	  /* Do not try to find hex_build_id again.  */
 	  bil->hex_build_id = xstrdup (BUILD_ID_INVALID);
-	  warning ("Could not determine load address; mapping entry with"
-		   " offset 0 corresponding to l_ld=0x%s could not be found; "
-		   "build-id can not be used.", paddress (l_ld));
+	  warning (_("Could not determine load address; mapping entry with "
+		     "offset 0 corresponding to l_ld = 0x%s could not be "
+		     "found; build-id can not be used."),
+		   paddress (l_ld));
 	}
     }
 
@@ -6050,7 +6050,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
   VEC_reserve (mapping_entry_s, data.list, 16);
   if (linux_find_memory_regions_full (pid, find_memory_region_callback, &data,
       NULL) < 0)
-    warning ("Finding memory regions failed");
+    warning (_("Finding memory regions failed"));
 
   while (annex[0] != '\0')
     {
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 235a7c2..d0c8761 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -737,7 +737,7 @@ linux_find_memory_regions_gdb (struct gdbarch *gdbarch,
   /* We need to know the real target PID so
      linux_find_memory_regions_full can access /proc.  */
   if (current_inferior ()->fake_pid_p)
-    return 1;
+    return -1;
 
   cleanup = make_cleanup (free_current_contents, &memory_to_free);
   retval = linux_find_memory_regions_full (current_inferior ()->pid,
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 9de1ae7..7d4cce9 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -968,12 +968,14 @@ svr4_keep_data_in_core (CORE_ADDR vaddr, unsigned long size)
 static int
 svr4_validate (const struct so_list *const so)
 {
-  gdb_byte *build_id;
-  size_t build_idsz;
-  size_t abfd_build_idsz;
-
   gdb_assert (so != NULL);
 
+  /* There is no way to safely fetch build-id from running inferior without OS
+     specific code.  The code from get_hex_build_id from gdbserver/linux-low.c
+     could be used for GNU/Linux NAT target.  */
+  if (so->build_id == NULL)
+    return 1;
+
   if (so->abfd == NULL)
     return 1;
 
@@ -982,96 +984,9 @@ svr4_validate (const struct so_list *const so)
       || elf_tdata (so->abfd)->build_id == NULL)
     return 1;
 
-  build_id = so->build_id;
-  build_idsz = so->build_idsz;
-  abfd_build_idsz = elf_tdata (so->abfd)->build_id->size;
-
-  if (build_id == NULL)
-    {
-      /* Get build_id from NOTE_GNU_BUILD_ID_NAME section.
-         This is a fallback mechanism for targets that do not
-	 implement TARGET_OBJECT_SOLIB_SVR4.  */
-
-      const asection *const asec
-	= bfd_get_section_by_name (so->abfd, NOTE_GNU_BUILD_ID_NAME);
-      ULONGEST bfd_sect_size;
-
-      if (asec == NULL)
-	return 1;
-
-      bfd_sect_size = bfd_get_section_size (asec);
-
-      if ((asec->flags & SEC_LOAD) == SEC_LOAD
-	  && bfd_sect_size != 0
-	  && strcmp (bfd_section_name (asec->bfd, asec),
-		     NOTE_GNU_BUILD_ID_NAME) == 0)
-	{
-	  const enum bfd_endian byte_order
-	    = gdbarch_byte_order (target_gdbarch ());
-	  Elf_External_Note *const note = xmalloc (bfd_sect_size);
-	  gdb_byte *const note_raw = (void *) note;
-	  struct cleanup *cleanups = make_cleanup (xfree, note);
-
-	  if (target_read_memory (bfd_get_section_vma (so->abfd, asec)
-				  + lm_addr_check (so, so->abfd),
-				  note_raw, bfd_sect_size) == 0)
-	    {
-	      build_idsz
-		= extract_unsigned_integer ((gdb_byte *) note->descsz,
-					    sizeof (note->descsz),
-					    byte_order);
-
-	      if (build_idsz == abfd_build_idsz)
-		{
-		  const char gnu[4] = "GNU\0";
-
-		  if (memcmp (note->name, gnu, 4) == 0)
-		    {
-		      ULONGEST namesz
-			= extract_unsigned_integer ((gdb_byte *) note->namesz,
-						    sizeof (note->namesz),
-						    byte_order);
-		      CORE_ADDR build_id_offs;
-
-		      /* Rounded to next sizeof (ElfXX_Word).  */
-		      namesz = ((namesz + (sizeof (note->namesz) - 1))
-				& ~((ULONGEST) (sizeof (note->namesz) - 1)));
-		      build_id_offs = (offsetof (Elf_External_Note, name)
-				       + namesz);
-		      build_id = xmalloc (build_idsz);
-		      memcpy (build_id, note_raw + build_id_offs, build_idsz);
-		    }
-		}
-
-	      if (build_id == NULL)
-		{
-		  /* If we are here, it means target memory read succeeded
-		     but note was not where it was expected according to the
-		     abfd.  Allow the logic below to perform the check
-		     with an impossible build-id and fail validation.  */
-		  build_idsz = 0;
-		  build_id = xmalloc (0);
-		}
-
-	    }
-	  do_cleanups (cleanups);
-	}
-    }
-
-  if (build_id != NULL)
-    {
-      const int match
-	= (abfd_build_idsz == build_idsz
-	   && memcmp (build_id, elf_tdata (so->abfd)->build_id->data,
-		      build_idsz) == 0);
-
-      if (build_id != so->build_id)
-	xfree (build_id);
-
-      return match;
-    }
-
-  return 1;
+  return (so->build_idsz == elf_tdata (so->abfd)->build_id->size
+	  && memcmp (so->build_id, elf_tdata (so->abfd)->build_id->data,
+		     so->build_idsz) == 0);
 }
 
 /* Implement the "open_symbol_file_object" target_so_ops method.
diff --git a/gdb/target.c b/gdb/target.c
index abf9058..d892e25 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3020,9 +3020,6 @@ target_fileio_close_cleanup (void *opaque)
   target_fileio_close (fd, &target_errno);
 }
 
-typedef int (read_alloc_pread_ftype) (int handle, gdb_byte *read_buf, int len,
-				      ULONGEST offset, int *target_errno);
-
 static read_alloc_pread_ftype target_fileio_read_alloc_1_pread;
 
 /* Helper for target_fileio_read_alloc_1 to make it interruptible.  */
@@ -3036,9 +3033,6 @@ target_fileio_read_alloc_1_pread (int handle, gdb_byte *read_buf, int len,
   return target_fileio_pread (handle, read_buf, len, offset, target_errno);
 }
 
-typedef LONGEST (read_stralloc_func_ftype) (const char *filename,
-					    gdb_byte **buf_p, int padding);
-
 static read_stralloc_func_ftype target_fileio_read_alloc_1;
 
 /* Read target file FILENAME.  Store the result in *BUF_P and
diff --git a/gdb/testsuite/gdb.base/solib-mismatch-lib.c b/gdb/testsuite/gdb.base/solib-mismatch-lib.c
index 19f1545..65b26af 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch-lib.c
+++ b/gdb/testsuite/gdb.base/solib-mismatch-lib.c
@@ -1,6 +1,6 @@
 /* This testcase is part of GDB, the GNU debugger.
 
-   Copyright 2013 Free Software Foundation, Inc.
+   Copyright 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
diff --git a/gdb/testsuite/gdb.base/solib-mismatch-libmod.c b/gdb/testsuite/gdb.base/solib-mismatch-libmod.c
index 3b025a8..fc8827e 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch-libmod.c
+++ b/gdb/testsuite/gdb.base/solib-mismatch-libmod.c
@@ -1,6 +1,6 @@
 /* This testcase is part of GDB, the GNU debugger.
 
-   Copyright 2013 Free Software Foundation, Inc.
+   Copyright 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
diff --git a/gdb/testsuite/gdb.base/solib-mismatch.c b/gdb/testsuite/gdb.base/solib-mismatch.c
index 7a5d960..7bf425d 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch.c
+++ b/gdb/testsuite/gdb.base/solib-mismatch.c
@@ -1,6 +1,6 @@
 /* This testcase is part of GDB, the GNU debugger.
 
-   Copyright 2013 Free Software Foundation, Inc.
+   Copyright 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
diff --git a/gdb/testsuite/gdb.base/solib-mismatch.exp b/gdb/testsuite/gdb.base/solib-mismatch.exp
index 4504bc3..7a625ba 100644
--- a/gdb/testsuite/gdb.base/solib-mismatch.exp
+++ b/gdb/testsuite/gdb.base/solib-mismatch.exp
@@ -1,4 +1,4 @@
-# Copyright 2013 Free Software Foundation, Inc.
+# Copyright 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
@@ -16,6 +16,11 @@
 standard_testfile
 set executable $testfile
 
+if ![is_remote target] {
+  untested "only gdbserver supports build-id reporting"
+  return -1
+}
+
 # Test overview:
 #  generate two shared objects. One that will be used by the process
 #  and another, modified, that will be found by gdb. Gdb should
@@ -69,7 +74,7 @@ if { [build_executable $testfile.exp $executable $srcfile $exec_opts] != 0 } {
 if { [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfilerun}" "${binlibfilerun}" [list debug ldflags=-Wl,--build-id]] != ""
      || [gdb_gnu_strip_debug "${binlibfilerun}"]
      || [gdb_compile_shlib "${srcdir}/${subdir}/${srclibfilegdb}" "${binlibfilegdb}" [list debug ldflags=-Wl,--build-id]] != "" } {
-  untested "gdb_compile_shlib failed."
+  untested "compilation failed."
   return -1
 }
 


More information about the Gdb-patches mailing list