[RFA] Ensure 'exec-file has changed' check has priority over 'exec-file-mismatch' check

[RFA] Ensure 'exec-file has changed' check has priority over 'exec-file-mismatch' check

[Philippe Waroquiers]

Following the implementation of exec-file-mismatch based on build-id,
an attach to a process that runs a modified exec-file was triggering
the exec-file-mismatch handling, giving a warning such as:
  warning: Mismatch between current exec-file /bd/home/philippe/gdb/git/build_termours/gdb/testsuite/outputs/gdb.base/attach/attach
  and automatically determined exec-file /bd/home/philippe/gdb/git/build_termours/gdb/testsuite/outputs/gdb.base/attach/attach
  exec-file-mismatch handling is currently "ask"
as the build-ids differ when an exec-file is recompiled.

This patch ensures that the exec-file-mismatch check is done with an up to date
build-id.  With this, exec-file-mismatch check will only trigger when the
PID file really differs from the (build-id refreshed) current exec-file.
Note that the additional check does not (yet) reload the symbols if
the exec-file is changed: this reload will happen later if needed.

gdb/ChangeLog
YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* exec.c (validate_exec_file): Ensure the build-id is up to
	date by calling reopen_exec_file (that checks file timestamp
	to decide to re-read the file).

gdb/testsuite/ChangeLog

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/attach.exp: Test priority of 'exec-file' changed
	over 'exec-file-mismatch'.
	* gdb.base/attach.c: Mark should_exit volatile.
	* gdb.base/attach2.c: Likewise.  Add a comment explaining
	why the sleep cannot be big.
	* gdb.base/attach3.c: New file.
---
 gdb/exec.c                        | 10 ++++++
 gdb/testsuite/gdb.base/attach.c   |  2 +-
 gdb/testsuite/gdb.base/attach.exp | 56 +++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.base/attach2.c  |  4 ++-
 4 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index 14c77495a3..021c9ea350 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -254,6 +254,16 @@ validate_exec_file (int from_tty)
   if (current_exec_file == NULL || pid_exec_file == NULL)
     return;
 
+   /* Try validating via build-id, if available.  This is the most
+      reliable check.  */
+
+  /* In case current_exec_file was changed, reopen_exec_file ensures
+     an up to date build_id (will do nothing if the file timestamp
+     did not change).  If exec file changed, reopen_exec_file has
+     allocated another file name, so get_exec_file again.  */
+  reopen_exec_file ();
+  current_exec_file = get_exec_file (0);
+
   /* Try validating via build-id, if available.  This is the most
      reliable check.  */
   const bfd_build_id *exec_file_build_id = build_id_bfd_get (exec_bfd);
diff --git a/gdb/testsuite/gdb.base/attach.c b/gdb/testsuite/gdb.base/attach.c
index 2e87f9b710..b3c5498401 100644
--- a/gdb/testsuite/gdb.base/attach.c
+++ b/gdb/testsuite/gdb.base/attach.c
@@ -8,7 +8,7 @@
 #include <unistd.h>
 
 int  bidule = 0;
-int  should_exit = 0;
+volatile int  should_exit = 0;
 
 int main ()
 {
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 32f72e2a9a..84aede7e2a 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -17,12 +17,13 @@ if {![can_spawn_for_attach]} {
     return 0
 }
 
-standard_testfile attach.c attach2.c
+standard_testfile attach.c attach2.c attach3.c
 set binfile2 ${binfile}2
+set binfile3 ${binfile}3
 set escapedbinfile  [string_to_regexp $binfile]
 
 #execute_anywhere "rm -f ${binfile} ${binfile2}"
-remote_exec build "rm -f ${binfile} ${binfile2}"
+remote_exec build "rm -f ${binfile} ${binfile2} ${binfile3}"
 # For debugging this test
 #
 #log_user 1
@@ -41,6 +42,13 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable {d
     return -1
 }
 
+# Build the third file, used to check attach when exec-file has changed.
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile3}" "${binfile3}" executable {debug}] != "" } {
+    untested "failed to compile attach exec-file changed test"
+    return -1
+}
+
 if [get_compiler_info] {
     return -1
 }
