Re: [PATCH 3/6] gdb: make user-created frames reinflatable

[Simon Marchi <simon.marchi@polymtl.ca>]

>> diff --git a/gdb/frame-info.c b/gdb/frame-info.c
>> index 40a872ea152d..d61fb7ed0e95 100644
>> --- a/gdb/frame-info.c
>> +++ b/gdb/frame-info.c
>> @@ -21,6 +21,9 @@
>>     #include "frame-info.h"
>>   #include "frame.h"
>> +#include "gdbsupport/selftest.h"
>> +#include "scoped-mock-context.h"
>> +#include "test-target.h"
>>     /* See frame-info-ptr.h.  */
>>   @@ -33,7 +36,8 @@ frame_info_ptr::prepare_reinflate ()
>>   {
>>     m_cached_level = frame_relative_level (*this);
>>   -  if (m_cached_level != 0)
>> +  if (m_cached_level != 0
>> +      || (m_ptr != nullptr && frame_is_user_created (m_ptr)))
>>       m_cached_id = get_frame_id (*this);
>>   }
>>   @@ -54,7 +58,13 @@ frame_info_ptr::reinflate ()
>>       /* Frame #0 needs special handling, see comment in select_frame.  */
>>     if (m_cached_level == 0)
>> -    m_ptr = get_current_frame ().get ();
>> +    {
>> +      if (!frame_id_p (m_cached_id))

> You seem to be using the stack being valid to check if the frame is

stack -> frame id?

> user created, but in the commit message you mention that you use null
> frame id. Wouldn't it be more reliable to check if m_cached_id ==
> null_frame_id ?

Comparing anything against null_frame_id (even a null frame id) yields
false:

  if (stack_status == FID_STACK_INVALID
      || r.stack_status == FID_STACK_INVALID)
    /* Like a NaN, if either ID is invalid, the result is false.
       Note that a frame ID is invalid iff it is the null frame ID.  */
    eq = false;

From: https://gitlab.com/gnutools/binutils-gdb/-/blob/a28fedbc3f582ce7c8bad2eb017b1dc072bb1da7/gdb/frame.c#L759-763

I don't really understand the point, I think it would be useful to be
able to compare a frame id to null_frame_id, like we compare pointers to
nullptr.  But currently, the correct (and only?) way of checking if we
have a frame id or not in a frame_id is frame_id_p.

When the commit message says:

  for user-created frames, m_cached_id is a non-null frame id, whereas
  for the current target frame, m_cached_id is left null.

I could say "is valid" and "is invalid" instead.

Simon