Re: [PATCH] Remove MAX_REGISTER_SIZE from py-unwind.c

[Alan Hayward <Alan.Hayward@arm.com>]

> On 22 Jun 2017, at 14:22, Pedro Alves <palves@redhat.com> wrote:
> 
> On 06/22/2017 02:13 PM, Alan Hayward wrote:
> 
>> Ok, pushed with changes as suggested.
>> 
>> Patch below.
> 
> Sorry, but this looks broken to me.
> 
> cached_frame_info is using the trailing array idiom ...
> 
>> @@ -93,7 +84,7 @@ typedef struct
>>   /* Length of the `reg' array below.  */
>>   int reg_count;
>> 
>> -  struct reg_info reg[];
>> +  cached_reg_t reg[];
>> } cached_frame_info;
>> 
> 
>> 
>> -    cached_frame
>> -      = ((cached_frame_info *)
>> -	 xmalloc (sizeof (*cached_frame)
>> -		  + reg_count * sizeof (cached_frame->reg[0])));
>> +    cached_frame = XNEW (cached_frame_info);
> 
> but now you're not allocating enough space for the array elements...
> 
> 
>>     cached_frame->gdbarch = gdbarch;
>>     cached_frame->frame_id = unwind_info->frame_id;
>>     cached_frame->reg_count = reg_count;
>> @@ -580,13 +568,14 @@ pyuw_sniffer (const struct frame_unwind *self, struct frame_info *this_frame,
>>         struct value *value = value_object_to_value (reg->value);
>>         size_t data_size = register_size (gdbarch, reg->number);
>> 
>> -        cached_frame->reg[i].number = reg->number;
>> +	cached_frame->reg[i].num = reg->number;
> 
> ... that you're accessing here and below.
> 
> Valgrind probably shows the now-out-of-bounds accesses.
> 
> 

Ouch! My mistake.

Quick patch to revert that line to it’s previous state.

Ok?


2017-06-22  Alan Hayward  <alan.hayward@arm.com>

	* python/py-unwind.c (pyuw_sniffer): Allocate space for
	registers.


diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index da1703ecf2b843d9a6790f095e64c2915280ae35..1d800a7b7861da6a168440b600fb2862c42b4e4b 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -557,7 +557,10 @@ pyuw_sniffer (const struct frame_unwind *self, struct frame_info *this_frame,
     saved_reg *reg;
     int i;

-    cached_frame = XNEW (cached_frame_info);
+    cached_frame
+      = ((cached_frame_info *)
+	 xmalloc (sizeof (*cached_frame)
+		  + reg_count * sizeof (cached_frame->reg[0])));
     cached_frame->gdbarch = gdbarch;
     cached_frame->frame_id = unwind_info->frame_id;
     cached_frame->reg_count = reg_count;