gdb: unify build-id to objfile lookup code

There are 3 places where we currently call debuginfod_exec_query to
lookup an objfile for a given build-id.

In one of these places we first call build_id_to_exec_bfd which also
looks up an objfile given a build-id, but this function looks on disk
for a symlink in the .build-id/ sub-directory (within the
debug-file-directory).

I can't think of any reason why we shouldn't call build_id_to_exec_bfd
before every call to debuginfod_exec_query.

So, in this commit I have added a new function in build-id.c,
find_objfile_by_build_id, this function calls build_id_to_exec_bfd,
and if that fails, then calls debuginfod_exec_query.

Everywhere we call debuginfod_exec_query is updated to call the new
function, and in locate_exec_from_corefile_build_id, the existing call
to build_id_to_exec_bfd is removed as calling find_objfile_by_build_id
does this for us.

One slight weird thing is in core_target::build_file_mappings, here we
call find_objfile_by_build_id which returns a gdb_bfd_ref_ptr for the
opened file, however we immediately reopen the file as "binary".  The
reason for this is that all the bfds opened in ::build_file_mappings
need to be opened as "binary" (see the function comments for why).

I did consider passing a target type into find_objfile_by_build_id,
which could then be forwarded to build_id_to_exec_bfd and used to open
the BFD as "binary", however, if you follow the call chain you'll end
up in build_id_to_debug_bfd_1, where we actually open the bfd.  Notice
in here that we call build_id_verify to double check the build-id of
the file we found, this requires that the bfd not be opened as
"binary".

What this means is that we always have to first open the bfd using the
gnutarget target type (for the build-id check), and then we would have
to reopen it as "binary".  There seems little point pushing the reopen
logic into find_objfile_by_build_id, so we just do this in the
::build_file_mappings function.

I've extended the tests to cover the two cases which actually changed
in this commit.

2 files changed, 55 insertions, 1 deletions

diff --git a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp
index 6e3301e..b5dee22 100644
--- a/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp
+++ b/gdb/testsuite/gdb.debuginfod/corefile-mapped-file.exp
@@ -296,6 +296,30 @@ set ptr_value [read_ptr_value]
 gdb_assert { $ptr_value eq "unavailable" } \
     "check value of pointer is unavailable with library file missing"
 
+# Now symlink the .build-id/xx/xxx...xxx filename within the debug
+# directory to library we just moved aside.  Restart GDB and setup the
+# debug-file-directory before loading the core file.
+#
+# GDB should lookup the file to map via the build-id link in the
+# .build-id/ directory.
+set debugdir [standard_output_file "debugdir"]
+set build_id_filename \
+    $debugdir/[build_id_debug_filename_get $library_backup_filename ""]
+
+remote_exec build "mkdir -p [file dirname $build_id_filename]"
+remote_exec build "ln -sf $library_backup_filename $build_id_filename"
+
+clean_restart $binfile
+
+gdb_test_no_output "set debug-file-directory $debugdir" \
+    "set debug-file-directory"
+
+load_core_file "load corefile, lookup in debug-file-directory"
+
+set ptr_value [read_ptr_value]
+gdb_assert { $ptr_value == $ptr_expected_value } \
+    "check value of pointer variable from core-file, lookup in debug-file-directory"
+
 # Build a new version of the shared library, keep the library the same size,
 # but change the contents so the build-id changes.  Then restart GDB and load
 # the core-file again.  GDB should spot that the build-id for the shared
diff --git a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp
index 5bf6817..9ef1204 100644
--- a/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp
+++ b/gdb/testsuite/gdb.debuginfod/solib-with-soname.exp
@@ -126,10 +126,19 @@ if {$corefile eq ""} {
 # If EXPECT_DOWNLOAD is true then we require a line indicating that
 # the shared library is being downloaded from debuginfod, otherwise
 # the shared library should not be downloaded.
-proc load_exec_and_core_file { expect_warning expect_download testname } {
+#
+# If DEBUGDIR is not the empty string then 'debug-file-directory' is
+# set to the value of DEBUGDIR.
+proc load_exec_and_core_file { expect_warning expect_download testname \
+				   {debugdir ""} } {
     with_test_prefix $testname {
 	clean_restart $::binfile
 
+	if { $debugdir ne "" } {
+	    gdb_test_no_output "set debug-file-directory $debugdir" \
+		"set debug directory"
+	}
+
 	set saw_warning false
 	set saw_download false
 	set saw_generated false
@@ -223,6 +232,27 @@ gdb_assert { [lindex $status 0] == 0 } \
 load_exec_and_core_file true false \
     "load core file, libfoo_1.so removed"
 
+# Symlink the .build-id/xx/xxx...xxx filename within the debug
+# directory to LIBRARY_1_BACKUP_FILENAME, now when we restart GDB it
+# should find the missing library within the debug directory.
+set debugdir [standard_output_file "debugdir"]
+set build_id_filename \
+    $debugdir/[build_id_debug_filename_get $library_1_backup_filename ""]
+set status \
+    [remote_exec build \
+	 "mkdir -p [file dirname $build_id_filename]"]
+gdb_assert { [lindex $status 0] == 0 } \
+    "create sub-directory within the debug directory"
+set status \
+    [remote_exec build \
+	 "ln -sf $library_1_backup_filename $build_id_filename"]
+gdb_assert { [lindex $status 0] == 0 } \
+    "create symlink within the debug directory "
+
+load_exec_and_core_file false false \
+    "load core file, find libfoo_1.so through debug-file-directory" \
+    $debugdir
+
 # Setup a debuginfod server which can serve the original shared
 # library file.
 if {![allow_debuginfod_tests]} {