Re: [PATCH] "tfind" across unavailable-stack frames.

[Pedro Alves <palves@redhat.com>]

On 12/14/2013 06:21 AM, Yao Qi wrote:
> On 12/14/2013 01:49 AM, Pedro Alves wrote:
>> gdb/
>> 2013-12-13  Pedro Alves  <palves@redhat.com>
>>
>> 	* frame.h (enum frame_id_stack_status): New enum.
>> 	(struct frame_id) <stack_addr>: Adjust comment.
>> 	<stack_addr_p>: Delete field, replaced with ...
>> 	<stack_status>: ... this new field.
>> 	(frame_id_build_unavailable_stack): Declare.
>> 	* frame.c (frame_addr_hash, fprint_field, outer_frame_id)
>> 	(frame_id_build_special): Adjust.
>> 	(frame_id_build_unavailable_stack): New function.
>> 	(frame_id_build, frame_id_build_wild): Adjust.
>> 	(frame_id_p, frame_id_eq, frame_id_inner): Adjust to take into
>> 	account frames with unavailable stack.
>>
>> 	* amd64-tdep.c (amd64_frame_this_id)
>> 	(amd64_sigtramp_frame_this_id, amd64_epilogue_frame_this_id): Use
>> 	frame_id_build_unavailable_stack.
>> 	* dwarf2-frame.c (dwarf2_frame_this_id): Likewise.
> 
> Do we need to update tailcall_frame_this_id as well?  

In case of trace frame debugging, I don't think you can ever
have a tailcall frame on top of a frame with unavailable
stack, because dwarf2_tailcall_sniffer_first will throw
an error computing prev_sp.  But even if that would work
somehow:

static void
tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
			struct frame_id *this_id)
{
  struct tailcall_cache *cache = *this_cache;
  struct frame_info *next_frame;

  /* Tail call does not make sense for a sentinel frame.  */
  next_frame = get_next_frame (this_frame);
  gdb_assert (next_frame != NULL);

  *this_id = get_frame_id (next_frame);
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  (*this_id).code_addr = get_frame_pc (this_frame);
  (*this_id).code_addr_p = 1;
  (*this_id).artificial_depth = (cache->chain_levels
				 - existing_next_levels (this_frame, cache));
  gdb_assert ((*this_id).artificial_depth > 0);
}

... the tailcall frame's ID's stack components (address
and status) are copied from the next frame's stack components.
Seems right to me.

>> @@ -169,6 +187,12 @@ extern struct frame_id frame_id_build_special (CORE_ADDR stack_addr,
>>  					       CORE_ADDR code_addr,
>>  					       CORE_ADDR special_addr);
>>  
>> +/* Construct a frame ID representing a frame where the stack address
>> +   exists, but is unavailable.  The first parameter is the frame's
> 
> Remove "first"? since we only have one parameter here.

Indeed.  Fixed locally.

> 
>> +   constant code address (typically the entry point).  The special
>> +   identifier address is set to indicate a wild card.  */
>> +extern struct frame_id frame_id_build_unavailable_stack (CORE_ADDR code_addr);
>> +
> 
> What does the last sentence mean in the comments?
> 

The same comment is in frame_id_build.  Re. what is means, see struct frame_id:

  /* The frame's special address.  This shall be constant through out the
     lifetime of the frame.  This is used for architectures that may have
     frames that do not change the stack but are still distinct and have
     some form of distinct identifier (e.g. the ia64 which uses a 2nd
     stack for registers).  This field is treated as unordered - i.e. will
     not be used in frame ordering comparisons.

     This field is valid only if special_addr_p is true.  Otherwise, this
     frame is considered to have a wildcard special address, i.e. one that
                                                             ^^^^^^^^^^^^^
     matches every address value in frame comparisons.  */
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  CORE_ADDR special_addr;

-- 
Pedro Alves