[Bug python/16324] New: py-finishbreakpoint.c incorrect frame_id_eq

[jan.kratochvil at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16324

Bug ID: 16324
Summary: py-finishbreakpoint.c incorrect frame_id_eq
Product: gdb
Version: HEAD
Status: NEW
Severity: normal
Priority: P2
Component: python
Assignee: unassigned at sourceware dot org
Reporter: jan.kratochvil at redhat dot com

if (frame_id_eq (frame_id, null_frame_id))
PyErr_SetString (PyExc_ValueError,
_("Invalid ID for the `frame' object."));

it can never happen as frame_id_eq (null_frame_id, null_frame_id) is false:
/* Like a NaN, if either ID is invalid, the result is false.
+
/* Make the sentinel frame's ID valid, but invalid. That way all
comparisons with it should fail. */
frame->this_id.p = 1;
frame->this_id.value = null_frame_id;

[pmuldoon at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16324

Phil Muldoon <pmuldoon at redhat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
CC| |pmuldoon at redhat dot com
Assignee|unassigned at sourceware dot org |pmuldoon at redhat dot com

--- Comment #1 from Phil Muldoon <pmuldoon at redhat dot com> ---
It seems the correct test here is to use frame_id_p which checks if a valid
stack is found (with a simple check to frame_id.stack_status !=
FID_STACK_INVALID). It also checks the frame_id is not an outer frame id.

There is no explicit check for a null_frame_id, which I found odd. get_frame_id
can return a null_frame_id, but lots of callers don't check the return so there
is a latent bug there too.

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=16324

Pedro Alves <palves at redhat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |palves at redhat dot com

--- Comment #2 from Pedro Alves <palves at redhat dot com> ---
The right thing to do is to just remove the dead code. No frame can ever have
a null_frame_id id nowadays. compute_frame_id would assert before that could
happen.