This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB 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]

Re: [PATCH v2 01/15] Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol creation)


On 2018-04-10 17:19, Pedro Alves wrote:
On 04/01/2018 04:35 AM, Simon Marchi wrote:
On 2018-03-25 03:19 PM, Pedro Alves wrote:

/* This search algorithm is from _bfd_elf_canonicalize_dynamic_reloc. */ for (relplt = obfd->sections; relplt != NULL; relplt = relplt->next)
-    if (elf_section_data (relplt)->this_hdr.sh_info == plt_elf_idx
-	&& (elf_section_data (relplt)->this_hdr.sh_type == SHT_REL
-	    || elf_section_data (relplt)->this_hdr.sh_type == SHT_RELA))
-      break;
+    {
+      const auto &this_hdr = elf_section_data (relplt)->this_hdr;
+
+ if (this_hdr.sh_type == SHT_REL || this_hdr.sh_type == SHT_RELA)
+	{
+	  asection *target_section = NULL;
+
+	  if (this_hdr.sh_info == plt_elf_idx)
+	    target_section = plt;
+	  else if (this_hdr.sh_info == got_plt_elf_idx)
+	    target_section = got_plt;
+
+	  if (target_section != NULL)
+	    break;

Is it really useful to have/set target_section? Couldn't we just break out of the
loop like this?

  if (this_hdr.sh_info == plt_elf_idx
      || this_hdr.sh_info == got_plt_elf_idx)
    break;


Hmm, the original intention was to use target_section in the other
loop, but that didn't work, so I reverted it, but somehow not that
part.  :-P


@@ -573,6 +586,18 @@ elf_rel_plt_read (minimal_symbol_reader &reader,

   std::string string_buffer;

+  /* Does ADDRESS reside in SECTION of OBFD?  */
+ auto within_section = [obfd] (asection *section, CORE_ADDR address)
+    {
+      if (section == NULL)
+	return false;
+
+      /* Does the pointer reside in the .got.plt section?  */

That comment should change, since it's not stricly .got.plt.


I've removed it, since the intro comment already says it all.

Or maybe you intended to use target_section here at some point? Is there a relationship between the section that matched in the for loop above and the section that will contain the address? In other words, could we save the
target_section from above and do

Yeah, in an earlier version I tried doing that, but then testing on the
different systems found out that there's no relation between the
two sections.

Here's the updated patch.  WDYT?

LGTM (though I trust the passing tests more than I trust myself).

Simon


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