This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] bfd: Fix Win64 build, rework find_separate_debug_file interface


Building binutils/gdb for --host=x86_64-w64-mingw32 currently trips on:

 ../../src/bfd/opncls.c: In function 'get_build_id_name':
 ../../src/bfd/opncls.c:1904:20: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
    * build_id_out = (unsigned long) build_id;
		     ^
 ../../src/bfd/opncls.c: In function 'check_build_id_file':
 ../../src/bfd/opncls.c:1954:19: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
    orig_build_id = (struct bfd_build_id *) buildid;
		    ^

AFAICS, the larger design problem here is that it looks like the
build_id version of the callbacks was grafted into an interface that
originally worked with crc32 only.  Storage for the object passed to
the find_separate_debug_file callbacks is allocated inside
find_separate_debug_file itself, meaning, find_separate_debug_file
needs to know a detail about the callback implementations, and then
the callbacks' second parameter reflects that.

Since a find_separate_debug_file caller can always statically know
which type (crc32 vs build_id vs etc.) the callbacks it is passing to
find_separate_debug_file work with, the fix is to generalize the
interface of find_separate_debug_file and the corresponding
get_func_type/check_func_type callback types by instead pass around an
untyped pointer to an object allocated (on the stack) by
find_separate_debug_file's callers.

Tested on x86_64 Fedora 23, with gdb's, binutils' and ld's testsuites.

gdb/ChangeLog:
2017-04-05  Pedro Alves  <palves@redhat.com>

	* opncls.c (bfd_get_debug_link_info): Rename to...
	(bfd_get_debug_link_info_1): ... this.  Change type of second
	parameter to void pointer.  Adjust.
	(bfd_get_debug_link_info): Reimplement on top of
	bfd_get_debug_link_info_1.
	(separate_debug_file_exists, separate_alt_debug_file_exists):
	Change type of second parameter to void pointer.  Adjust.
	(get_func_type, check_func_type): Change type of second parameter
	to void pointer.
	(find_separate_debug_file): Add 'func_data' parameter.  Pass it to
	the callback functions instead of passing the address of a local.
	(bfd_follow_gnu_debuglink): Pass address of unsigned long local to
	find_separate_debug_file.
	(get_alt_debug_link_info_shim): Change type of second parameter to
	void pointer.  Adjust.
	(bfd_follow_gnu_debugaltlink): Adjust to pass NULL to
	find_separate_debug_file.
	(get_build_id_name, bfd_boolean check_build_id_file): Change type
	of second parameter to void pointer.  Adjust.
	(bfd_follow_build_id_debuglink): Pass address of bfd_build_id
	pointer local to find_separate_debug_file.
---
 bfd/opncls.c | 167 +++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 105 insertions(+), 62 deletions(-)

diff --git a/bfd/opncls.c b/bfd/opncls.c
index 4137a3b..0eb677c 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -1158,24 +1158,28 @@ bfd_calc_gnu_debuglink_crc32 (unsigned long crc,
 
 /*
 FUNCTION
-	bfd_get_debug_link_info
+	bfd_get_debug_link_info_1
 
 SYNOPSIS
-	char *bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out);
+	char *bfd_get_debug_link_info_1 (bfd *abfd, void *crc32_out);
 
 DESCRIPTION
-	Fetch the filename and CRC32 value for any separate debuginfo
-	associated with @var{abfd}.  Return NULL if no such info found,
-	otherwise return filename and update @var{crc32_out}.  The
-	returned filename is allocated with @code{malloc}; freeing it
-	is the responsibility of the caller.
+	bfd_get_debug_link_info Fetch the filename and CRC32 value for
+	any separate debuginfo associated with @var{abfd}.  Return
+	NULL if no such info found, otherwise return filename and
+	update the CRC32 pointed at by @var{crc32_out}.  The returned
+	filename is allocated with @code{malloc}; freeing it is the
+	responsibility of the caller.
+
+	The @var{crc32_out} parameter is an untyped pointer because
+	this routine is used as a @code{get_func_type} function.
 */
 
-char *
-bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out)
+static char *
+bfd_get_debug_link_info_1 (bfd *abfd, void *crc32_out)
 {
   asection *sect;
-  unsigned long crc32;
+  unsigned long *crc32 = (unsigned long *) crc32_out;
   bfd_byte *contents;
   unsigned int crc_offset;
   char *name;
@@ -1203,12 +1207,32 @@ bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out)
   if (crc_offset >= bfd_get_section_size (sect))
     return NULL;
 
