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

Jiong Wang
2013-02-20T02:49:10Z

[-- 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