@@ -515,6 +523,7 @@ proc_with_prefix do_attach_exec_mismatch_handling_tests {} {
     global gdb_prompt
     global binfile
     global binfile2
+    global binfile3
 
     clean_restart $binfile
 
@@ -577,10 +586,53 @@ proc_with_prefix do_attach_exec_mismatch_handling_tests {} {
     # Detach the process.
     gdb_test "detach" "Detaching from program: .* detached\\\]" "$test detach attach"
 
+    # Test that the 'exec-file' changed is checked before exec-file-mismatch.
+    set test "mismatch exec-file changed has priority"
+    gdb_test_no_output "set exec-file-mismatch ask"
+    gdb_test_multiple "attach $testpid" "$test attach1 again, initial exec-file" {
+	-re "Attaching to program.*exec-file-mismatch handling is currently \"ask\".*Load new symbol table from .*attach\".*\(y or n\)" {
+	    pass "$test attach1 again"
+	}
+    }
+    gdb_test "y" "Reading symbols from .*attach.*" "$test load attach1 again"
+
+    gdb_test "detach" "Detaching from program: .* detached\\\]" "$test detach attach initial exec-file"
+
+    # Change the exec-file and attach to a new process using the changed file.
+    remote_exec build "mv ${binfile} ${binfile}.initial"
+    remote_exec build "mv ${binfile3} ${binfile}"
+    # Ensure GDB detects ${binfile} has changed when checking timestamp.
+    sleep 1
+    remote_exec build "touch ${binfile}"
+    set test_spawn_id3 [spawn_wait_for_attach $binfile]
+    set testpid3 [spawn_id_get_pid $test_spawn_id3]
+
+    gdb_test "attach $testpid3" "Attaching to program.*attach' has changed; re-reading symbols.*" \
+	"$test attach1 again, after changing exec-file"
+    gdb_test "detach" "Detaching from program: .* detached\\\]" "$test detach after attach changed exec-file"
+
+    # Now, test the situation when current exec-file has changed
+    # and we attach to a pid using another file.
+    # Ensure GDB detects ${binfile} has changed when checking timestamp.
+    sleep 1
+    remote_exec build "touch ${binfile}"
+
+    gdb_test_multiple "attach $testpid2" "$test attach2" {
+	-re "Attaching to program.*exec-file-mismatch handling is currently \"ask\".*Load new symbol table from .*attach2\".*\(y or n\)" {
+	    pass "$test attach2 with exec-file changed and need to load another exec-file"
+	}
+    }
+    gdb_test "y" "Reading symbols from .*attach2.*" \
+	"$test load attach2 exec-file changed and load another exec file"
+
+    # Restore initial build situation.
+    remote_exec build "mv ${binfile} ${binfile3}"
+    remote_exec build "mv ${binfile}.initial ${binfile}"
 
     # Don't leave a process around
     kill_wait_spawned_process $test_spawn_id
     kill_wait_spawned_process $test_spawn_id2
+    kill_wait_spawned_process $test_spawn_id3
 }
 
 do_attach_tests
diff --git a/gdb/testsuite/gdb.base/attach2.c b/gdb/testsuite/gdb.base/attach2.c
index 44d37258fb..d070d933b0 100644
--- a/gdb/testsuite/gdb.base/attach2.c
+++ b/gdb/testsuite/gdb.base/attach2.c
@@ -9,12 +9,14 @@
 #include <unistd.h>
 
 float  bidule = 0.0;
-int  should_exit = 0;
+volatile int  should_exit = 0;
 
 int main ()
 {
   int  local_i = 0;
 
+  /* Cannot sleep a very long time, as attach.exp assumes the
+     process will exit before the standard GDB timeout.  */
   sleep( 10 ); /* System call causes register fetch to fail */
                /* This is a known HPUX "feature"            */
   while (! should_exit)
-- 
2.20.1

Re: [RFA] Ensure 'exec-file has changed' check has priority over 'exec-file-mismatch' check

[Pedro Alves]

Hi Philippe,

On 5/23/20 2:08 PM, Philippe Waroquiers via Gdb-patches wrote:
> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/attach.exp: Test priority of 'exec-file' changed
> 	over 'exec-file-mismatch'.
> 	* gdb.base/attach.c: Mark should_exit volatile.
> 	* gdb.base/attach2.c: Likewise.  Add a comment explaining
> 	why the sleep cannot be big.
> 	* gdb.base/attach3.c: New file.
> ---
>  gdb/exec.c                        | 10 ++++++
>  gdb/testsuite/gdb.base/attach.c   |  2 +-
>  gdb/testsuite/gdb.base/attach.exp | 56 +++++++++++++++++++++++++++++--
>  gdb/testsuite/gdb.base/attach2.c  |  4 ++-
>  4 files changed, 68 insertions(+), 4 deletions(-)

attach3.c is missing from the patch.

Thanks,
Pedro Alves