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

[Aleksandar Ristovski <aristovski@qnx.com>]

[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]

On 12-02-14 01:53 PM, Joel Brobecker wrote:
>> ChangeLog:
>
> I haven't looked at the validity of the patch (Pedro has a better
> understanding of this area, for instance), but I still noticed some
> trivial deviations from the GNU Coding Standards.
>
> Your ChangeLog entry, for instance. Lines should be folded at 70 chars
> (hard limit is 80 chars). Other violations highlighted below:

Ok.

>
>> +      /* Set pspace with frame's pspace */
>
> End the sentence with a period (and two spaces before the '*/').

I removed the comment. It is stating what code unambiguously states already.

>
>> +  if (valid&&  pspace == NULL) {
>> +	warning(_("Trying to set NULL pspace."));
>> +  }
>
> Wrong formatting for first and second line.

Yes, thank you. Moved 'warning' and check down.

>
> Why are you adding an empty line here?
>
>> #   Copyright 2012 Free
>> #   Software Foundation, Inc.
>
> Missing (C), and please join the two lines. If you copied some of
> the testcase from another testcase, then you need to preserve the
> original copyright years, I think.

Added '(C)'. I copied only the copyright notice, the tests are new.

>> #include<stdio.h>

>
> Is there a way to trigger the same problem without the dependency
> on stdio.h? Many systems do not provide it (bareboard). I would
> think that all you need is to define a function that has the same
> profile as printf, no?
>

Yes, I removed printf and stdio, they are not important and the issue is 
still reproduced (the only important thing is that foo actually gets 
inlined which -O2 optimization level should do).

New patch, fixed tests attached.

Change logs:
2012-02-14  Aleksandar Ristovski  <aristovski@qnx.com>

     * frame.c (find_frame_sal): Initialise sal->pspace field from frame
     data.
     * stack.c (set_last_displayed_sal): Perform sanity check of the data
     passed in, in particular, validate that PSPACE is not NULL if 
requesting
     valid last_displayed_* data.


Testsuite:
2012-02-14  Aleksandar Ristovski  <aristovski@qnx.com>

     * gdb.base/break-inline.exp: New test.
     * gdb.base/break-inline.c: New test.






Thanks,

Aleksandar




[-- Attachment #2: pspace-assert-201202141406.patch --]
[-- Type: text/x-patch, Size: 1086 bytes --]

Index: gdb/frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.304
diff -u -p -r1.304 frame.c
--- gdb/frame.c	4 Jan 2012 08:17:02 -0000	1.304
+++ gdb/frame.c	14 Feb 2012 19:05:52 -0000
@@ -2096,6 +2096,8 @@ find_frame_sal (struct frame_info *frame
 	   we can't do much better.  */
 	sal->pc = get_frame_pc (frame);
 
+      sal->pspace = get_frame_program_space (frame);
+
       return;
     }
 
Index: gdb/stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.247
diff -u -p -r1.247 stack.c
--- gdb/stack.c	7 Feb 2012 04:48:22 -0000	1.247
+++ gdb/stack.c	14 Feb 2012 19:05:52 -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."));
+      clear_last_displayed_sal ();
+    }
 }
 
 /* Forget the last sal we displayed.  */

[-- Attachment #3: break-inline.exp --]
[-- Type: text/plain, Size: 927 bytes --]

#   Copyright (C) 2012 Free Software Foundation, Inc.

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.


if { [prepare_for_testing break-inline.exp "break-inline" {} {debug nowarnings optimize=-O2}] } {
    return -1
}

gdb_test "start" "Temporary breakpoint.*foo().*"

# Now test 'break' with no arguments.
gdb_test "break" "Breakpoint.*"


[-- Attachment #4: break-inline.c --]
[-- Type: text/x-csrc, Size: 872 bytes --]

/* This testcase is part of GDB, the GNU debugger.

   Copyright (C) 2012 Free Software
   Foundation, Inc.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 3 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

static int g;
static inline void foo(void)
{
  g = 42;
}
int main(int argc, char *argv[])
{
  foo();
  return g;
}