Hi Ulrich, Please find attached the patch with all the changes mentioned. Kindly let me know if any more changes is need. If not kindly check this in. Have a nice day ahead. Thanks and regards, Aditya. >>@@ -508,14 +552,17 @@ pdc_read_data (pthdb_user_t user_current_pid, void *buf, >> /* This is needed to eliminate the dependency of current thread >> which is null so that thread reads the correct target memory. */ >> { >>- scoped_restore_current_thread restore_current_thread; >>+ scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid); >> /* Before the first inferior is added, we pass inferior_ptid.pid () >> from pd_enable () which is 0. There is no need to switch threads >> during first initialisation. In the rest of the callbacks the >> current thread needs to be correct. */ >This comment is no longer relevant as the code relating to it was >deleted. The comment should be deleted as well. >>- if (user_current_pid != 0) >>- switch_to_thread (current_inferior ()->process_target (), >>- ptid_t (user_current_pid)); >>+ inferior_ptid = ptid_t (user_current_pid); >>+ scoped_restore_current_inferior restore_inferior; >>+ set_current_inferior (inf); >>+ >>+ scoped_restore_current_program_space restore_current_progspace; >>+ set_current_program_space (inf->pspace); >> status = target_read_memory (addr, (gdb_byte *) buf, len); Done.. >>+ tp = find_thread_ptid (proc_target, ptid_t (pid)); >>+ >>+ /* If the pthreadlibrary is not ready to debug >>+ then this is just a main process which needs >>+ a priv to be set. The if condition below does >>+ the same. Otherwise we go to the for loop to >>+ sync the pthread and GDB thread lists. */ >These changes all seem to be leftovers from previous attempts, >I guess they should be removed again. Done.. >>+ inferior *inf = current_inferior (); >>+ /* When attaching / handling fork child, don't try loading libthread_db >>+ until we know about all shared libraries. */ >>+ if (inf->in_initial_library_scan) >>+ return; >"libthread_db" is Linux specific. Please update the comment so >it makes sense in the AIX context. Done.. >>@@ -1362,12 +1439,16 @@ aix_thread_target::fetch_registers (struct regcache *regcache, int regno) >>+ thread = find_thread_ptid (current_inferior ()->process_target (), ptid_t (regcache->ptid ().pid (), 0, regcache->ptid ().tid ())); >I hadn't seen this change below, it doesn't really make sense to me. >You really need to use regcache->ptid here, this should be correct. >When did you see a case where this was not correct? Does this still >happen now that we have the in_initial_library_scan check? This works with that flag change. Removed it. Thanks. From: Ulrich Weigand Date: Friday, 17 February 2023 at 7:48 PM To: simark@simark.ca , Aditya Kamath1 , gdb-patches@sourceware.org Cc: Sangamesh Mallayya Subject: Re: [PATCH] 0001-Fix-multi-thread-debug-bug-in-AIX.patch Aditya Kamath1 wrote: >Yes, it resolves the issue. Excellent. A few final comment on the patch, including one change I hadn't noticed before: >@@ -508,14 +552,17 @@ pdc_read_data (pthdb_user_t user_current_pid, void *buf, > /* This is needed to eliminate the dependency of current thread > which is null so that thread reads the correct target memory. */ > { >- scoped_restore_current_thread restore_current_thread; >+ scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid); > /* Before the first inferior is added, we pass inferior_ptid.pid () > from pd_enable () which is 0. There is no need to switch threads > during first initialisation. In the rest of the callbacks the > current thread needs to be correct. */ This comment is no longer relevant as the code relating to it was deleted. The comment should be deleted as well. >- if (user_current_pid != 0) >- switch_to_thread (current_inferior ()->process_target (), >- ptid_t (user_current_pid)); >+ inferior_ptid = ptid_t (user_current_pid); >+ scoped_restore_current_inferior restore_inferior; >+ set_current_inferior (inf); >+ >+ scoped_restore_current_program_space restore_current_progspace; >+ set_current_program_space (inf->pspace); > status = target_read_memory (addr, (gdb_byte *) buf, len); > } > ret = status == 0 ? PDC_SUCCESS : PDC_FAILURE; >+ tp = find_thread_ptid (proc_target, ptid_t (pid)); >+ >+ /* If the pthreadlibrary is not ready to debug >+ then this is just a main process which needs >+ a priv to be set. The if condition below does >+ the same. Otherwise we go to the for loop to >+ sync the pthread and GDB thread lists. */ >+ > /* Apply differences between the two arrays to GDB's thread list. */ >+ > for (pi = gi = 0; pi < pcount || gi < gcount;) These changes all seem to be leftovers from previous attempts, I guess they should be removed again. >+ inferior *inf = current_inferior (); >+ /* When attaching / handling fork child, don't try loading libthread_db >+ until we know about all shared libraries. */ >+ if (inf->in_initial_library_scan) >+ return; "libthread_db" is Linux specific. Please update the comment so it makes sense in the AIX context. >@@ -1362,12 +1439,16 @@ aix_thread_target::fetch_registers (struct regcache *regcache, int regno) > { > struct thread_info *thread; > pthdb_tid_t tid; >+ thread = find_thread_ptid (current_inferior ()->process_target (), ptid_t (regcache->ptid ().pid (), 0, regcache->ptid ().tid ())); > >- if (!PD_TID (regcache->ptid ())) >+ /* If a new inferior is born, then its pthread debug library is yet to >+ initialised and hence has no private data. So the below if condition >+ exists. */ >+ >+ if (regcache->ptid ().tid () == 0) > beneath ()->fetch_registers (regcache, regno); > else > { >- thread = find_thread_ptid (current_inferior (), regcache->ptid ()); > aix_thread_info *priv = get_aix_thread_info (thread); > tid = priv->tid; I hadn't seen this change below, it doesn't really make sense to me. You really need to use regcache->ptid here, this should be correct. When did you see a case where this was not correct? Does this still happen now that we have the in_initial_library_scan check? Bye, Ulrich