From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11740 invoked by alias); 14 Feb 2012 19:23:56 -0000 Received: (qmail 11729 invoked by uid 22791); 14 Feb 2012 19:23:54 -0000 X-SWARE-Spam-Status: No, hits=-0.4 required=5.0 tests=AWL,BAYES_20,RCVD_NUMERIC_HELO,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from plane.gmane.org (HELO plane.gmane.org) (80.91.229.3) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Feb 2012 19:23:39 +0000 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1RxNyU-0004dJ-2g for gdb-patches@sources.redhat.com; Tue, 14 Feb 2012 20:23:38 +0100 Received: from 209.226.137.107 ([209.226.137.107]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 14 Feb 2012 20:23:38 +0100 Received: from aristovski by 209.226.137.107 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 14 Feb 2012 20:23:38 +0100 To: gdb-patches@sources.redhat.com From: Aleksandar Ristovski Subject: Re: [patch] Assert when 'break' with no arguments Date: Tue, 14 Feb 2012 20:09:00 -0000 Message-ID: <4F3AB4AE.2010504@qnx.com> References: <20120214185333.GB14803@adacore.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------060900060407040000000108" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20120129 Thunderbird/10.0 In-Reply-To: <20120214185333.GB14803@adacore.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-02/txt/msg00281.txt.bz2 This is a multi-part message in MIME format. --------------060900060407040000000108 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2140 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 > > 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 * 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 * gdb.base/break-inline.exp: New test. * gdb.base/break-inline.c: New test. Thanks, Aleksandar --------------060900060407040000000108 Content-Type: text/x-patch; name="pspace-assert-201202141406.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pspace-assert-201202141406.patch" Content-length: 1086 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. */ --------------060900060407040000000108 Content-Type: text/plain; name="break-inline.exp" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="break-inline.exp" Content-length: 927 # 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 . 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.*" --------------060900060407040000000108 Content-Type: text/x-csrc; name="break-inline.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="break-inline.c" Content-length: 872 /* 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 . */ static int g; static inline void foo(void) { g = 42; } int main(int argc, char *argv[]) { foo(); return g; } --------------060900060407040000000108--