Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Jan Kratochvil <jan.kratochvil@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH v12 17/32] Add file_location utility functions
Date: Fri, 21 Aug 2015 21:22:00 -0000	[thread overview]
Message-ID: <20150821212230.6673.63868.stgit@host1.jankratochvil.net> (raw)
In-Reply-To: <20150821212006.6673.35100.stgit@host1.jankratochvil.net>

Hi,

build id verification faces a problem current codebase calls openp() and
similar functions which search many directories and after they find the file
they return its filename.  Caller then typically opens that file as BFD.

But to search the directories openp() needs to open each file it iterates to
verify their build-id.  Then it would close the file and caller would reopen it
again.  This is inefficient (+racy), so a new intermediate representation of
the found files is created (file_location).

Another possibility would be to depends on bfd cache as being done in
exec_file_attach.  But then a similar problem is needed for callers which want
only fd (and not bfd).  Besides that I find depending on pending
make_cleanup_bfd_unref() from openp() calls would be tricky.


Jan


gdb/ChangeLog
2015-08-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* defs.h (enum openp_flags): Add OPF_IS_BFD.
	* gdb_bfd.c (fileio_errno_to_host): Make it public.
	* gdb_bfd.h (fileio_errno_to_host): Add prototype.
	* source.c: Include inferior.h and gdb/fileio.h.
	(file_location_enoent, file_location_free, file_location_cleanup)
	(file_location_is_valid, file_location_from_filename)
	(file_location_to_bfd, filename_to_bfd): New functions.
	* source.h (struct file_location): New definition.
	(file_location_enoent, file_location_free, file_location_is_valid)
	(file_location file_location_from_filename, file_location_to_bfd)
	(filename_to_bfd): New prototypes.
---
 0 files changed

diff --git a/gdb/defs.h b/gdb/defs.h
index 7795b4a..8ee8b33 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -338,6 +338,9 @@ enum openp_flags
      for source files but not for executables).  */
   OPF_SEARCH_IN_PATH  = (1 << 1),
 
+  /* Ask for bfd * to be returned in file_location.  */
+  OPF_IS_BFD          = (1 << 2),
+
   /* Open the file in read/write mode if WRITE_FILES says so.  */
   OPF_OPEN_RW_TMP     = (1 << 3),
 };
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 1224695..dc2d6c0 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -197,7 +197,7 @@ gdb_bfd_has_target_filename (struct bfd *abfd)
 
 /* Return the system error number corresponding to ERRNUM.  */
 
-static int
+int
 fileio_errno_to_host (int errnum)
 {
   switch (errnum)
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index dcc6755..99fabfd 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -172,4 +172,6 @@ int gdb_bfd_count_sections (bfd *abfd);
 
 int gdb_bfd_requires_relocations (bfd *abfd);
 
+int fileio_errno_to_host (int errnum);
+
 #endif /* GDB_BFD_H */
diff --git a/gdb/source.c b/gdb/source.c
index 4096d0d..d06cc84 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -27,6 +27,8 @@
 #include "frame.h"
 #include "value.h"
 #include "filestuff.h"
+#include "inferior.h"
+#include "gdb/fileio.h"
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -737,6 +739,211 @@ dirnames_to_char_ptr_vec_target_exc (const char *string)
   return vec;
 }
 