-  crc32 = bfd_get_32 (abfd, contents + crc_offset);
-
-  *crc32_out = crc32;
+  *crc32 = bfd_get_32 (abfd, contents + crc_offset);
   return name;
 }
 
+
+/*
+FUNCTION
+	bfd_get_debug_link_info
+
+SYNOPSIS
+	char *bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out);
+
+DESCRIPTION
+	Fetch the filename and CRC32 value for any separate debuginfo
+	associated with @var{abfd}.  Return NULL if no such info found,
+	otherwise return filename and update @var{crc32_out}.  The
+	returned filename is allocated with @code{malloc}; freeing it
+	is the responsibility of the caller.
+*/
+
+char *
+bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out)
+{
+  return bfd_get_debug_link_info_1 (abfd, crc32_out);
+}
+
 /*
 FUNCTION
 	bfd_get_alt_debug_link_info
@@ -1271,22 +1295,30 @@ INTERNAL_FUNCTION
 
 SYNOPSIS
 	bfd_boolean separate_debug_file_exists
-	  (char *name, unsigned long crc32);
+	  (char *name, void *crc32_p);
 
 DESCRIPTION
 	Checks to see if @var{name} is a file and if its contents
-	match @var{crc32}.
+	match @var{crc32}, which is a pointer to an @code{unsigned
+	long} containing a CRC32.
+
+	The @var{crc32_p} parameter is an untyped pointer because
+	this routine is used as a @code{check_func_type} function.
 */
 
 static bfd_boolean
