From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17862 invoked by alias); 9 Mar 2009 22:07:50 -0000 Received: (qmail 17854 invoked by uid 22791); 9 Mar 2009 22:07:48 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 09 Mar 2009 22:07:41 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n29M7eNF013669 for ; Mon, 9 Mar 2009 18:07:40 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n29M7en5024744 for ; Mon, 9 Mar 2009 18:07:40 -0400 Received: from host0.dyn.jankratochvil.net (sebastian-int.corp.redhat.com [172.16.52.221]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n29M7dmj023809 for ; Mon, 9 Mar 2009 18:07:39 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id n29M7bBE027989 for ; Mon, 9 Mar 2009 23:07:37 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.2/Submit) id n29M7a8K027986 for gdb-patches@sourceware.org; Mon, 9 Mar 2009 23:07:36 +0100 Date: Mon, 09 Mar 2009 22:17:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Subject: [patch] Fix internal error on breaking at a multi-locations caller Message-ID: <20090309220736.GA27259@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.18 (2008-05-17) 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: 2009-03/txt/msg00132.txt.bz2 Hi, patch deals with break after `up' into the caller where the caller line has multiple locations (instances). Discussion is contained in the code. Original bugreport at: https://bugzilla.redhat.com/show_bug.cgi?id=488572 No regressions on x86_64-unknown-linux-gnu. Thanks, Jan gdb/ 2009-03-09 Jan Kratochvil Fix internal error on breaking at a multi-locations caller source line. * breakpoint.c (expand_line_sal_maybe): Remove the variable `found'. (expand_line_sal_maybe = 2>): New initialized variable `best'. Set `expanded.sals[best].pc'. New comment. gdb/testsuite/ 2009-03-09 Jan Kratochvil * gdb.cp/expand-sals.exp, gdb.cp/expand-sals.cc: New. --- gdb/breakpoint.c 6 Mar 2009 18:51:05 -0000 1.382 +++ gdb/breakpoint.c 9 Mar 2009 21:55:50 -0000 @@ -5189,7 +5189,6 @@ expand_line_sal_maybe (struct symtab_and struct symtabs_and_lines expanded; CORE_ADDR original_pc = sal.pc; char *original_function = NULL; - int found; int i; /* If we have explicit pc, don't expand. @@ -5265,14 +5264,42 @@ expand_line_sal_maybe (struct symtab_and if (original_pc) { - found = 0; + /* Find all the other PCs for a line of code with multiple instances + (locations). If the instruction is in the middle of an instruction + block for source line GDB cannot safely find the same instruction in + the other compiled instances of the same source line because the other + instances may have been compiled completely differently. + + The testcase gdb.cp/expand-sals.exp shows that breaking at the return + address in a caller of the current frame works for the current + instance but the breakpoint cannot catch the point (instruction) where + the callee returns in the other compiled instances of this source line. + + The current implementation will place the breakpoint at the expected + returning address of the current instance of the caller. But the + other instances will get the breakpoint at the first instruction of + the source line - therefore before the call would be made. Another + possibility would be to place the breakpoint in the other instances at + the start of the next source line. + + A possible heuristics would compare the instructions length of each of + the instances of the current source line and if it matches it would + place the breakpoint at the same offset. Unfortunately a mistaken + guess would possibly crash the inferior. */ + + CORE_ADDR best = -1; + + /* Find the nearest preceding PC and set it to ORIGINAL_PC. */ for (i = 0; i < expanded.nelts; ++i) - if (expanded.sals[i].pc == original_pc) - { - found = 1; - break; - } - gdb_assert (found); + if (expanded.sals[i].pc <= original_pc + && (best == -1 || expanded.sals[best].pc < expanded.sals[i].pc)) + best = i; + + if (best == -1) + error (_("Cannot find the best address for %s out of the %d locations"), + paddr (original_pc), expanded.nelts); + + expanded.sals[best].pc = original_pc; } return expanded; --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gdb/testsuite/gdb.cp/expand-sals.cc 9 Mar 2009 21:55:51 -0000 @@ -0,0 +1,53 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright (C) 2009 Free Software Foundation, Inc. + + This file is part of GDB. + + 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 . */ + +int +func () +{ + return 42; /* func-line */ +} + +volatile int global_x; + +class A +{ +public: + A () + { + global_x = func (); /* caller-line */ + } +}; + +/* class B is here just to make the `func' calling line above having multiple + instances - multiple locations. Template cannot be used as its instances + would have different function names which get discarded by GDB + expand_line_sal_maybe. */ + +class B : public A +{ +}; + +int +main (void) +{ + A a; + B b; + + return 0; /* exit-line */ +} --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ gdb/testsuite/gdb.cp/expand-sals.exp 9 Mar 2009 21:55:51 -0000 @@ -0,0 +1,100 @@ +# Copyright 2009 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 { [skip_cplus_tests] } { continue } + +set srcfile expand-sals.cc +if { [prepare_for_testing expand-sals.exp expand-sals $srcfile {debug c++}] } { + return -1 +} +if ![runto_main] { + return -1 +} + +gdb_breakpoint [gdb_get_line_number "exit-line"] + +gdb_breakpoint [gdb_get_line_number "func-line"] +gdb_continue_to_breakpoint "func" ".*func-line.*" + +gdb_test "up" "caller-line.*" + +# PC should not be at the boundary of source lines to make the original bug +# exploitable. + +set test "p/x \$pc" +set pc {} +gdb_test_multiple $test $test { + -re "\\$\[0-9\]+ = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" { + set pc $expect_out(1,string) + pass $test + } +} + +set test "info line" +set end {} +gdb_test_multiple $test $test { + -re "Line \[0-9\]+ of .* starts at address 0x\[0-9a-f\]+.* and ends at (0x\[0-9a-f\]+).*\\.\r\n$gdb_prompt $" { + set end $expect_out(1,string) + pass $test + } +} + +set test "caller line has trailing code" +if {$pc != $end} { + pass $test +} else { + fail $test +} + +# Original problem was an internal error here. Still sanity multiple locations +# were found at this code place as otherwise this test would not test anything. +set test "break" +gdb_test_multiple $test $test { + -re "Breakpoint \[0-9\]+ at .*, line \[0-9\]+\\. \\(\[2-9\] locations\\)\r\n$gdb_prompt $" { + pass $test + } + -re "Breakpoint \[0-9\]+ at .*, line \[0-9\]+\\.\r\n$gdb_prompt $" { + # It just could not be decided if GDB is OK by this testcase. + setup_xfail *-*-* + fail $test + return 0 + } +} + +gdb_continue_to_breakpoint "caller" ".*caller-line.*" + +# Test GDB caught this return call and not the next one through B::B() +gdb_test "bt" \ + "#0 \[^\r\n\]* A \[^\r\n\]*\r\n#1 \[^\r\n\]* main \[^\r\n\]*" \ + "bt from A" + +gdb_continue_to_breakpoint "next caller instance" ".*caller-line.*" + +# Test that GDB caught now already A through B::B() in the other instance. +# As discussed in GDB expand_line_sal_maybe it would more match the original +# instance behavior to catch here the `func' breakpoint and catch the +# multiple-locations breakpoint only during the call return. This is not the +# case, expecting here to catch the breakpoint before the call happens. + +gdb_test "bt" \ + "#0 \[^\r\n\]* A \[^\r\n\]*\r\n#1 \[^\r\n\]* B \[^\r\n\]*\r\n#2 \[^\r\n\]* main \[^\r\n\]*" \ + "bt from B before the call" + +gdb_continue_to_breakpoint "next caller func" ".*func-line.*" + +# Verify GDB really could not catch the originally intended point of the return +# from func. + +gdb_continue_to_breakpoint "uncaught return" ".*exit-line.*"