Re: [RFC/TileGX 2/6] simplify the handling of skip prologue for plt stub

[Jiong Wang <jiwang@tilera.com>]

[-- Attachment #1: Type: text/plain, Size: 2368 bytes --]


> On 01/18/2013 11:12 PM, Jiong Wang wrote:
>> this is because tilegx skip_prologue will invoke
>> tilegx_analyze_prologue, which
>> will prefetch 32*8 bytes.
>>
>> while for when the address is in plt stub,  you can see it near the
>> eh_frame_hdr section
>>
>>     [14] .plt                         0000000000010a00 000a00 0000a0 28
>> AX  0   0 64
>>     ...
>>     [16] .eh_frame_hdr     0000000000010ac0 000ac0 000024 00 A  0 0  4
>>     [17] .eh_frame             0000000000010ae8 000ae8 0000b4 00   A 
>> 0   0  8
>>
>> the .eh_frame_hdr aligns to 4, there is a hole between .eh_frame_hdr and
>> .eh_frame, and this
>> will cause trouble for section_table_xfer_memory_partial.
>>
>> after fetch memory starting from 0x10ac0 to 0x10ae4, then the memaddr
>> will be 0x10ae4 in section_table_xfer_memory_partial,
>> while this function did not consider this hole situation, so goes to
>> line 666, error occured.
>
> Wang Jiong,
>
> AFAICT, the root cause of this problem is GDB prefetches too much 
> contents in one time that exceeds the boundary of a section.
>
> At the beginning of tilegx_analyze_prologue, I notice this comment
>
>   /* To cut down on round-trip overhead, we fetch multiple bundles
>      at once.  These variables describe the range of memory we have
>      prefetched.  */
>
> Can't we fetch one bundle in one time?  Fetching multiple bundles 
> causes this problem, so we have to disable it.

I think we should keep prefetching multiple instruction bundles to cut 
down on round-trip overhead, just as the comment explained.

>
> Even we still decide to use fetching multiple bundle in one time, we 
> should take care of the boundary and existing code does this, see this 
> comment,
>
>       /* Figure out how many bytes to fetch.  Don't span a page
>          boundary since that might cause an unnecessary memory
>          error.  */
>
> Looks existing code takes care of not crossing the page boundary, 
> similarly, we should also take care of not crossing the section 
> boundary.  What do you think?

thanks, check section boundary looks better, and I think we can remove 
the old page boundary check, please CR the new patch

gdb/ChangeLog:

         * tilegx-tdep.c (tilegx_skip_prologue): when prefetching
         multiple instruction bundles, check section boundary
         instead of page boundary.

Regards,
Jiong






[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: section.patch.patch --]
[-- Type: text/plain; charset="gb18030"; name="section.patch.patch", Size: 1204 bytes --]

---
 gdb/tilegx-tdep.c | 12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/tilegx-tdep.c b/gdb/tilegx-tdep.c
index 2c4e349..f8a6255 100644
--- a/gdb/tilegx-tdep.c
+++ b/gdb/tilegx-tdep.c
@@ -424,15 +424,18 @@ tilegx_analyze_prologue (struct gdbarch* gdbarch,
       /* Retrieve the next instruction.  */
       if (next_addr - instbuf_start >= instbuf_size)
 	{
-	  /* Figure out how many bytes to fetch.  Don't span a page
+	  /* Figure out how many bytes to fetch.  Don't span a section
 	     boundary since that might cause an unnecessary memory
 	     error.  */
-	  unsigned int size_on_same_page = 4096 - (next_addr & 4095);
+	  unsigned int size_on_same_section;
+	  struct obj_section *s = find_pc_section(next_addr);
+	  gdb_assert(s != NULL);
+	  size_on_same_section =
+	    s->the_bfd_section->vma + s->the_bfd_section->size - next_addr;
 
 	  instbuf_size = sizeof instbuf;
 
-	  if (instbuf_size > size_on_same_page)
-	    instbuf_size = size_on_same_page;
+	  if (instbuf_size > size_on_same_section)
+	    instbuf_size = size_on_same_section;
 	  instbuf_start = next_addr;
 
 	  status = safe_frame_unwind_memory (next_frame, instbuf_start,
-- 
1.8.1