-separate_debug_file_exists (const char *name, const unsigned long crc)
+separate_debug_file_exists (const char *name, void *crc32_p)
 {
   static unsigned char buffer [8 * 1024];
   unsigned long file_crc = 0;
   FILE *f;
   bfd_size_type count;
+  unsigned long crc;
 
   BFD_ASSERT (name);
+  BFD_ASSERT (crc32_p);
+
+  crc = *(unsigned long *) crc32_p;
 
   f = _bfd_real_fopen (name, FOPEN_RB);
   if (f == NULL)
@@ -1306,16 +1338,14 @@ INTERNAL_FUNCTION
 
 SYNOPSIS
 	bfd_boolean separate_alt_debug_file_exists
-	  (char *name, unsigned long buildid);
+	  (char *name, void *unused);
 
 DESCRIPTION
-	Checks to see if @var{name} is a file and if its BuildID
-	matches @var{buildid}.
+	Checks to see if @var{name} is a file.
 */
 
 static bfd_boolean
-separate_alt_debug_file_exists (const char *name,
-				const unsigned long buildid ATTRIBUTE_UNUSED)
+separate_alt_debug_file_exists (const char *name, void *unused ATTRIBUTE_UNUSED)
 {
   FILE *f;
 
@@ -1343,32 +1373,40 @@ SYNOPSIS
 
 DESCRIPTION
 	Searches for a debug information file corresponding to @var{abfd}.
-	The name of the separate debug info file is returned by the @var{get}
-	function.  This function scans various fixed locations in the
-	filesystem, including the file tree rooted at @var{dir}.  If the
-	@var{include_dirs} parameter is true then the directory components of
-	@var{abfd}'s filename will be included in the searched locations.
-
-	Returns the filename of the first file to be found which receives a
-	TRUE result from the @var{check} function.  Returns NULL if no valid
-	file could be found.
+
+	The name of the separate debug info file is returned by the
+	@var{get_func} function.  This function scans various fixed
+	locations in the filesystem, including the file tree rooted at
+	@var{debug_file_directory}.  If the @var{include_dirs}
+	parameter is true then the directory components of
+	@var{abfd}'s filename will be included in the searched
+	locations.
+
+	@var{func_data} is passed unmodified to the @var{get_func} and
+	@var{check_func} functions.  It is generally used to implement
+	build-id-like matching in the callback functions.
+
+	Returns the filename of the first file to be found which
+	receives a TRUE result from the @var{check_func} function.
+	Returns NULL if no valid file could be found.
+
 */
 
-typedef char *      (* get_func_type) (bfd *, unsigned long *);
-typedef bfd_boolean (* check_func_type) (const char *, const unsigned long);
+typedef char *      (* get_func_type) (bfd *, void *);
+typedef bfd_boolean (* check_func_type) (const char *, void *);
 
 static char *
 find_separate_debug_file (bfd *           abfd,
 			  const char *    debug_file_directory,
 			  bfd_boolean     include_dirs,
 			  get_func_type   get_func,
-			  check_func_type check_func)
+			  check_func_type check_func,
+			  void *          func_data)
 {
   char *base;
   char *dir;
   char *debugfile;
   char *canon_dir;
-  unsigned long crc32;
   size_t dirlen;
   size_t canon_dirlen;
 
@@ -1383,7 +1421,7 @@ find_separate_debug_file (bfd *           abfd,
       return NULL;
     }
 
-  base = get_func (abfd, & crc32);
+  base = get_func (abfd, func_data);
 
   if (base == NULL)
     return NULL;
@@ -1457,19 +1495,19 @@ find_separate_debug_file (bfd *           abfd,
      a file into the root filesystem.  (See binutils/testsuite/
      binutils-all/objdump.exp for the test).  */
   sprintf (debugfile, "%s%s", dir, base);
-  if (check_func (debugfile, crc32))
+  if (check_func (debugfile, func_data))
     goto found;
 
   /* Then try in a subdirectory called .debug.  */
   sprintf (debugfile, "%s.debug/%s", dir, base);
-  if (check_func (debugfile, crc32))
+  if (check_func (debugfile, func_data))
     goto found;
 
 #ifdef EXTRA_DEBUG_ROOT1
   /* Try the first extra debug file root.  */
   sprintf (debugfile, "%s%s%s", EXTRA_DEBUG_ROOT1,
 	   include_dirs ? canon_dir : "/", base);
-  if (check_func (debugfile, crc32))
+  if (check_func (debugfile, func_data))
     goto found;
 #endif
 
@@ -1477,7 +1515,7 @@ find_separate_debug_file (bfd *           abfd,
   /* Try the second extra debug file root.  */
   sprintf (debugfile, "%s%s%s", EXTRA_DEBUG_ROOT2,
 	   include_dirs ? canon_dir : "/", base);
-  if (check_func (debugfile, crc32))
+  if (check_func (debugfile, func_data))
     goto found;
 #endif
   
@@ -1499,7 +1537,7 @@ find_separate_debug_file (bfd *           abfd,
     }
   strcat (debugfile, base);
 
-  if (check_func (debugfile, crc32))
+  if (check_func (debugfile, func_data))
     goto found;
 
   /* Failed to find the file.  */
@@ -1542,24 +1580,24 @@ RETURNS
 char *
 bfd_follow_gnu_debuglink (bfd *abfd, const char *dir)
 {
+  unsigned long crc32;
+
   return find_separate_debug_file (abfd, dir, TRUE,
-				   bfd_get_debug_link_info,
-				   separate_debug_file_exists);
+				   bfd_get_debug_link_info_1,
+				   separate_debug_file_exists, &crc32);
 }
 
-/* Helper for bfd_follow_gnu_debugaltlink.  It just pretends to return
-   a CRC.  .gnu_debugaltlink supplies a build-id, which is different,
-   but this is ok because separate_alt_debug_file_exists ignores the
-   CRC anyway.  */
+/* Helper for bfd_follow_gnu_debugaltlink.  It just returns the name
+   of the separate debug file.  */
+
 
 static char *
-get_alt_debug_link_info_shim (bfd * abfd, unsigned long *crc32_out)
+get_alt_debug_link_info_shim (bfd * abfd, void *unused ATTRIBUTE_UNUSED)
 {
   bfd_size_type len;
   bfd_byte *buildid = NULL;
   char *result = bfd_get_alt_debug_link_info (abfd, &len, &buildid);
 
-  *crc32_out = 0;
   free (buildid);
 
   return result;
@@ -1595,7 +1633,8 @@ bfd_follow_gnu_debugaltlink (bfd *abfd, const char *dir)
 {
   return find_separate_debug_file (abfd, dir, TRUE,
 				   get_alt_debug_link_info_shim,
-				   separate_alt_debug_file_exists);
+				   separate_alt_debug_file_exists,
+				   NULL);
 }
 
 /*
@@ -1850,7 +1889,7 @@ INTERNAL_FUNCTION
 
 SYNOPSIS
 	char * get_build_id_name
-	  (bfd *abfd, unsigned long *build_id_out)
+	  (bfd *abfd, void *build_id_out_p)
 
 DESCRIPTION
 	Searches @var{abfd} for a build-id, and then constructs a pathname
@@ -1860,13 +1899,15 @@ DESCRIPTION
 	Returns the constructed filename or NULL upon error.
 	It is the caller's responsibility to free the memory used to hold the
 	filename.
-	If a filename is returned then the @var{build_id_out} parameter is
-	set to a pointer to the build_id structure.
+	If a filename is returned then the @var{build_id_out_p}
+	parameter (which points to a @code{struct bfd_build_id}
+	pointer) is set to a pointer to the build_id structure.
 */
 
 static char *
-get_build_id_name (bfd *abfd, unsigned long *build_id_out)
+get_build_id_name (bfd *abfd, void *build_id_out_p)
 {
+  struct bfd_build_id **build_id_out = build_id_out_p;
   struct bfd_build_id *build_id;
   char *name;
   char *n;
@@ -1901,7 +1942,7 @@ get_build_id_name (bfd *abfd, unsigned long *build_id_out)
     n += sprintf (n, "%02x", (unsigned) *d++);
   n += sprintf (n, ".debug");
 
-  * build_id_out = (unsigned long) build_id;
+  *build_id_out = build_id;
   return name;
 }
 
@@ -1911,19 +1952,19 @@ INTERNAL_FUNCTION
 
 SYNOPSIS
 	bfd_boolean check_build_id_file
-	  (char *name, unsigned long buildid);
+	  (char *name, void *buildid_p);
 
 DESCRIPTION
 	Checks to see if @var{name} is a readable file and if its build-id
 	matches @var{buildid}.
 
-	Returns TRUE if the file exists, is readable, and contains a build-id
-	which matches @var{build-id}.
+	Returns TRUE if the file exists, is readable, and contains a
+	build-id which matches the build-id pointed at by
+	@var{build_id_p} (which is really a @code{struct bfd_build_id **}).
 */
 
 static bfd_boolean
-check_build_id_file (const char *name,
-		     const unsigned long buildid)
+check_build_id_file (const char *name, void *buildid_p)
 {
   struct bfd_build_id *orig_build_id;
   struct bfd_build_id *build_id;
@@ -1931,7 +1972,7 @@ check_build_id_file (const char *name,
   bfd_boolean result;
 
   BFD_ASSERT (name);
-  BFD_ASSERT (buildid);
+  BFD_ASSERT (buildid_p);
 
   file = bfd_openr (name, NULL);
   if (file == NULL)
@@ -1951,7 +1992,7 @@ check_build_id_file (const char *name,
       return FALSE;
     }
 
-  orig_build_id = (struct bfd_build_id *) buildid;
+  orig_build_id = *(struct bfd_build_id **) buildid_p;
 
   result = build_id->size == orig_build_id->size
     && memcmp (build_id->data, orig_build_id->data, build_id->size) == 0;
@@ -1993,7 +2034,9 @@ RETURNS
 char *
 bfd_follow_build_id_debuglink (bfd *abfd, const char *dir)
 {
+  struct bfd_build_id *build_id;
+
   return find_separate_debug_file (abfd, dir, FALSE,
 				   get_build_id_name,
-				   check_build_id_file);
+				   check_build_id_file, &build_id);
 }
-- 
2.5.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]