+/* Initialize *FILE as failed one with error code ENOENT.  Such
+   file_location still should be processed by file_location_free.  */
+
+void
+file_location_enoent (struct file_location *file)
+{
+  memset (file, 0, sizeof (*file));
+  file->fd = -1;
+  file->file_errno = ENOENT;
+}
+
+/* Free resources of *FILE.  *FILE must be properly initialized, either
+   by a successful or unsuccessful locator.  *FILE remains unspecified
+   after this call.  */
+
+void
+file_location_free (struct file_location *file)
+{
+  if (file->fd != -1)
+    {
+      if (file->load_via_target)
+	{
+	  int target_errno;
+
+	  target_fileio_close (file->fd, &target_errno);
+	}
+      else
+	close (file->fd);
+    }
+  gdb_bfd_unref (file->abfd);
+  xfree (file->filename);
+}
+
+/* Call file_location_free suitable for make_cleanup.  */
+
+static void
+file_location_cleanup (void *file_voidp)
+{
+  file_location_free (file_voidp);
+}
+
+/* Return boolean 1 if *FILE contains a successful file representation.
+   Return 0 otherwise.  */
+
+int
+file_location_is_valid (const struct file_location *file)
+{
+  if (!file->file_errno && !file->bfderr)
+    return 1;
+  gdb_assert (file->abfd == NULL);
+  return 0;
+}
+
+/* Return new file_location from FILENAME and OPTS.  */
+
+struct file_location
+file_location_from_filename (const char *filename, enum openp_flags opts)
+{
+  struct file_location file;
+  struct cleanup *back_to;
+
+  memset (&file, 0, sizeof (file));
+  file.fd = -1;
+  back_to = make_cleanup (file_location_cleanup, &file);
+  file.filename = xstrdup (filename);
+
+  if (is_target_filename (filename))
+    {
+      filename += strlen (TARGET_SYSROOT_PREFIX);
+      if (!target_filesystem_is_local ())
+	{
+	  file.load_via_target = 1;
+
+	  /* gdb_bfd_fopen does not support "target:" filenames.  */
+	  if (write_files)
+	    warning (_("writing into executable files is "
+		       "not supported for %s sysroots"),
+		     TARGET_SYSROOT_PREFIX);
+	}
+    }
+
+  if (file.load_via_target)
+    {
+      int target_errno;
+
+      file.fd = target_fileio_open (current_inferior (),
+				    filename, FILEIO_O_RDONLY, 0,
+				    &target_errno);
+      if (file.fd == -1)
+	{
+	  file.file_errno = fileio_errno_to_host (target_errno);
+	  discard_cleanups (back_to);
+	  return file;
+	}
+    }
+  else
+    {
+      /* WRITE_FILES is ignored if !OPF_IS_BFD.  */
+
+      file.fd = gdb_open_cloexec (filename, O_RDONLY | O_BINARY, 0);
+      if (file.fd == -1)
+	{
+	  file.file_errno = errno;
+	  discard_cleanups (back_to);
+	  return file;
+	}
+    }
+
+  if ((opts & OPF_IS_BFD) == 0)
+    {
+      discard_cleanups (back_to);
+      return file;
+    }
+
+  if (file.load_via_target)
+    {
+      const int do_close = (opts & OPF_IS_BFD) != 0;
+
+      gdb_assert (strcmp (filename,
+			  file.filename + strlen (TARGET_SYSROOT_PREFIX)) == 0);
+      file.abfd = gdb_bfd_open_from_target (file.filename, gnutarget, file.fd,
+					    do_close);
+      if (do_close && file.abfd != NULL)
+	file.fd = -1;
+    }
+  else
+    {
+      if (write_files)
+	file.abfd = gdb_bfd_fopen (filename, gnutarget, FOPEN_RUB, file.fd);
+      else
+	file.abfd = gdb_bfd_open (filename, gnutarget, file.fd);
+      if ((opts & OPF_IS_BFD) != 0)
+	file.fd = -1;
+      else
+	{
+	  file.fd = dup (file.fd);
+	  if (file.fd == -1)
+	    {
+	      int save_errno = errno;
+
+	      do_cleanups (back_to);
+	      file_location_enoent (&file);
+	      file.file_errno = save_errno;
+	      return file;
+	    }
+	}
+    }
+
+  if (file.abfd == NULL)
+    {
+      file.bfderr = bfd_get_error ();
+      discard_cleanups (back_to);
+      return file;
+    }
+
+  discard_cleanups (back_to);
+  return file;
+}
+
+/* Return new BFD * from FILE.  If FILE represents a failed locator
+   result then bfd_set_error is called and NULL is returned.  FILE
+   content is always freed by this function.  The locator for FILE must
+   have been called with OPF_IS_BFD.  */
+
+bfd *
+file_location_to_bfd (struct file_location file)
+{
+  bfd *retval;
+
+  if (file.abfd == NULL)
+    {
+      const int file_errno = file.file_errno;
+      const bfd_error_type file_bfderr = file.bfderr;
+
+      gdb_assert (file_errno || file_bfderr);
+
+      file_location_free (&file);
+
+      if (file_bfderr == bfd_error_on_input)
+	bfd_set_error (bfd_error_malformed_archive);
+      else if (file_bfderr)
+	bfd_set_error (file_bfderr);
+      else
+	{
+	  bfd_set_error (bfd_error_system_call);
+	  errno = file_errno;
+	}
+      return NULL;
+    }
+  gdb_bfd_ref (file.abfd);
+  retval = file.abfd;
+  file_location_free (&file);
+  return retval;
+}
+
+/* Return new BFD * from FILENAME.  See file_location_from_filename and
+   file_location_to_bfd.  */
+
+bfd *
+filename_to_bfd (const char *filename)
+{
+  return file_location_to_bfd (file_location_from_filename (filename,
+							    OPF_IS_BFD));
+}
+
 /* Open a file named STRING, searching path PATH (dir names sep by some char).
    You cannot use this function to create files.
 
diff --git a/gdb/source.h b/gdb/source.h
index fd58d8e..f511959 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -101,4 +101,50 @@ extern void add_substitute_path_rule (char *, char *);
 
 extern VEC (char_ptr) *dirnames_to_char_ptr_vec_target_exc (const char *string);
 
+/* Unified file representation which can contain BFD *, filename and FD
+   in all or some of the forms.  */
+struct file_location
+{
+  /* This pointer can be NULL.  Use OPF_IS_BFD to get it filled in.  */
+  bfd *abfd;
+
+  /* This pointer is never NULL.  It names the actual file opened (this
+     string will always start with a "/").  We have to take special
+     pains to avoid doubling the "/" between the directory and the file,
+     sigh!  Emacs gets confuzzed by this when we print the source file
+     name!!!  */
+  char *filename;
+
+  /* A flag whether FD represents remote target fileio descriptor or
+     local operating system file descriptor.  */
+  unsigned char load_via_target;
+
+  /* File descriptor (see LOAD_VIA_TARGET) or -1.  Do not use OPF_IS_BFD
+     to get it filled in.  */
+  int fd;
+
+  /* ERRNO value if a call to fill-in this structure failed.  See also
+     BFDERR.  */
+  int file_errno;
+
+  /* BFD error value if a call to fill-in this structure failed or
+     bfd_error_no_error.  If either FILE_ERRNO or BFDERR are non-zero
+     then this file_location failed and ABFD must be NULL.  There is no
+     guarantee for failed file_location about FD value.  */
+  bfd_error_type bfderr;
+};
+
+extern void file_location_enoent (struct file_location *file);
+
+extern void file_location_free (struct file_location *file);
+
+extern int file_location_is_valid (const struct file_location *file);
+
+extern struct file_location file_location_from_filename (const char *filename,
+							 enum openp_flags opts);
+
+extern bfd *file_location_to_bfd (struct file_location file);
+
+extern bfd *filename_to_bfd (const char *filename);
+
 #endif


  parent reply	other threads:[~2015-08-21 21:22 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-21 21:20 [PATCH v12 00/32] Validate binary before use Jan Kratochvil
2015-08-21 21:20 ` [PATCH v12 04/32] Move linux_find_memory_regions_full & co Jan Kratochvil
2015-08-21 21:20 ` [PATCH v12 05/32] gdbserver build-id attribute generator Jan Kratochvil
2015-08-22  7:17   ` Eli Zaretskii
2015-08-21 21:20 ` [PATCH v12 03/32] Prepare linux_find_memory_regions_full & co. for move Jan Kratochvil
2015-08-21 21:20 ` [PATCH v12 01/32] Create empty common/linux-maps.[ch] and common/target-utils.[ch] Jan Kratochvil
2015-08-21 21:20 ` [PATCH v12 02/32] Move gdb_regex* to common/ Jan Kratochvil
2015-08-21 21:21 ` [PATCH v12 10/32] Code cleanup: Add enum for openp_flags Jan Kratochvil
2015-08-21 21:21 ` [PATCH v12 06/32] Validate symbol file using build-id Jan Kratochvil
2015-08-22  7:23   ` Eli Zaretskii
2015-08-22 19:19     ` Jan Kratochvil
2015-08-21 21:21 ` [PATCH v12 12/32] Code cleanup: Remove openp parameter mode Jan Kratochvil
2015-08-21 21:21 ` [PATCH v12 08/32] Permit multiple sysroot directories Jan Kratochvil
2015-08-22  7:31   ` Eli Zaretskii
2015-08-22 19:24     ` Jan Kratochvil
2015-08-21 21:21 ` [PATCH v12 07/32] Code cleanup: Make solib_find_1 variable const Jan Kratochvil
2015-08-21 21:21 ` [PATCH v12 11/32] Code cleanup: Remove OPF_RETURN_REALPATH Jan Kratochvil
2015-08-21 21:22 ` [PATCH v12 15/32] gdb_bfd_open_from_target: Support real fd Jan Kratochvil
2015-08-21 21:22 ` [PATCH v12 18/32] Refactor openp() to return file_location Jan Kratochvil
2015-08-25 19:07   ` [PATCH v13 " Jan Kratochvil
2015-08-21 21:22 ` [PATCH v12 19/32] solib_find: Make it use file_location Jan Kratochvil
2015-08-21 21:22 ` [PATCH v12 13/32] Code cleanup: openp parameter filename_opened is never NULL Jan Kratochvil
2015-08-21 21:22 ` [PATCH v12 14/32] Provide new gdb_bfd_open_from_target Jan Kratochvil
2015-08-21 21:22 ` [PATCH v12 20/32] symfile_bfd_open: Make it use openp_bfd() Jan Kratochvil
2015-08-21 21:22 ` [PATCH v12 16/32] gdb_bfd_open_from_target: Optionally do not close fd Jan Kratochvil
2015-08-21 21:22 ` Jan Kratochvil [this message]
2015-08-21 21:22 ` [PATCH v12 09/32] Change sysroot to ":target:" Jan Kratochvil
2015-08-21 21:23 ` [PATCH v12 27/32] build_id_verify: Make it more verbose Jan Kratochvil
2015-08-21 21:23 ` [PATCH v12 26/32] solib_bfd_open: Optimize opening twice Jan Kratochvil
2015-08-21 21:23 ` [PATCH v12 21/32] build_id_to_debug_bfd: Make it also non-.debug capable Jan Kratochvil
2015-08-21 21:23 ` [PATCH v12 23/32] build_id_to_bfd: Make it return file_location Jan Kratochvil
2015-08-21 21:23 ` [PATCH v12 24/32] solib_find: Search also by build-id Jan Kratochvil
2015-08-21 21:23 ` [PATCH v12 22/32] Add dummy build-id params to prototypes Jan Kratochvil
2015-08-21 21:23 ` [PATCH v12 25/32] Verify the build-id Jan Kratochvil
2015-08-21 21:24 ` [PATCH v12 28/32] Tests for validate symbol file using build-id Jan Kratochvil
2015-08-21 21:24 ` [PATCH v12 29/32] testcase: Test also locating shlibs by build-id Jan Kratochvil
2015-08-21 21:24 ` [PATCH v12 32/32] Support build-id for main executables Jan Kratochvil
2015-08-21 21:24 ` [PATCH v12 30/32] Code cleanup: New hex2bin_allocate() Jan Kratochvil
2015-08-21 21:24 ` [PATCH v12 31/32] Make only user-specified executable and symbol filenames sticky Jan Kratochvil
2015-09-02  3:57 ` [PATCH v12 00/32] Validate binary before use Doug Evans
2015-09-02  7:41   ` Jan Kratochvil
2015-09-02 14:58     ` Doug Evans
2015-09-02 15:14       ` Jan Kratochvil
2015-09-02 15:22         ` Doug Evans
2015-09-02 19:12           ` Jan Kratochvil
2015-09-15  1:37             ` Doug Evans

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150821212230.6673.63868.stgit@host1.jankratochvil.net \
    --to=jan.kratochvil@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox