From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6385 invoked by alias); 16 Oct 2007 00:18:29 -0000 Received: (qmail 6373 invoked by uid 22791); 16 Oct 2007 00:18:28 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.45.13) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 16 Oct 2007 00:18:24 +0000 Received: from zps38.corp.google.com (zps38.corp.google.com [172.25.146.38]) by smtp-out.google.com with ESMTP id l9G0IJSG014668 for ; Mon, 15 Oct 2007 17:18:19 -0700 Received: from localhost (ruffy.corp.google.com [172.18.118.116]) by zps38.corp.google.com with ESMTP id l9G0IHqr024254; Mon, 15 Oct 2007 17:18:17 -0700 Received: by localhost (Postfix, from userid 67641) id F059B1C7E69; Mon, 15 Oct 2007 17:18:16 -0700 (PDT) To: gdb-patches@sourceware.org cc: dje@google.com Subject: RFA: patch to fix multi-breakpoint enable/disable handling of inline functions Message-Id: <20071016001816.F059B1C7E69@localhost> Date: Tue, 16 Oct 2007 04:12:00 -0000 From: dje@google.com (Doug Evans) 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: 2007-10/txt/msg00412.txt.bz2 Consider: dje@ruffy:~/gnu/sourceware/head/obj/gdb/testsuite/gdb.cp$ ~/gnu/sourceware/head/rel/bin/gdb mb-inline GNU gdb 6.7.50-20071009-cvs Copyright (C) 2007 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "i386-linux"... Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1". (gdb) b mb-inline.h:6 Breakpoint 1 at 0x80483e9: file ../../../src/gdb/testsuite/gdb.cp/mb-inline.h, line 6. (2 locations) (gdb) i b Num Type Disp Enb Address What 1 breakpoint keep y 1.1 y 0x080483e9 in foo at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6 1.2 y 0x0804843d in foo at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6 (gdb) dis 1.2 (gdb) i b Num Type Disp Enb Address What 1 breakpoint keep y 1.1 y 0x080483e9 in foo at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6 1.2 n 0x0804843d in foo at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6 (gdb) r Starting program: /home/dje/gnu/sourceware/head/obj/gdb/testsuite/gdb.cp/mb-inline Breakpoint 1, foo (i=1) at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6 6 return i; // set breakpoint here (gdb) i b Num Type Disp Enb Address What 1 breakpoint keep y breakpoint already hit 1 time 1.1 n 0x080483e9 in foo at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6 1.2 y 0x0804843d in foo at ../../../src/gdb/testsuite/gdb.cp/mb-inline.h:6 (gdb) Note that 1.1 is now disabled and 1.2 is enabled. Here's a patch. There is a heuristic involved in knowing when to not compare function names. I've tried to come up with something reasonable. 2007-10-15 Doug Evans * breakpoint.c (ambiguous_names_p): New fn. (update_breakpoint_locations): When restoring bp enable status, don't compare function names if all functions have same name. * gdb.cp/mb-ctor.exp: Check skip_cplus_tests. * gdb.cp/mb-templates.exp: Check skip_cplus_tests. * gdb.cp/mb-inline.exp: New. * gdb.cp/mb-inline.h: New. * gdb.cp/mb-inline1.cc: New. * gdb.cp/mb-inline2.cc: New. Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.274 diff -u -u -p -r1.274 breakpoint.c --- breakpoint.c 12 Oct 2007 16:11:11 -0000 1.274 +++ breakpoint.c 15 Oct 2007 23:47:54 -0000 @@ -7496,6 +7496,37 @@ all_locations_are_pending (struct bp_loc return 1; } +/* Return non-zero if multiple fns in list LOC have the same name. + Null names are ignored. + This returns zero if there's <= one named function, there's no ambiguity. + ??? This tests for ambiguity with the first named function, it doesn't + handle the case of multiple ambiguities. This can be fixed at the cost of + some complexity in the caller, but it's unknown if this is a practical + issue. */ + +static int +ambiguous_names_p (struct bp_location *loc) +{ + struct bp_location *l; + const char *name = NULL; + + for (l = loc; l != NULL; l = l->next) + { + /* Allow for some names to be NULL, ignore them. */ + if (l->function_name == NULL) + continue; + if (name == NULL) + { + name = l->function_name; + continue; + } + if (strcmp (name, l->function_name) == 0) + return 1; + } + + return 0; +} + static void update_breakpoint_locations (struct breakpoint *b, struct symtabs_and_lines sals) @@ -7558,18 +7589,37 @@ update_breakpoint_locations (struct brea /* If possible, carry over 'disable' status from existing breakpoints. */ { struct bp_location *e = existing_locations; + /* If there are multiple breakpoints with the same function name, + e.g. for inline functions, comparing function names won't work. + Instead compare pc addresses; this is just a heuristic as things + may have moved, but in practice it gives the correct answer + often enough until a better solution is found. */ + int have_ambiguous_names = ambiguous_names_p (b->loc); + for (; e; e = e->next) { if (!e->enabled && e->function_name) { struct bp_location *l = b->loc; - for (; l; l = l->next) - if (l->function_name - && strcmp (e->function_name, l->function_name) == 0) - { - l->enabled = 0; - break; - } + if (have_ambiguous_names) + { + for (; l; l = l->next) + if (e->address == l->address) + { + l->enabled = 0; + break; + } + } + else + { + for (; l; l = l->next) + if (l->function_name + && strcmp (e->function_name, l->function_name) == 0) + { + l->enabled = 0; + break; + } + } } } } Index: testsuite/gdb.cp/mb-ctor.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-ctor.exp,v retrieving revision 1.1 diff -u -u -p -r1.1 mb-ctor.exp --- testsuite/gdb.cp/mb-ctor.exp 24 Sep 2007 07:40:32 -0000 1.1 +++ testsuite/gdb.cp/mb-ctor.exp 15 Oct 2007 23:47:55 -0000 @@ -21,6 +21,8 @@ if $tracelevel then { strace $tracelevel } +if { [skip_cplus_tests] } { continue } + set prms_id 0 set bug_id 0 Index: testsuite/gdb.cp/mb-templates.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/mb-templates.exp,v retrieving revision 1.1 diff -u -u -p -r1.1 mb-templates.exp --- testsuite/gdb.cp/mb-templates.exp 24 Sep 2007 07:40:32 -0000 1.1 +++ testsuite/gdb.cp/mb-templates.exp 15 Oct 2007 23:47:55 -0000 @@ -21,6 +21,8 @@ if $tracelevel then { strace $tracelevel } +if { [skip_cplus_tests] } { continue } + set prms_id 0 set bug_id 0 --- testsuite/gdb.cp/mb-inline.exp= 1969-12-31 16:00:00.000000000 -0800 +++ testsuite/gdb.cp/mb-inline.exp 2007-10-15 16:44:36.858766000 -0700 @@ -0,0 +1,105 @@ +# Copyright 2007 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 . + +# This file is part of the gdb testsuite. + +# This test verifies that setting breakpoint on line in template +# function will fire in all instantiations of that template. + +if $tracelevel then { + strace $tracelevel +} + +if { [skip_cplus_tests] } { continue } + +set prms_id 0 +set bug_id 0 + +set testfile "mb-inline" +set hdrfile "${testfile}.h" +set srcfile1 "${testfile}1.cc" +set objfile1 "${testfile}1.o" +set srcfile2 "${testfile}2.cc" +set objfile2 "${testfile}2.o" +set binfile "${objdir}/${subdir}/${testfile}" + +if { [gdb_compile "$srcdir/$subdir/$srcfile1" "$objdir/$subdir/$objfile1" object {debug c++}] != "" } { + untested rtti.exp + return -1 +} + +if { [gdb_compile "$srcdir/$subdir/$srcfile2" "$objdir/$subdir/$objfile2" object {debug c++}] != "" } { + untested rtti.exp + return -1 +} + +if { [gdb_compile "$objdir/$subdir/$objfile1 $objdir/$subdir/$objfile2" "${binfile}" executable {debug c++}] != "" } { + untested rtti.exp + return -1 +} + +if [get_compiler_info ${binfile} "c++"] { + return -1 +} + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +set bp_location [gdb_get_line_number "set breakpoint here" $hdrfile] + +# Set a breakpoint with multiple locations. + +gdb_test "break $hdrfile:$bp_location" \ + "Breakpoint.*at.* file .*$hdrfile, line.*\\(2 locations\\).*" \ + "set breakpoint" + +gdb_run_cmd +gdb_expect { + -re "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*$gdb_prompt $" { + pass "run to breakpoint" + } + -re "$gdb_prompt $" { + fail "run to breakpoint" + } + timeout { + fail "run to breakpoint (timeout)" + } +} + +gdb_test "continue" \ + ".*Breakpoint.*foo \\(i=1\\).*" \ + "run to breakpoint 2" + +# Try disabling a single location. We also test +# that at least in simple cases, the enable/disable +# state of locations survive "run". +# Testing 1.2 here is important - early bug would disable 1.1 when program +# is run. +gdb_test "disable 1.2" "" "disabling location: disable" + +gdb_run_cmd +gdb_expect { + -re "Breakpoint \[0-9\]+,.*foo \\(i=0\\).*$gdb_prompt $" { + pass "disabling location: run to breakpoint" + } + -re "$gdb_prompt $" { + fail "disabling location: run to breakpoint" + } + timeout { + fail "disabling location: run to breakpoint (timeout)" + } +} --- testsuite/gdb.cp/mb-inline.h= 1969-12-31 16:00:00.000000000 -0800 +++ testsuite/gdb.cp/mb-inline.h 2007-10-15 16:40:42.908114000 -0700 @@ -0,0 +1,10 @@ +// Test gdb support for setting file:line breakpoints on inline fns. + +static inline int +foo (int i) +{ + return i; // set breakpoint here +} + +extern int afn (); +extern int bfn (); --- testsuite/gdb.cp/mb-inline1.cc= 1969-12-31 16:00:00.000000000 -0800 +++ testsuite/gdb.cp/mb-inline1.cc 2007-10-15 16:42:02.644024000 -0700 @@ -0,0 +1,17 @@ +// Test gdb support for setting file:line breakpoints on inline fns. + +#include "mb-inline.h" + +int +afn () +{ + return foo (0); +} + +int +main () +{ + int a = afn (); + int b = bfn (); + return a * b; +} --- testsuite/gdb.cp/mb-inline2.cc= 1969-12-31 16:00:00.000000000 -0800 +++ testsuite/gdb.cp/mb-inline2.cc 2007-10-15 16:42:06.315615000 -0700 @@ -0,0 +1,7 @@ +#include "mb-inline.h" + +int +bfn () +{ + return foo (1); +}