Re: [PATCH v2] gdb/arm: Stop unwinding on error, but do not assert

[Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>]

Hi Pedro,

On 2022-10-13 13:21, Pedro Alves wrote:
> On 2022-10-13 10:17 a.m., Torbjörn SVENSSON via Gdb-patches wrote:
> 
>> +  /* Unwind of this frame is not possible.  Return outer_frame_id to stop the
>> +     unwinding.  */
>> +  if (cache == NULL)
>> +    {
>> +      *this_id = outer_frame_id;
>> +      return;
>> +    }
> 
> Please let's not add more uses of outer_frame_id if we can avoid it.  We're getting
> close to eliminating it.  Can a cache object still be returned, and then a frame id
> be successfully computed?

The problem is that it's not always possible to know what registers that 
was written on the stack or if there was padding between 2 frames on the 
stack.
If a cache object is returned, wouldn't that imply that the content of 
this frame is supposed to be valid?

> You can stop the unwinding in some other way.  For example, arm_m_exception_cache has a few
> of these:
> 
> 	      /* Terminate any further stack unwinding by referring to self.  */
> 	      arm_cache_set_active_sp_value (cache, tdep, sp);
> 	      return cache;
> 
> (Not sure exactly how that works.)

I'm behind the some of those statements in arm-tdep.c, but the construct 
was copied from some other place in the GDB sources. I think there is 
some code in GDB that checks if 2 frames have the same SP value and in 
that case, stops the unwinding.

> Alternatively, you can implement a frame_unwind::stop_reason callback and return
> UNWIND_OUTERMOST, which is already done in arm-tdep.c in other scenarios too.

Is it guaranteed that the prev_register method won't be called for a 
cache object that have the UNWIND_OUTERMOST stop reason? If so, I 
suppose the struct arm_prologue_cache could be extended with another 
member that indicates if the frame was successfully unwinded or if there 
were some problem and the UNWIND_OUTERMOST should be returned. Would 
this be okay?

Kind regards,
Torbjörn