Re: [patch] Assert when 'break' with no arguments

[Pedro Alves <palves@redhat.com>]

On 02/24/2012 03:54 PM, Aleksandar Ristovski wrote:
> On 12-02-24 10:41 AM, Pedro Alves wrote:
>> On 02/14/2012 08:44 PM, Aleksandar Ristovski wrote:
>>> --- gdb/stack.c    7 Feb 2012 04:48:22 -0000    1.247
>>> +++ gdb/stack.c    14 Feb 2012 20:43:08 -0000
>>> @@ -909,6 +909,11 @@ set_last_displayed_sal (int valid, struc
>>>     last_displayed_addr = addr;
>>>     last_displayed_symtab = symtab;
>>>     last_displayed_line = line;
>>> +  if (valid&&  pspace == NULL)
>>> +    {
>>> +      warning (_("Trying to set NULL pspace."));
>>
>> Is there a case when this isn't a gdb bug?  IOW, any reason this isn't a gdb_assert?
>> This seems like a rather cryptic warning to show to a user.
> 
> I believe it would be a bug.
> 
> However, gdb_assert is rather destructive. The warning is to bring it to our attention, but continue so users can actually do debugging if this happens.
> 
> My thinking was: if sal is invalidated the rest of the code should continue functioning thus providing more value to the user (albeit maybe somewhat confused user) than killing the session.
> 
> Anyway that was my rationale.

Trouble is that cryptic warnings usually end up unnoticed
and unreported (users will just shrug at gdb's lameness).

If we clear the last displayed sal before issuing an internal
error, then we're more sure to catch the bug, and users can still say
no to "Quit this debugging session? (y or n)" and continue
debugging.  WDYT?

diff --git a/gdb/stack.c b/gdb/stack.c
index 070d658..22b16a5 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -911,8 +911,9 @@ set_last_displayed_sal (int valid, struct program_space *pspace,
   last_displayed_line = line;
   if (valid && pspace == NULL)
     {
-      warning (_("Trying to set NULL pspace."));
       clear_last_displayed_sal ();
+      internal_error (__FILE__, __LINE__,
+		      _("Trying to set NULL pspace."));
     }
 }


-- 
Pedro Alves