Message ID | 1465854882-42527-1-git-send-email-donb@codesourcery.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 22448 invoked by alias); 13 Jun 2016 21:54:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 22325 invoked by uid 89); 13 Jun 2016 21:54:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=1999-2016, UD:n, 19992016 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 13 Jun 2016 21:54:46 +0000 Received: from svr-orw-fem-02x.mgc.mentorg.com ([147.34.96.206] helo=SVR-ORW-FEM-02.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1bCZoh-0000kO-GU from Don_Breazeal@mentor.com ; Mon, 13 Jun 2016 14:54:43 -0700 Received: from build2-trusty-cs (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.3.224.2; Mon, 13 Jun 2016 14:54:43 -0700 Received: by build2-trusty-cs (Postfix, from userid 1905) id D354D420A22; Mon, 13 Jun 2016 14:54:42 -0700 (PDT) From: Don Breazeal <donb@codesourcery.com> To: <gdb-patches@sourceware.org>, <andrew.burgess@embecosm.com> Subject: [PATCH v2] Fix -var-update for registers in frames 1 and up Date: Mon, 13 Jun 2016 14:54:42 -0700 Message-ID: <1465854882-42527-1-git-send-email-donb@codesourcery.com> In-Reply-To: <20160613090459.GM26734@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes |
Commit Message
Don Breazeal
June 13, 2016, 9:54 p.m. UTC
This is an updated version of a patch to fix access to registers for frames other than the current frame via var-update. The piece of the patch affecting GDB proper has been completely re-written. It changes the initialization of varobj->root->frame in varobj_create so that existing mechanisms (with minimal changes) select the correct frame and provide correct register values from -var-update. In addition, the new test for this functionality has been expanded to cover an analogous case using floating variable objects, and several tests have been updated to account for the fact that with the new initialization, -var-create and -var-list-children now provide a thread-id field where previously they didn't. Thanks --Don ------------ This patch fixes a problem with using the MI -var-update command to access the values of registers in frames other than the current frame. The patch includes a test that demonstrates the problem: * run so there are several frames on the stack * create a fixed varobj for $pc in each frame, #'s 1 and above * step one instruction, to modify the value of $pc * call -var-update for each of the previously created varobjs to verify that they are not reported as having changed. Without the patch, the -var-update command reported that $pc for all frames 1 and above had changed to the value of $pc in frame 0. When a varobj is created by -var-create, varobj->root->frame should be set to specified frame. Then for a subsequent -var-update, varobj.c:check_scope can use varobj->root->frame to select the right frame based on the type of varobj. The problem is that for register expressions varobj->root->frame is not set correctly. This frame tracking is done using the global 'innermost_block' which is set in the various parser files (for example c-exp.y). However, this is not set for register expressions. The fix is to change the initialization of innermost_block in varobj_create from NULL to the global block, as returned by block_global_block. Then varobj_create sets varobj->root->frame for register varobjs, and value_of_root_1 can check for the global block when determining whether the variable is in-scope and select the frame appropriately. A side-effect of this change is that for register varobjs and some global variable varobjs, the output of -var-create now includes the thread-id field. The rationale for this is as follow: if we ask for a particular expression in a particular frame, that implies a particular thread. Thus including the thread-id is correct behavior, and the test results have been updated accordingly. In addition there is a new test, gdb.mi/mi-frame-regs.exp, which performs the test described in the bullet list above as well as a similar test using a floating variable object to represent $pc. We attempted a couple alternative solutions: * a special case in varobj_update to select the frame was not ideal because it didn't use the existing mechanisms to select the frame. * setting innermost_block in write_dollar_variable had side-effects in the CLI that would have required special case code. Tested on x86_64 Linux native, no regressions. gdb/testsuite/ChangeLog: 2016-06-13 Don Breazeal <dbreazea@my.domain.org> * gdb.ada/mi_interface.exp: Add thread-id field to expected output of -var-create and -var-list-children. * gdb.ada/mi_var_array.exp: Add thread-id field to expected output of -var-list-children. * gdb.mi/mi-break.exp (test_error): Add thread-id field to expected output of -var-create. * gdb.mi/mi-frame-regs.exp: New test script. * gdb.mi/mi-var-cmd.exp: Add thread-id field to expected output of -var-create. * gdb.mi/mi-var-create-rtti.exp: Likewise. gdb/ChangeLog: 2016-06-13 Don Breazeal <donb@codesourcery.com> Andrew Burgess <andrew.burgess@embecosm.com> * varobj.c (varobj_create): Initialize innermost_block to the global block instead of NULL. (new_root_variable): Initialize the thread_id and next fields. (value_of_root_1): Set within_scope if the variable's valid_block field is the global block. --- gdb/testsuite/gdb.ada/mi_interface.exp | 4 +- gdb/testsuite/gdb.ada/mi_var_array.exp | 2 +- gdb/testsuite/gdb.mi/mi-break.exp | 2 +- gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 ++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/mi-var-cmd.exp | 4 +- gdb/testsuite/gdb.mi/mi-var-create-rtti.exp | 2 +- gdb/varobj.c | 8 +- 7 files changed, 196 insertions(+), 9 deletions(-)
Comments
On 6/13/2016 2:54 PM, Don Breazeal wrote: > This is an updated version of a patch to fix access to registers > for frames other than the current frame via var-update. The piece > of the patch affecting GDB proper has been completely re-written. > It changes the initialization of varobj->root->frame in varobj_create > so that existing mechanisms (with minimal changes) select the correct > frame and provide correct register values from -var-update. > > In addition, the new test for this functionality has been expanded > to cover an analogous case using floating variable objects, and > several tests have been updated to account for the fact that with > the new initialization, -var-create and -var-list-children now > provide a thread-id field where previously they didn't. > > Thanks > --Don > > ------------ > This patch fixes a problem with using the MI -var-update command > to access the values of registers in frames other than the current > frame. The patch includes a test that demonstrates the problem: > > * run so there are several frames on the stack > * create a fixed varobj for $pc in each frame, #'s 1 and above > * step one instruction, to modify the value of $pc > * call -var-update for each of the previously created varobjs > to verify that they are not reported as having changed. > > Without the patch, the -var-update command reported that $pc for all > frames 1 and above had changed to the value of $pc in frame 0. > > When a varobj is created by -var-create, varobj->root->frame should > be set to specified frame. Then for a subsequent -var-update, > varobj.c:check_scope can use varobj->root->frame to select the > right frame based on the type of varobj. > > The problem is that for register expressions varobj->root->frame is not > set correctly. This frame tracking is done using the global > 'innermost_block' which is set in the various parser files (for example > c-exp.y). However, this is not set for register expressions. > > The fix is to change the initialization of innermost_block in > varobj_create from NULL to the global block, as returned by > block_global_block. Then varobj_create sets varobj->root->frame > for register varobjs, and value_of_root_1 can check for the > global block when determining whether the variable is in-scope > and select the frame appropriately. > > A side-effect of this change is that for register varobjs and some > global variable varobjs, the output of -var-create now includes the > thread-id field. The rationale for this is as follow: if we ask for a > particular expression in a particular frame, that implies a particular > thread. Thus including the thread-id is correct behavior, and the > test results have been updated accordingly. > > In addition there is a new test, gdb.mi/mi-frame-regs.exp, which > performs the test described in the bullet list above as well as a > similar test using a floating variable object to represent $pc. > > We attempted a couple alternative solutions: > * a special case in varobj_update to select the frame was not ideal > because it didn't use the existing mechanisms to select the frame. > * setting innermost_block in write_dollar_variable had side-effects > in the CLI that would have required special case code. > > Tested on x86_64 Linux native, no regressions. > > gdb/testsuite/ChangeLog: > 2016-06-13 Don Breazeal <dbreazea@my.domain.org> > > * gdb.ada/mi_interface.exp: Add thread-id field to expected > output of -var-create and -var-list-children. > * gdb.ada/mi_var_array.exp: Add thread-id field to expected > output of -var-list-children. > * gdb.mi/mi-break.exp (test_error): Add thread-id field to > expected output of -var-create. > * gdb.mi/mi-frame-regs.exp: New test script. > * gdb.mi/mi-var-cmd.exp: Add thread-id field to expected > output of -var-create. > * gdb.mi/mi-var-create-rtti.exp: Likewise. > > gdb/ChangeLog: > 2016-06-13 Don Breazeal <donb@codesourcery.com> > Andrew Burgess <andrew.burgess@embecosm.com> > > * varobj.c (varobj_create): Initialize innermost_block to > the global block instead of NULL. > (new_root_variable): Initialize the thread_id and next > fields. > (value_of_root_1): Set within_scope if the variable's > valid_block field is the global block. > > --- > gdb/testsuite/gdb.ada/mi_interface.exp | 4 +- > gdb/testsuite/gdb.ada/mi_var_array.exp | 2 +- > gdb/testsuite/gdb.mi/mi-break.exp | 2 +- > gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 ++++++++++++++++++++++++++++ > gdb/testsuite/gdb.mi/mi-var-cmd.exp | 4 +- > gdb/testsuite/gdb.mi/mi-var-create-rtti.exp | 2 +- > gdb/varobj.c | 8 +- > 7 files changed, 196 insertions(+), 9 deletions(-) > > diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp > index 6000ec8..b948cd5 100644 > --- a/gdb/testsuite/gdb.ada/mi_interface.exp > +++ b/gdb/testsuite/gdb.ada/mi_interface.exp > @@ -44,9 +44,9 @@ mi_continue_to_line \ > "stop at start of main Ada procedure" > > mi_gdb_test "-var-create ggg1 * ggg1" \ > - "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \ > + "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \ > "Create ggg1 varobj" > > mi_gdb_test "-var-list-children 1 ggg1" \ > - "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \ > + "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \ > "list ggg1's children" > diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp > index c648e7e..c02d4c9 100644 > --- a/gdb/testsuite/gdb.ada/mi_var_array.exp > +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp > @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \ > "Create bt varobj" > > mi_gdb_test "-var-list-children vta" \ > - "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \ > + "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \ > "list vta's children" > diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp > index 85f328d..bdd43e0 100644 > --- a/gdb/testsuite/gdb.mi/mi-break.exp > +++ b/gdb/testsuite/gdb.mi/mi-break.exp > @@ -213,7 +213,7 @@ proc test_error {} { > # containing function call, the internal breakpoint created to handle > # function call would be reported, messing up MI output. > mi_gdb_test "-var-create V * return_1()" \ > - "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \ > + "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \ > "create varobj for function call" > > mi_gdb_test "-var-update *" \ > diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp > new file mode 100644 > index 0000000..45f81d6 > --- /dev/null > +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp > @@ -0,0 +1,183 @@ > +# Copyright 1999-2016 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/>. > + > +# Test essential Machine interface (MI) operations > +# > +# Verify that -var-update will provide the correct values for floating > +# and fixed varobjs that represent the pc register. > +# > + > +load_lib mi-support.exp > +set MIFLAGS "-i=mi" > + > +standard_testfile basics.c > + > +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ > + executable {debug}] != "" } then { > + untested mi-frame-regs.exp > + return -1 > +} > + > +# Return the address of the specified breakpoint. > + > +proc breakpoint_address {bpnum} { > + global hex > + global expect_out > + global mi_gdb_prompt > + > + send_gdb "info breakpoint $bpnum\n" > + gdb_expect { > + -re ".*($hex).*$mi_gdb_prompt$" { > + return $expect_out(1,string) > + } > + -re ".*$mi_gdb_prompt$" { > + return "" > + } > + timeout { > + return "" > + } > + } > +} > + > +# Test that a floating varobj representing $pc will provide the > +# correct value via -var-update as the program stops at > +# breakpoints in different functions. > + > +proc do_floating_varobj_test {} { > + global srcfile > + global hex > + global expect_out > + > + gdb_exit > + if {[mi_gdb_start]} then { > + continue > + } > + > + with_test_prefix "floating" { > + mi_run_to_main > + > + # Create a floating varobj for $pc. > + mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \ > + "\\^done,.*value=\"$hex.*" \ > + "create varobj for pc in frame 0" > + > + set nframes 4 > + for {set i 1} {$i < $nframes} {incr i} { > + > + # Run to a breakpoint in each callee function in succession. > + # Note that we can't use mi_runto because we need the > + # breakpoint to be persistent, so we can use its address. > + set bpnum [expr $i + 1] > + mi_create_breakpoint \ > + "basics.c:callee$i" \ > + "insert breakpoint at basics.c:callee$i" \ > + -number $bpnum -func callee$i -file ".*basics.c" > + > + mi_execute_to "exec-continue" "breakpoint-hit" \ > + "callee$i" ".*" ".*${srcfile}" ".*" \ > + { "" "disp=\"keep\"" } "breakpoint hit" > + > + # Get the value of $pc from the floating varobj. > + mi_gdb_test "-var-update 1 var1" \ > + "\\^done,.*value=\"($hex) .*" \ > + "-var-update for frame $i" > + set pcval $expect_out(3,string) > + > + # Get the address of the current breakpoint. > + set bpaddr [breakpoint_address $bpnum] > + if {$bpaddr == ""} then { > + unresolved "get address of breakpoint $bpnum" > + return > + } > + > + # Check that the addresses are the same. > + if {[expr $bpaddr != $pcval]} then { > + fail "\$pc does not equal address of breakpoint" > + } else { > + pass "\$pc equals address of breakpoint" > + } > + } > + } > +} > + > +# Create a varobj for the pc register in each of the frames other > +# than frame 0. > + > +proc var_create_regs {nframes} { > + global hex > + > + for {set i 1} {$i < $nframes} {incr i} { > + mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \ > + "\\^done,.*value=\"$hex.*" \ > + "create varobj for pc in frame $i" > + } > +} > + > +# Check that -var-update reports that the value of the pc register > +# for each of the frames 1 and above is reported as unchanged. > + > +proc var_update_regs {nframes} { > + > + for {set i 1} {$i < $nframes} {incr i} { > + mi_gdb_test "-var-update 1 var$i" \ > + "\\^done,(changelist=\\\[\\\])" \ > + "pc unchanged in -var-update for frame $i" > + } > +} > + > +# Test that fixed varobjs representing $pc in different stack frames > +# will provide the correct value via -var-update after the program > +# counter changes (without substantially changing the stack). > + > +proc do_fixed_varobj_test {} { > + global srcfile > + > + gdb_exit > + if {[mi_gdb_start]} then { > + continue > + } > + > + with_test_prefix "fixed" { > + mi_run_to_main > + > + # Run to the function 'callee3' so we have several frames. > + mi_create_breakpoint "basics.c:callee3" \ > + "insert breakpoint at basics.c:callee3" \ > + -number 2 -func callee3 -file ".*basics.c" > + > + mi_execute_to "exec-continue" "breakpoint-hit" \ > + "callee3" ".*" ".*${srcfile}" ".*" \ > + { "" "disp=\"keep\"" } "breakpoint hit" > + > + # At the breakpoint in callee3 there are 4 frames. Create a > + # varobj for the pc in each of frames 1 and above. > + set nframes 4 > + var_create_regs $nframes > + > + # Step one instruction to change the program counter. > + mi_execute_to "exec-next-instruction" "end-stepping-range" \ > + "callee3" ".*" ".*${srcfile}" ".*" "" \ > + "next instruction in callee3" > + > + # Check that -var-update reports that the values are unchanged. > + var_update_regs $nframes > + } > +} > + > +do_fixed_varobj_test > +do_floating_varobj_test > + > +mi_gdb_exit > +return 0 > diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp > index 558cd6c..68e3cf8 100644 > --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp > +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp > @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \ > "assign same value to func (update)" > > mi_gdb_test "-var-create array_ptr * array_ptr" \ > - "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \ > + "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \ > "create global variable array_ptr" > > mi_gdb_test "-var-assign array_ptr array2" \ > @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee" > # A varobj we fail to read during -var-update should be considered > # out of scope. > mi_gdb_test "-var-create null_ptr * **0" \ > - {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \ > + {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \ > "create null_ptr" > > # Allow this to succeed, if address zero is readable, although it > diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp > index 3bcb36c..a8cc76f 100644 > --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp > +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp > @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*" > # We use a explicit cast to (void *) as that is the > # type that caused the bug this testcase is testing for. > mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \ > - "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \ > + "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \ > "-var-create sp1 * \$sp" > gdb_exit > diff --git a/gdb/varobj.c b/gdb/varobj.c > index 6f56cba..1e3f192 100644 > --- a/gdb/varobj.c > +++ b/gdb/varobj.c > @@ -323,7 +323,7 @@ varobj_create (char *objname, > } > > p = expression; > - innermost_block = NULL; > + innermost_block = block_global_block (block); > /* Wrap the call to parse expression, so we can > return a sensible error. */ > TRY > @@ -2103,6 +2103,8 @@ new_root_variable (void) > var->root->floating = 0; > var->root->rootvar = NULL; > var->root->is_valid = 1; > + var->root->thread_id = 0; > + var->root->next = NULL; > > return var; > } > @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle) > back_to = make_cleanup_restore_current_thread (); > > /* Determine whether the variable is still around. */ > - if (var->root->valid_block == NULL || var->root->floating) > + if (var->root->valid_block == NULL > + || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL > + || var->root->floating) > within_scope = 1; > else if (var->root->thread_id == 0) > { >
ping On 6/20/2016 8:31 AM, Don Breazeal wrote: > On 6/13/2016 2:54 PM, Don Breazeal wrote: >> This is an updated version of a patch to fix access to registers >> for frames other than the current frame via var-update. The piece >> of the patch affecting GDB proper has been completely re-written. >> It changes the initialization of varobj->root->frame in varobj_create >> so that existing mechanisms (with minimal changes) select the correct >> frame and provide correct register values from -var-update. >> >> In addition, the new test for this functionality has been expanded >> to cover an analogous case using floating variable objects, and >> several tests have been updated to account for the fact that with >> the new initialization, -var-create and -var-list-children now >> provide a thread-id field where previously they didn't. >> >> Thanks >> --Don >> >> ------------ >> This patch fixes a problem with using the MI -var-update command >> to access the values of registers in frames other than the current >> frame. The patch includes a test that demonstrates the problem: >> >> * run so there are several frames on the stack >> * create a fixed varobj for $pc in each frame, #'s 1 and above >> * step one instruction, to modify the value of $pc >> * call -var-update for each of the previously created varobjs >> to verify that they are not reported as having changed. >> >> Without the patch, the -var-update command reported that $pc for all >> frames 1 and above had changed to the value of $pc in frame 0. >> >> When a varobj is created by -var-create, varobj->root->frame should >> be set to specified frame. Then for a subsequent -var-update, >> varobj.c:check_scope can use varobj->root->frame to select the >> right frame based on the type of varobj. >> >> The problem is that for register expressions varobj->root->frame is not >> set correctly. This frame tracking is done using the global >> 'innermost_block' which is set in the various parser files (for example >> c-exp.y). However, this is not set for register expressions. >> >> The fix is to change the initialization of innermost_block in >> varobj_create from NULL to the global block, as returned by >> block_global_block. Then varobj_create sets varobj->root->frame >> for register varobjs, and value_of_root_1 can check for the >> global block when determining whether the variable is in-scope >> and select the frame appropriately. >> >> A side-effect of this change is that for register varobjs and some >> global variable varobjs, the output of -var-create now includes the >> thread-id field. The rationale for this is as follow: if we ask for a >> particular expression in a particular frame, that implies a particular >> thread. Thus including the thread-id is correct behavior, and the >> test results have been updated accordingly. >> >> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which >> performs the test described in the bullet list above as well as a >> similar test using a floating variable object to represent $pc. >> >> We attempted a couple alternative solutions: >> * a special case in varobj_update to select the frame was not ideal >> because it didn't use the existing mechanisms to select the frame. >> * setting innermost_block in write_dollar_variable had side-effects >> in the CLI that would have required special case code. >> >> Tested on x86_64 Linux native, no regressions. >> >> gdb/testsuite/ChangeLog: >> 2016-06-13 Don Breazeal <dbreazea@my.domain.org> >> >> * gdb.ada/mi_interface.exp: Add thread-id field to expected >> output of -var-create and -var-list-children. >> * gdb.ada/mi_var_array.exp: Add thread-id field to expected >> output of -var-list-children. >> * gdb.mi/mi-break.exp (test_error): Add thread-id field to >> expected output of -var-create. >> * gdb.mi/mi-frame-regs.exp: New test script. >> * gdb.mi/mi-var-cmd.exp: Add thread-id field to expected >> output of -var-create. >> * gdb.mi/mi-var-create-rtti.exp: Likewise. >> >> gdb/ChangeLog: >> 2016-06-13 Don Breazeal <donb@codesourcery.com> >> Andrew Burgess <andrew.burgess@embecosm.com> >> >> * varobj.c (varobj_create): Initialize innermost_block to >> the global block instead of NULL. >> (new_root_variable): Initialize the thread_id and next >> fields. >> (value_of_root_1): Set within_scope if the variable's >> valid_block field is the global block. >> >> --- >> gdb/testsuite/gdb.ada/mi_interface.exp | 4 +- >> gdb/testsuite/gdb.ada/mi_var_array.exp | 2 +- >> gdb/testsuite/gdb.mi/mi-break.exp | 2 +- >> gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 ++++++++++++++++++++++++++++ >> gdb/testsuite/gdb.mi/mi-var-cmd.exp | 4 +- >> gdb/testsuite/gdb.mi/mi-var-create-rtti.exp | 2 +- >> gdb/varobj.c | 8 +- >> 7 files changed, 196 insertions(+), 9 deletions(-) >> >> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp >> index 6000ec8..b948cd5 100644 >> --- a/gdb/testsuite/gdb.ada/mi_interface.exp >> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp >> @@ -44,9 +44,9 @@ mi_continue_to_line \ >> "stop at start of main Ada procedure" >> >> mi_gdb_test "-var-create ggg1 * ggg1" \ >> - "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \ >> + "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \ >> "Create ggg1 varobj" >> >> mi_gdb_test "-var-list-children 1 ggg1" \ >> - "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \ >> + "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \ >> "list ggg1's children" >> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp >> index c648e7e..c02d4c9 100644 >> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp >> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp >> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \ >> "Create bt varobj" >> >> mi_gdb_test "-var-list-children vta" \ >> - "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \ >> + "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \ >> "list vta's children" >> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp >> index 85f328d..bdd43e0 100644 >> --- a/gdb/testsuite/gdb.mi/mi-break.exp >> +++ b/gdb/testsuite/gdb.mi/mi-break.exp >> @@ -213,7 +213,7 @@ proc test_error {} { >> # containing function call, the internal breakpoint created to handle >> # function call would be reported, messing up MI output. >> mi_gdb_test "-var-create V * return_1()" \ >> - "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \ >> + "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \ >> "create varobj for function call" >> >> mi_gdb_test "-var-update *" \ >> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >> new file mode 100644 >> index 0000000..45f81d6 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >> @@ -0,0 +1,183 @@ >> +# Copyright 1999-2016 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/>. >> + >> +# Test essential Machine interface (MI) operations >> +# >> +# Verify that -var-update will provide the correct values for floating >> +# and fixed varobjs that represent the pc register. >> +# >> + >> +load_lib mi-support.exp >> +set MIFLAGS "-i=mi" >> + >> +standard_testfile basics.c >> + >> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ >> + executable {debug}] != "" } then { >> + untested mi-frame-regs.exp >> + return -1 >> +} >> + >> +# Return the address of the specified breakpoint. >> + >> +proc breakpoint_address {bpnum} { >> + global hex >> + global expect_out >> + global mi_gdb_prompt >> + >> + send_gdb "info breakpoint $bpnum\n" >> + gdb_expect { >> + -re ".*($hex).*$mi_gdb_prompt$" { >> + return $expect_out(1,string) >> + } >> + -re ".*$mi_gdb_prompt$" { >> + return "" >> + } >> + timeout { >> + return "" >> + } >> + } >> +} >> + >> +# Test that a floating varobj representing $pc will provide the >> +# correct value via -var-update as the program stops at >> +# breakpoints in different functions. >> + >> +proc do_floating_varobj_test {} { >> + global srcfile >> + global hex >> + global expect_out >> + >> + gdb_exit >> + if {[mi_gdb_start]} then { >> + continue >> + } >> + >> + with_test_prefix "floating" { >> + mi_run_to_main >> + >> + # Create a floating varobj for $pc. >> + mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \ >> + "\\^done,.*value=\"$hex.*" \ >> + "create varobj for pc in frame 0" >> + >> + set nframes 4 >> + for {set i 1} {$i < $nframes} {incr i} { >> + >> + # Run to a breakpoint in each callee function in succession. >> + # Note that we can't use mi_runto because we need the >> + # breakpoint to be persistent, so we can use its address. >> + set bpnum [expr $i + 1] >> + mi_create_breakpoint \ >> + "basics.c:callee$i" \ >> + "insert breakpoint at basics.c:callee$i" \ >> + -number $bpnum -func callee$i -file ".*basics.c" >> + >> + mi_execute_to "exec-continue" "breakpoint-hit" \ >> + "callee$i" ".*" ".*${srcfile}" ".*" \ >> + { "" "disp=\"keep\"" } "breakpoint hit" >> + >> + # Get the value of $pc from the floating varobj. >> + mi_gdb_test "-var-update 1 var1" \ >> + "\\^done,.*value=\"($hex) .*" \ >> + "-var-update for frame $i" >> + set pcval $expect_out(3,string) >> + >> + # Get the address of the current breakpoint. >> + set bpaddr [breakpoint_address $bpnum] >> + if {$bpaddr == ""} then { >> + unresolved "get address of breakpoint $bpnum" >> + return >> + } >> + >> + # Check that the addresses are the same. >> + if {[expr $bpaddr != $pcval]} then { >> + fail "\$pc does not equal address of breakpoint" >> + } else { >> + pass "\$pc equals address of breakpoint" >> + } >> + } >> + } >> +} >> + >> +# Create a varobj for the pc register in each of the frames other >> +# than frame 0. >> + >> +proc var_create_regs {nframes} { >> + global hex >> + >> + for {set i 1} {$i < $nframes} {incr i} { >> + mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \ >> + "\\^done,.*value=\"$hex.*" \ >> + "create varobj for pc in frame $i" >> + } >> +} >> + >> +# Check that -var-update reports that the value of the pc register >> +# for each of the frames 1 and above is reported as unchanged. >> + >> +proc var_update_regs {nframes} { >> + >> + for {set i 1} {$i < $nframes} {incr i} { >> + mi_gdb_test "-var-update 1 var$i" \ >> + "\\^done,(changelist=\\\[\\\])" \ >> + "pc unchanged in -var-update for frame $i" >> + } >> +} >> + >> +# Test that fixed varobjs representing $pc in different stack frames >> +# will provide the correct value via -var-update after the program >> +# counter changes (without substantially changing the stack). >> + >> +proc do_fixed_varobj_test {} { >> + global srcfile >> + >> + gdb_exit >> + if {[mi_gdb_start]} then { >> + continue >> + } >> + >> + with_test_prefix "fixed" { >> + mi_run_to_main >> + >> + # Run to the function 'callee3' so we have several frames. >> + mi_create_breakpoint "basics.c:callee3" \ >> + "insert breakpoint at basics.c:callee3" \ >> + -number 2 -func callee3 -file ".*basics.c" >> + >> + mi_execute_to "exec-continue" "breakpoint-hit" \ >> + "callee3" ".*" ".*${srcfile}" ".*" \ >> + { "" "disp=\"keep\"" } "breakpoint hit" >> + >> + # At the breakpoint in callee3 there are 4 frames. Create a >> + # varobj for the pc in each of frames 1 and above. >> + set nframes 4 >> + var_create_regs $nframes >> + >> + # Step one instruction to change the program counter. >> + mi_execute_to "exec-next-instruction" "end-stepping-range" \ >> + "callee3" ".*" ".*${srcfile}" ".*" "" \ >> + "next instruction in callee3" >> + >> + # Check that -var-update reports that the values are unchanged. >> + var_update_regs $nframes >> + } >> +} >> + >> +do_fixed_varobj_test >> +do_floating_varobj_test >> + >> +mi_gdb_exit >> +return 0 >> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >> index 558cd6c..68e3cf8 100644 >> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp >> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \ >> "assign same value to func (update)" >> >> mi_gdb_test "-var-create array_ptr * array_ptr" \ >> - "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \ >> + "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \ >> "create global variable array_ptr" >> >> mi_gdb_test "-var-assign array_ptr array2" \ >> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee" >> # A varobj we fail to read during -var-update should be considered >> # out of scope. >> mi_gdb_test "-var-create null_ptr * **0" \ >> - {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \ >> + {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \ >> "create null_ptr" >> >> # Allow this to succeed, if address zero is readable, although it >> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >> index 3bcb36c..a8cc76f 100644 >> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*" >> # We use a explicit cast to (void *) as that is the >> # type that caused the bug this testcase is testing for. >> mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \ >> - "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \ >> + "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \ >> "-var-create sp1 * \$sp" >> gdb_exit >> diff --git a/gdb/varobj.c b/gdb/varobj.c >> index 6f56cba..1e3f192 100644 >> --- a/gdb/varobj.c >> +++ b/gdb/varobj.c >> @@ -323,7 +323,7 @@ varobj_create (char *objname, >> } >> >> p = expression; >> - innermost_block = NULL; >> + innermost_block = block_global_block (block); >> /* Wrap the call to parse expression, so we can >> return a sensible error. */ >> TRY >> @@ -2103,6 +2103,8 @@ new_root_variable (void) >> var->root->floating = 0; >> var->root->rootvar = NULL; >> var->root->is_valid = 1; >> + var->root->thread_id = 0; >> + var->root->next = NULL; >> >> return var; >> } >> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle) >> back_to = make_cleanup_restore_current_thread (); >> >> /* Determine whether the variable is still around. */ >> - if (var->root->valid_block == NULL || var->root->floating) >> + if (var->root->valid_block == NULL >> + || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL >> + || var->root->floating) >> within_scope = 1; >> else if (var->root->thread_id == 0) >> { >> >
ping On 7/11/2016 7:48 AM, Don Breazeal wrote: > > ping > > On 6/20/2016 8:31 AM, Don Breazeal wrote: >> On 6/13/2016 2:54 PM, Don Breazeal wrote: >>> This is an updated version of a patch to fix access to registers >>> for frames other than the current frame via var-update. The piece >>> of the patch affecting GDB proper has been completely re-written. >>> It changes the initialization of varobj->root->frame in varobj_create >>> so that existing mechanisms (with minimal changes) select the correct >>> frame and provide correct register values from -var-update. >>> >>> In addition, the new test for this functionality has been expanded >>> to cover an analogous case using floating variable objects, and >>> several tests have been updated to account for the fact that with >>> the new initialization, -var-create and -var-list-children now >>> provide a thread-id field where previously they didn't. >>> >>> Thanks >>> --Don >>> >>> ------------ >>> This patch fixes a problem with using the MI -var-update command >>> to access the values of registers in frames other than the current >>> frame. The patch includes a test that demonstrates the problem: >>> >>> * run so there are several frames on the stack >>> * create a fixed varobj for $pc in each frame, #'s 1 and above >>> * step one instruction, to modify the value of $pc >>> * call -var-update for each of the previously created varobjs >>> to verify that they are not reported as having changed. >>> >>> Without the patch, the -var-update command reported that $pc for all >>> frames 1 and above had changed to the value of $pc in frame 0. >>> >>> When a varobj is created by -var-create, varobj->root->frame should >>> be set to specified frame. Then for a subsequent -var-update, >>> varobj.c:check_scope can use varobj->root->frame to select the >>> right frame based on the type of varobj. >>> >>> The problem is that for register expressions varobj->root->frame is not >>> set correctly. This frame tracking is done using the global >>> 'innermost_block' which is set in the various parser files (for example >>> c-exp.y). However, this is not set for register expressions. >>> >>> The fix is to change the initialization of innermost_block in >>> varobj_create from NULL to the global block, as returned by >>> block_global_block. Then varobj_create sets varobj->root->frame >>> for register varobjs, and value_of_root_1 can check for the >>> global block when determining whether the variable is in-scope >>> and select the frame appropriately. >>> >>> A side-effect of this change is that for register varobjs and some >>> global variable varobjs, the output of -var-create now includes the >>> thread-id field. The rationale for this is as follow: if we ask for a >>> particular expression in a particular frame, that implies a particular >>> thread. Thus including the thread-id is correct behavior, and the >>> test results have been updated accordingly. >>> >>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which >>> performs the test described in the bullet list above as well as a >>> similar test using a floating variable object to represent $pc. >>> >>> We attempted a couple alternative solutions: >>> * a special case in varobj_update to select the frame was not ideal >>> because it didn't use the existing mechanisms to select the frame. >>> * setting innermost_block in write_dollar_variable had side-effects >>> in the CLI that would have required special case code. >>> >>> Tested on x86_64 Linux native, no regressions. >>> >>> gdb/testsuite/ChangeLog: >>> 2016-06-13 Don Breazeal <dbreazea@my.domain.org> >>> >>> * gdb.ada/mi_interface.exp: Add thread-id field to expected >>> output of -var-create and -var-list-children. >>> * gdb.ada/mi_var_array.exp: Add thread-id field to expected >>> output of -var-list-children. >>> * gdb.mi/mi-break.exp (test_error): Add thread-id field to >>> expected output of -var-create. >>> * gdb.mi/mi-frame-regs.exp: New test script. >>> * gdb.mi/mi-var-cmd.exp: Add thread-id field to expected >>> output of -var-create. >>> * gdb.mi/mi-var-create-rtti.exp: Likewise. >>> >>> gdb/ChangeLog: >>> 2016-06-13 Don Breazeal <donb@codesourcery.com> >>> Andrew Burgess <andrew.burgess@embecosm.com> >>> >>> * varobj.c (varobj_create): Initialize innermost_block to >>> the global block instead of NULL. >>> (new_root_variable): Initialize the thread_id and next >>> fields. >>> (value_of_root_1): Set within_scope if the variable's >>> valid_block field is the global block. >>> >>> --- >>> gdb/testsuite/gdb.ada/mi_interface.exp | 4 +- >>> gdb/testsuite/gdb.ada/mi_var_array.exp | 2 +- >>> gdb/testsuite/gdb.mi/mi-break.exp | 2 +- >>> gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 ++++++++++++++++++++++++++++ >>> gdb/testsuite/gdb.mi/mi-var-cmd.exp | 4 +- >>> gdb/testsuite/gdb.mi/mi-var-create-rtti.exp | 2 +- >>> gdb/varobj.c | 8 +- >>> 7 files changed, 196 insertions(+), 9 deletions(-) >>> >>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp >>> index 6000ec8..b948cd5 100644 >>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp >>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp >>> @@ -44,9 +44,9 @@ mi_continue_to_line \ >>> "stop at start of main Ada procedure" >>> >>> mi_gdb_test "-var-create ggg1 * ggg1" \ >>> - "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \ >>> + "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \ >>> "Create ggg1 varobj" >>> >>> mi_gdb_test "-var-list-children 1 ggg1" \ >>> - "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \ >>> + "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \ >>> "list ggg1's children" >>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp >>> index c648e7e..c02d4c9 100644 >>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp >>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp >>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \ >>> "Create bt varobj" >>> >>> mi_gdb_test "-var-list-children vta" \ >>> - "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \ >>> + "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \ >>> "list vta's children" >>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp >>> index 85f328d..bdd43e0 100644 >>> --- a/gdb/testsuite/gdb.mi/mi-break.exp >>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp >>> @@ -213,7 +213,7 @@ proc test_error {} { >>> # containing function call, the internal breakpoint created to handle >>> # function call would be reported, messing up MI output. >>> mi_gdb_test "-var-create V * return_1()" \ >>> - "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \ >>> + "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \ >>> "create varobj for function call" >>> >>> mi_gdb_test "-var-update *" \ >>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>> new file mode 100644 >>> index 0000000..45f81d6 >>> --- /dev/null >>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>> @@ -0,0 +1,183 @@ >>> +# Copyright 1999-2016 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/>. >>> + >>> +# Test essential Machine interface (MI) operations >>> +# >>> +# Verify that -var-update will provide the correct values for floating >>> +# and fixed varobjs that represent the pc register. >>> +# >>> + >>> +load_lib mi-support.exp >>> +set MIFLAGS "-i=mi" >>> + >>> +standard_testfile basics.c >>> + >>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ >>> + executable {debug}] != "" } then { >>> + untested mi-frame-regs.exp >>> + return -1 >>> +} >>> + >>> +# Return the address of the specified breakpoint. >>> + >>> +proc breakpoint_address {bpnum} { >>> + global hex >>> + global expect_out >>> + global mi_gdb_prompt >>> + >>> + send_gdb "info breakpoint $bpnum\n" >>> + gdb_expect { >>> + -re ".*($hex).*$mi_gdb_prompt$" { >>> + return $expect_out(1,string) >>> + } >>> + -re ".*$mi_gdb_prompt$" { >>> + return "" >>> + } >>> + timeout { >>> + return "" >>> + } >>> + } >>> +} >>> + >>> +# Test that a floating varobj representing $pc will provide the >>> +# correct value via -var-update as the program stops at >>> +# breakpoints in different functions. >>> + >>> +proc do_floating_varobj_test {} { >>> + global srcfile >>> + global hex >>> + global expect_out >>> + >>> + gdb_exit >>> + if {[mi_gdb_start]} then { >>> + continue >>> + } >>> + >>> + with_test_prefix "floating" { >>> + mi_run_to_main >>> + >>> + # Create a floating varobj for $pc. >>> + mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \ >>> + "\\^done,.*value=\"$hex.*" \ >>> + "create varobj for pc in frame 0" >>> + >>> + set nframes 4 >>> + for {set i 1} {$i < $nframes} {incr i} { >>> + >>> + # Run to a breakpoint in each callee function in succession. >>> + # Note that we can't use mi_runto because we need the >>> + # breakpoint to be persistent, so we can use its address. >>> + set bpnum [expr $i + 1] >>> + mi_create_breakpoint \ >>> + "basics.c:callee$i" \ >>> + "insert breakpoint at basics.c:callee$i" \ >>> + -number $bpnum -func callee$i -file ".*basics.c" >>> + >>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>> + "callee$i" ".*" ".*${srcfile}" ".*" \ >>> + { "" "disp=\"keep\"" } "breakpoint hit" >>> + >>> + # Get the value of $pc from the floating varobj. >>> + mi_gdb_test "-var-update 1 var1" \ >>> + "\\^done,.*value=\"($hex) .*" \ >>> + "-var-update for frame $i" >>> + set pcval $expect_out(3,string) >>> + >>> + # Get the address of the current breakpoint. >>> + set bpaddr [breakpoint_address $bpnum] >>> + if {$bpaddr == ""} then { >>> + unresolved "get address of breakpoint $bpnum" >>> + return >>> + } >>> + >>> + # Check that the addresses are the same. >>> + if {[expr $bpaddr != $pcval]} then { >>> + fail "\$pc does not equal address of breakpoint" >>> + } else { >>> + pass "\$pc equals address of breakpoint" >>> + } >>> + } >>> + } >>> +} >>> + >>> +# Create a varobj for the pc register in each of the frames other >>> +# than frame 0. >>> + >>> +proc var_create_regs {nframes} { >>> + global hex >>> + >>> + for {set i 1} {$i < $nframes} {incr i} { >>> + mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \ >>> + "\\^done,.*value=\"$hex.*" \ >>> + "create varobj for pc in frame $i" >>> + } >>> +} >>> + >>> +# Check that -var-update reports that the value of the pc register >>> +# for each of the frames 1 and above is reported as unchanged. >>> + >>> +proc var_update_regs {nframes} { >>> + >>> + for {set i 1} {$i < $nframes} {incr i} { >>> + mi_gdb_test "-var-update 1 var$i" \ >>> + "\\^done,(changelist=\\\[\\\])" \ >>> + "pc unchanged in -var-update for frame $i" >>> + } >>> +} >>> + >>> +# Test that fixed varobjs representing $pc in different stack frames >>> +# will provide the correct value via -var-update after the program >>> +# counter changes (without substantially changing the stack). >>> + >>> +proc do_fixed_varobj_test {} { >>> + global srcfile >>> + >>> + gdb_exit >>> + if {[mi_gdb_start]} then { >>> + continue >>> + } >>> + >>> + with_test_prefix "fixed" { >>> + mi_run_to_main >>> + >>> + # Run to the function 'callee3' so we have several frames. >>> + mi_create_breakpoint "basics.c:callee3" \ >>> + "insert breakpoint at basics.c:callee3" \ >>> + -number 2 -func callee3 -file ".*basics.c" >>> + >>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>> + "callee3" ".*" ".*${srcfile}" ".*" \ >>> + { "" "disp=\"keep\"" } "breakpoint hit" >>> + >>> + # At the breakpoint in callee3 there are 4 frames. Create a >>> + # varobj for the pc in each of frames 1 and above. >>> + set nframes 4 >>> + var_create_regs $nframes >>> + >>> + # Step one instruction to change the program counter. >>> + mi_execute_to "exec-next-instruction" "end-stepping-range" \ >>> + "callee3" ".*" ".*${srcfile}" ".*" "" \ >>> + "next instruction in callee3" >>> + >>> + # Check that -var-update reports that the values are unchanged. >>> + var_update_regs $nframes >>> + } >>> +} >>> + >>> +do_fixed_varobj_test >>> +do_floating_varobj_test >>> + >>> +mi_gdb_exit >>> +return 0 >>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>> index 558cd6c..68e3cf8 100644 >>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \ >>> "assign same value to func (update)" >>> >>> mi_gdb_test "-var-create array_ptr * array_ptr" \ >>> - "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \ >>> + "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \ >>> "create global variable array_ptr" >>> >>> mi_gdb_test "-var-assign array_ptr array2" \ >>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee" >>> # A varobj we fail to read during -var-update should be considered >>> # out of scope. >>> mi_gdb_test "-var-create null_ptr * **0" \ >>> - {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \ >>> + {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \ >>> "create null_ptr" >>> >>> # Allow this to succeed, if address zero is readable, although it >>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>> index 3bcb36c..a8cc76f 100644 >>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*" >>> # We use a explicit cast to (void *) as that is the >>> # type that caused the bug this testcase is testing for. >>> mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \ >>> - "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \ >>> + "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \ >>> "-var-create sp1 * \$sp" >>> gdb_exit >>> diff --git a/gdb/varobj.c b/gdb/varobj.c >>> index 6f56cba..1e3f192 100644 >>> --- a/gdb/varobj.c >>> +++ b/gdb/varobj.c >>> @@ -323,7 +323,7 @@ varobj_create (char *objname, >>> } >>> >>> p = expression; >>> - innermost_block = NULL; >>> + innermost_block = block_global_block (block); >>> /* Wrap the call to parse expression, so we can >>> return a sensible error. */ >>> TRY >>> @@ -2103,6 +2103,8 @@ new_root_variable (void) >>> var->root->floating = 0; >>> var->root->rootvar = NULL; >>> var->root->is_valid = 1; >>> + var->root->thread_id = 0; >>> + var->root->next = NULL; >>> >>> return var; >>> } >>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle) >>> back_to = make_cleanup_restore_current_thread (); >>> >>> /* Determine whether the variable is still around. */ >>> - if (var->root->valid_block == NULL || var->root->floating) >>> + if (var->root->valid_block == NULL >>> + || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL >>> + || var->root->floating) >>> within_scope = 1; >>> else if (var->root->thread_id == 0) >>> { >>> >> >
Hi, see below, On 7/20/2016 2:06 PM, Don Breazeal wrote: > ping > > On 7/11/2016 7:48 AM, Don Breazeal wrote: >> >> ping >> >> On 6/20/2016 8:31 AM, Don Breazeal wrote: >>> On 6/13/2016 2:54 PM, Don Breazeal wrote: >>>> This is an updated version of a patch to fix access to registers >>>> for frames other than the current frame via var-update. The piece >>>> of the patch affecting GDB proper has been completely re-written. >>>> It changes the initialization of varobj->root->frame in varobj_create >>>> so that existing mechanisms (with minimal changes) select the correct >>>> frame and provide correct register values from -var-update. >>>> >>>> In addition, the new test for this functionality has been expanded >>>> to cover an analogous case using floating variable objects, and >>>> several tests have been updated to account for the fact that with >>>> the new initialization, -var-create and -var-list-children now >>>> provide a thread-id field where previously they didn't. >>>> >>>> Thanks >>>> --Don >>>> >>>> ------------ >>>> This patch fixes a problem with using the MI -var-update command >>>> to access the values of registers in frames other than the current >>>> frame. The patch includes a test that demonstrates the problem: >>>> >>>> * run so there are several frames on the stack >>>> * create a fixed varobj for $pc in each frame, #'s 1 and above >>>> * step one instruction, to modify the value of $pc >>>> * call -var-update for each of the previously created varobjs >>>> to verify that they are not reported as having changed. >>>> >>>> Without the patch, the -var-update command reported that $pc for all >>>> frames 1 and above had changed to the value of $pc in frame 0. >>>> >>>> When a varobj is created by -var-create, varobj->root->frame should >>>> be set to specified frame. Then for a subsequent -var-update, >>>> varobj.c:check_scope can use varobj->root->frame to select the >>>> right frame based on the type of varobj. >>>> >>>> The problem is that for register expressions varobj->root->frame is not >>>> set correctly. This frame tracking is done using the global >>>> 'innermost_block' which is set in the various parser files (for example >>>> c-exp.y). However, this is not set for register expressions. >>>> >>>> The fix is to change the initialization of innermost_block in >>>> varobj_create from NULL to the global block, as returned by >>>> block_global_block. Then varobj_create sets varobj->root->frame >>>> for register varobjs, and value_of_root_1 can check for the >>>> global block when determining whether the variable is in-scope >>>> and select the frame appropriately. >>>> >>>> A side-effect of this change is that for register varobjs and some >>>> global variable varobjs, the output of -var-create now includes the >>>> thread-id field. The rationale for this is as follow: if we ask for a >>>> particular expression in a particular frame, that implies a particular >>>> thread. Thus including the thread-id is correct behavior, and the >>>> test results have been updated accordingly. >>>> >>>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which >>>> performs the test described in the bullet list above as well as a >>>> similar test using a floating variable object to represent $pc. >>>> >>>> We attempted a couple alternative solutions: >>>> * a special case in varobj_update to select the frame was not ideal >>>> because it didn't use the existing mechanisms to select the frame. >>>> * setting innermost_block in write_dollar_variable had side-effects >>>> in the CLI that would have required special case code. >>>> >>>> Tested on x86_64 Linux native, no regressions. >>>> >>>> gdb/testsuite/ChangeLog: >>>> 2016-06-13 Don Breazeal <dbreazea@my.domain.org> >>>> >>>> * gdb.ada/mi_interface.exp: Add thread-id field to expected >>>> output of -var-create and -var-list-children. >>>> * gdb.ada/mi_var_array.exp: Add thread-id field to expected >>>> output of -var-list-children. >>>> * gdb.mi/mi-break.exp (test_error): Add thread-id field to >>>> expected output of -var-create. >>>> * gdb.mi/mi-frame-regs.exp: New test script. >>>> * gdb.mi/mi-var-cmd.exp: Add thread-id field to expected >>>> output of -var-create. >>>> * gdb.mi/mi-var-create-rtti.exp: Likewise. >>>> >>>> gdb/ChangeLog: >>>> 2016-06-13 Don Breazeal <donb@codesourcery.com> >>>> Andrew Burgess <andrew.burgess@embecosm.com> >>>> >>>> * varobj.c (varobj_create): Initialize innermost_block to >>>> the global block instead of NULL. >>>> (new_root_variable): Initialize the thread_id and next >>>> fields. >>>> (value_of_root_1): Set within_scope if the variable's >>>> valid_block field is the global block. >>>> >>>> --- >>>> gdb/testsuite/gdb.ada/mi_interface.exp | 4 +- >>>> gdb/testsuite/gdb.ada/mi_var_array.exp | 2 +- >>>> gdb/testsuite/gdb.mi/mi-break.exp | 2 +- >>>> gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 ++++++++++++++++++++++++++++ >>>> gdb/testsuite/gdb.mi/mi-var-cmd.exp | 4 +- >>>> gdb/testsuite/gdb.mi/mi-var-create-rtti.exp | 2 +- >>>> gdb/varobj.c | 8 +- >>>> 7 files changed, 196 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp >>>> index 6000ec8..b948cd5 100644 >>>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp >>>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp >>>> @@ -44,9 +44,9 @@ mi_continue_to_line \ >>>> "stop at start of main Ada procedure" >>>> >>>> mi_gdb_test "-var-create ggg1 * ggg1" \ >>>> - "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \ >>>> + "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \ >>>> "Create ggg1 varobj" >>>> >>>> mi_gdb_test "-var-list-children 1 ggg1" \ >>>> - "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \ >>>> + "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \ >>>> "list ggg1's children" >>>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp >>>> index c648e7e..c02d4c9 100644 >>>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp >>>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp >>>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \ >>>> "Create bt varobj" >>>> >>>> mi_gdb_test "-var-list-children vta" \ >>>> - "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \ >>>> + "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \ >>>> "list vta's children" >>>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp >>>> index 85f328d..bdd43e0 100644 >>>> --- a/gdb/testsuite/gdb.mi/mi-break.exp >>>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp >>>> @@ -213,7 +213,7 @@ proc test_error {} { >>>> # containing function call, the internal breakpoint created to handle >>>> # function call would be reported, messing up MI output. >>>> mi_gdb_test "-var-create V * return_1()" \ >>>> - "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \ >>>> + "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \ >>>> "create varobj for function call" >>>> >>>> mi_gdb_test "-var-update *" \ >>>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>>> new file mode 100644 >>>> index 0000000..45f81d6 >>>> --- /dev/null >>>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>>> @@ -0,0 +1,183 @@ >>>> +# Copyright 1999-2016 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/>. >>>> + >>>> +# Test essential Machine interface (MI) operations >>>> +# >>>> +# Verify that -var-update will provide the correct values for floating >>>> +# and fixed varobjs that represent the pc register. >>>> +# >>>> + >>>> +load_lib mi-support.exp >>>> +set MIFLAGS "-i=mi" >>>> + >>>> +standard_testfile basics.c >>>> + >>>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ >>>> + executable {debug}] != "" } then { >>>> + untested mi-frame-regs.exp >>>> + return -1 >>>> +} >>>> + >>>> +# Return the address of the specified breakpoint. >>>> + >>>> +proc breakpoint_address {bpnum} { >>>> + global hex >>>> + global expect_out >>>> + global mi_gdb_prompt >>>> + >>>> + send_gdb "info breakpoint $bpnum\n" >>>> + gdb_expect { >>>> + -re ".*($hex).*$mi_gdb_prompt$" { >>>> + return $expect_out(1,string) >>>> + } >>>> + -re ".*$mi_gdb_prompt$" { >>>> + return "" >>>> + } >>>> + timeout { >>>> + return "" >>>> + } >>>> + } >>>> +} >>>> + >>>> +# Test that a floating varobj representing $pc will provide the >>>> +# correct value via -var-update as the program stops at >>>> +# breakpoints in different functions. >>>> + >>>> +proc do_floating_varobj_test {} { >>>> + global srcfile >>>> + global hex >>>> + global expect_out >>>> + >>>> + gdb_exit >>>> + if {[mi_gdb_start]} then { >>>> + continue >>>> + } >>>> + >>>> + with_test_prefix "floating" { >>>> + mi_run_to_main >>>> + >>>> + # Create a floating varobj for $pc. >>>> + mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \ >>>> + "\\^done,.*value=\"$hex.*" \ >>>> + "create varobj for pc in frame 0" >>>> + >>>> + set nframes 4 >>>> + for {set i 1} {$i < $nframes} {incr i} { >>>> + >>>> + # Run to a breakpoint in each callee function in succession. >>>> + # Note that we can't use mi_runto because we need the >>>> + # breakpoint to be persistent, so we can use its address. >>>> + set bpnum [expr $i + 1] >>>> + mi_create_breakpoint \ >>>> + "basics.c:callee$i" \ >>>> + "insert breakpoint at basics.c:callee$i" \ >>>> + -number $bpnum -func callee$i -file ".*basics.c" >>>> + >>>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>>> + "callee$i" ".*" ".*${srcfile}" ".*" \ >>>> + { "" "disp=\"keep\"" } "breakpoint hit" >>>> + >>>> + # Get the value of $pc from the floating varobj. >>>> + mi_gdb_test "-var-update 1 var1" \ >>>> + "\\^done,.*value=\"($hex) .*" \ >>>> + "-var-update for frame $i" >>>> + set pcval $expect_out(3,string) >>>> + >>>> + # Get the address of the current breakpoint. >>>> + set bpaddr [breakpoint_address $bpnum] >>>> + if {$bpaddr == ""} then { >>>> + unresolved "get address of breakpoint $bpnum" >>>> + return >>>> + } >>>> + >>>> + # Check that the addresses are the same. >>>> + if {[expr $bpaddr != $pcval]} then { >>>> + fail "\$pc does not equal address of breakpoint" >>>> + } else { >>>> + pass "\$pc equals address of breakpoint" >>>> + } >>>> + } >>>> + } >>>> +} >>>> + >>>> +# Create a varobj for the pc register in each of the frames other >>>> +# than frame 0. >>>> + >>>> +proc var_create_regs {nframes} { >>>> + global hex >>>> + >>>> + for {set i 1} {$i < $nframes} {incr i} { >>>> + mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \ >>>> + "\\^done,.*value=\"$hex.*" \ >>>> + "create varobj for pc in frame $i" >>>> + } >>>> +} >>>> + >>>> +# Check that -var-update reports that the value of the pc register >>>> +# for each of the frames 1 and above is reported as unchanged. >>>> + >>>> +proc var_update_regs {nframes} { >>>> + >>>> + for {set i 1} {$i < $nframes} {incr i} { >>>> + mi_gdb_test "-var-update 1 var$i" \ >>>> + "\\^done,(changelist=\\\[\\\])" \ >>>> + "pc unchanged in -var-update for frame $i" >>>> + } >>>> +} >>>> + >>>> +# Test that fixed varobjs representing $pc in different stack frames >>>> +# will provide the correct value via -var-update after the program >>>> +# counter changes (without substantially changing the stack). >>>> + >>>> +proc do_fixed_varobj_test {} { >>>> + global srcfile >>>> + >>>> + gdb_exit >>>> + if {[mi_gdb_start]} then { >>>> + continue >>>> + } >>>> + >>>> + with_test_prefix "fixed" { >>>> + mi_run_to_main >>>> + >>>> + # Run to the function 'callee3' so we have several frames. >>>> + mi_create_breakpoint "basics.c:callee3" \ >>>> + "insert breakpoint at basics.c:callee3" \ >>>> + -number 2 -func callee3 -file ".*basics.c" >>>> + >>>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>>> + "callee3" ".*" ".*${srcfile}" ".*" \ >>>> + { "" "disp=\"keep\"" } "breakpoint hit" >>>> + >>>> + # At the breakpoint in callee3 there are 4 frames. Create a >>>> + # varobj for the pc in each of frames 1 and above. >>>> + set nframes 4 >>>> + var_create_regs $nframes >>>> + >>>> + # Step one instruction to change the program counter. >>>> + mi_execute_to "exec-next-instruction" "end-stepping-range" \ >>>> + "callee3" ".*" ".*${srcfile}" ".*" "" \ >>>> + "next instruction in callee3" >>>> + >>>> + # Check that -var-update reports that the values are unchanged. >>>> + var_update_regs $nframes >>>> + } >>>> +} >>>> + >>>> +do_fixed_varobj_test >>>> +do_floating_varobj_test >>>> + >>>> +mi_gdb_exit >>>> +return 0 >>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>> index 558cd6c..68e3cf8 100644 >>>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \ >>>> "assign same value to func (update)" >>>> >>>> mi_gdb_test "-var-create array_ptr * array_ptr" \ >>>> - "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \ >>>> + "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \ >>>> "create global variable array_ptr" >>>> >>>> mi_gdb_test "-var-assign array_ptr array2" \ >>>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee" >>>> # A varobj we fail to read during -var-update should be considered >>>> # out of scope. >>>> mi_gdb_test "-var-create null_ptr * **0" \ >>>> - {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \ >>>> + {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \ >>>> "create null_ptr" >>>> >>>> # Allow this to succeed, if address zero is readable, although it >>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>> index 3bcb36c..a8cc76f 100644 >>>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*" >>>> # We use a explicit cast to (void *) as that is the >>>> # type that caused the bug this testcase is testing for. >>>> mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \ >>>> - "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \ >>>> + "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \ >>>> "-var-create sp1 * \$sp" >>>> gdb_exit >>>> diff --git a/gdb/varobj.c b/gdb/varobj.c >>>> index 6f56cba..1e3f192 100644 >>>> --- a/gdb/varobj.c >>>> +++ b/gdb/varobj.c >>>> @@ -323,7 +323,7 @@ varobj_create (char *objname, >>>> } >>>> >>>> p = expression; >>>> - innermost_block = NULL; >>>> + innermost_block = block_global_block (block); >>>> /* Wrap the call to parse expression, so we can >>>> return a sensible error. */ >>>> TRY >>>> @@ -2103,6 +2103,8 @@ new_root_variable (void) >>>> var->root->floating = 0; >>>> var->root->rootvar = NULL; >>>> var->root->is_valid = 1; >>>> + var->root->thread_id = 0; >>>> + var->root->next = NULL; >>>> >>>> return var; >>>> } >>>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle) >>>> back_to = make_cleanup_restore_current_thread (); >>>> >>>> /* Determine whether the variable is still around. */ >>>> - if (var->root->valid_block == NULL || var->root->floating) >>>> + if (var->root->valid_block == NULL >>>> + || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL >>>> + || var->root->floating) >>>> within_scope = 1; >>>> else if (var->root->thread_id == 0) >>>> { >>>> >>> >> > I've created PR mi/20395 for this issue and marked it with the Target Milestone 7.12. Since it's an issue of returning incorrect data I'm proposing that it's a blocker. thanks --Don
ping On 7/21/2016 9:59 AM, Don Breazeal wrote: > Hi, see below, > > On 7/20/2016 2:06 PM, Don Breazeal wrote: >> ping >> >> On 7/11/2016 7:48 AM, Don Breazeal wrote: >>> >>> ping >>> >>> On 6/20/2016 8:31 AM, Don Breazeal wrote: >>>> On 6/13/2016 2:54 PM, Don Breazeal wrote: >>>>> This is an updated version of a patch to fix access to registers >>>>> for frames other than the current frame via var-update. The piece >>>>> of the patch affecting GDB proper has been completely re-written. >>>>> It changes the initialization of varobj->root->frame in varobj_create >>>>> so that existing mechanisms (with minimal changes) select the correct >>>>> frame and provide correct register values from -var-update. >>>>> >>>>> In addition, the new test for this functionality has been expanded >>>>> to cover an analogous case using floating variable objects, and >>>>> several tests have been updated to account for the fact that with >>>>> the new initialization, -var-create and -var-list-children now >>>>> provide a thread-id field where previously they didn't. >>>>> >>>>> Thanks >>>>> --Don >>>>> >>>>> ------------ >>>>> This patch fixes a problem with using the MI -var-update command >>>>> to access the values of registers in frames other than the current >>>>> frame. The patch includes a test that demonstrates the problem: >>>>> >>>>> * run so there are several frames on the stack >>>>> * create a fixed varobj for $pc in each frame, #'s 1 and above >>>>> * step one instruction, to modify the value of $pc >>>>> * call -var-update for each of the previously created varobjs >>>>> to verify that they are not reported as having changed. >>>>> >>>>> Without the patch, the -var-update command reported that $pc for all >>>>> frames 1 and above had changed to the value of $pc in frame 0. >>>>> >>>>> When a varobj is created by -var-create, varobj->root->frame should >>>>> be set to specified frame. Then for a subsequent -var-update, >>>>> varobj.c:check_scope can use varobj->root->frame to select the >>>>> right frame based on the type of varobj. >>>>> >>>>> The problem is that for register expressions varobj->root->frame is not >>>>> set correctly. This frame tracking is done using the global >>>>> 'innermost_block' which is set in the various parser files (for example >>>>> c-exp.y). However, this is not set for register expressions. >>>>> >>>>> The fix is to change the initialization of innermost_block in >>>>> varobj_create from NULL to the global block, as returned by >>>>> block_global_block. Then varobj_create sets varobj->root->frame >>>>> for register varobjs, and value_of_root_1 can check for the >>>>> global block when determining whether the variable is in-scope >>>>> and select the frame appropriately. >>>>> >>>>> A side-effect of this change is that for register varobjs and some >>>>> global variable varobjs, the output of -var-create now includes the >>>>> thread-id field. The rationale for this is as follow: if we ask for a >>>>> particular expression in a particular frame, that implies a particular >>>>> thread. Thus including the thread-id is correct behavior, and the >>>>> test results have been updated accordingly. >>>>> >>>>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which >>>>> performs the test described in the bullet list above as well as a >>>>> similar test using a floating variable object to represent $pc. >>>>> >>>>> We attempted a couple alternative solutions: >>>>> * a special case in varobj_update to select the frame was not ideal >>>>> because it didn't use the existing mechanisms to select the frame. >>>>> * setting innermost_block in write_dollar_variable had side-effects >>>>> in the CLI that would have required special case code. >>>>> >>>>> Tested on x86_64 Linux native, no regressions. >>>>> >>>>> gdb/testsuite/ChangeLog: >>>>> 2016-06-13 Don Breazeal <dbreazea@my.domain.org> >>>>> >>>>> * gdb.ada/mi_interface.exp: Add thread-id field to expected >>>>> output of -var-create and -var-list-children. >>>>> * gdb.ada/mi_var_array.exp: Add thread-id field to expected >>>>> output of -var-list-children. >>>>> * gdb.mi/mi-break.exp (test_error): Add thread-id field to >>>>> expected output of -var-create. >>>>> * gdb.mi/mi-frame-regs.exp: New test script. >>>>> * gdb.mi/mi-var-cmd.exp: Add thread-id field to expected >>>>> output of -var-create. >>>>> * gdb.mi/mi-var-create-rtti.exp: Likewise. >>>>> >>>>> gdb/ChangeLog: >>>>> 2016-06-13 Don Breazeal <donb@codesourcery.com> >>>>> Andrew Burgess <andrew.burgess@embecosm.com> >>>>> >>>>> * varobj.c (varobj_create): Initialize innermost_block to >>>>> the global block instead of NULL. >>>>> (new_root_variable): Initialize the thread_id and next >>>>> fields. >>>>> (value_of_root_1): Set within_scope if the variable's >>>>> valid_block field is the global block. >>>>> >>>>> --- >>>>> gdb/testsuite/gdb.ada/mi_interface.exp | 4 +- >>>>> gdb/testsuite/gdb.ada/mi_var_array.exp | 2 +- >>>>> gdb/testsuite/gdb.mi/mi-break.exp | 2 +- >>>>> gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 ++++++++++++++++++++++++++++ >>>>> gdb/testsuite/gdb.mi/mi-var-cmd.exp | 4 +- >>>>> gdb/testsuite/gdb.mi/mi-var-create-rtti.exp | 2 +- >>>>> gdb/varobj.c | 8 +- >>>>> 7 files changed, 196 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp >>>>> index 6000ec8..b948cd5 100644 >>>>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp >>>>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp >>>>> @@ -44,9 +44,9 @@ mi_continue_to_line \ >>>>> "stop at start of main Ada procedure" >>>>> >>>>> mi_gdb_test "-var-create ggg1 * ggg1" \ >>>>> - "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \ >>>>> + "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \ >>>>> "Create ggg1 varobj" >>>>> >>>>> mi_gdb_test "-var-list-children 1 ggg1" \ >>>>> - "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \ >>>>> + "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \ >>>>> "list ggg1's children" >>>>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp >>>>> index c648e7e..c02d4c9 100644 >>>>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp >>>>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp >>>>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \ >>>>> "Create bt varobj" >>>>> >>>>> mi_gdb_test "-var-list-children vta" \ >>>>> - "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \ >>>>> + "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \ >>>>> "list vta's children" >>>>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp >>>>> index 85f328d..bdd43e0 100644 >>>>> --- a/gdb/testsuite/gdb.mi/mi-break.exp >>>>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp >>>>> @@ -213,7 +213,7 @@ proc test_error {} { >>>>> # containing function call, the internal breakpoint created to handle >>>>> # function call would be reported, messing up MI output. >>>>> mi_gdb_test "-var-create V * return_1()" \ >>>>> - "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \ >>>>> + "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \ >>>>> "create varobj for function call" >>>>> >>>>> mi_gdb_test "-var-update *" \ >>>>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>>>> new file mode 100644 >>>>> index 0000000..45f81d6 >>>>> --- /dev/null >>>>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>>>> @@ -0,0 +1,183 @@ >>>>> +# Copyright 1999-2016 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/>. >>>>> + >>>>> +# Test essential Machine interface (MI) operations >>>>> +# >>>>> +# Verify that -var-update will provide the correct values for floating >>>>> +# and fixed varobjs that represent the pc register. >>>>> +# >>>>> + >>>>> +load_lib mi-support.exp >>>>> +set MIFLAGS "-i=mi" >>>>> + >>>>> +standard_testfile basics.c >>>>> + >>>>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ >>>>> + executable {debug}] != "" } then { >>>>> + untested mi-frame-regs.exp >>>>> + return -1 >>>>> +} >>>>> + >>>>> +# Return the address of the specified breakpoint. >>>>> + >>>>> +proc breakpoint_address {bpnum} { >>>>> + global hex >>>>> + global expect_out >>>>> + global mi_gdb_prompt >>>>> + >>>>> + send_gdb "info breakpoint $bpnum\n" >>>>> + gdb_expect { >>>>> + -re ".*($hex).*$mi_gdb_prompt$" { >>>>> + return $expect_out(1,string) >>>>> + } >>>>> + -re ".*$mi_gdb_prompt$" { >>>>> + return "" >>>>> + } >>>>> + timeout { >>>>> + return "" >>>>> + } >>>>> + } >>>>> +} >>>>> + >>>>> +# Test that a floating varobj representing $pc will provide the >>>>> +# correct value via -var-update as the program stops at >>>>> +# breakpoints in different functions. >>>>> + >>>>> +proc do_floating_varobj_test {} { >>>>> + global srcfile >>>>> + global hex >>>>> + global expect_out >>>>> + >>>>> + gdb_exit >>>>> + if {[mi_gdb_start]} then { >>>>> + continue >>>>> + } >>>>> + >>>>> + with_test_prefix "floating" { >>>>> + mi_run_to_main >>>>> + >>>>> + # Create a floating varobj for $pc. >>>>> + mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \ >>>>> + "\\^done,.*value=\"$hex.*" \ >>>>> + "create varobj for pc in frame 0" >>>>> + >>>>> + set nframes 4 >>>>> + for {set i 1} {$i < $nframes} {incr i} { >>>>> + >>>>> + # Run to a breakpoint in each callee function in succession. >>>>> + # Note that we can't use mi_runto because we need the >>>>> + # breakpoint to be persistent, so we can use its address. >>>>> + set bpnum [expr $i + 1] >>>>> + mi_create_breakpoint \ >>>>> + "basics.c:callee$i" \ >>>>> + "insert breakpoint at basics.c:callee$i" \ >>>>> + -number $bpnum -func callee$i -file ".*basics.c" >>>>> + >>>>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>>>> + "callee$i" ".*" ".*${srcfile}" ".*" \ >>>>> + { "" "disp=\"keep\"" } "breakpoint hit" >>>>> + >>>>> + # Get the value of $pc from the floating varobj. >>>>> + mi_gdb_test "-var-update 1 var1" \ >>>>> + "\\^done,.*value=\"($hex) .*" \ >>>>> + "-var-update for frame $i" >>>>> + set pcval $expect_out(3,string) >>>>> + >>>>> + # Get the address of the current breakpoint. >>>>> + set bpaddr [breakpoint_address $bpnum] >>>>> + if {$bpaddr == ""} then { >>>>> + unresolved "get address of breakpoint $bpnum" >>>>> + return >>>>> + } >>>>> + >>>>> + # Check that the addresses are the same. >>>>> + if {[expr $bpaddr != $pcval]} then { >>>>> + fail "\$pc does not equal address of breakpoint" >>>>> + } else { >>>>> + pass "\$pc equals address of breakpoint" >>>>> + } >>>>> + } >>>>> + } >>>>> +} >>>>> + >>>>> +# Create a varobj for the pc register in each of the frames other >>>>> +# than frame 0. >>>>> + >>>>> +proc var_create_regs {nframes} { >>>>> + global hex >>>>> + >>>>> + for {set i 1} {$i < $nframes} {incr i} { >>>>> + mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \ >>>>> + "\\^done,.*value=\"$hex.*" \ >>>>> + "create varobj for pc in frame $i" >>>>> + } >>>>> +} >>>>> + >>>>> +# Check that -var-update reports that the value of the pc register >>>>> +# for each of the frames 1 and above is reported as unchanged. >>>>> + >>>>> +proc var_update_regs {nframes} { >>>>> + >>>>> + for {set i 1} {$i < $nframes} {incr i} { >>>>> + mi_gdb_test "-var-update 1 var$i" \ >>>>> + "\\^done,(changelist=\\\[\\\])" \ >>>>> + "pc unchanged in -var-update for frame $i" >>>>> + } >>>>> +} >>>>> + >>>>> +# Test that fixed varobjs representing $pc in different stack frames >>>>> +# will provide the correct value via -var-update after the program >>>>> +# counter changes (without substantially changing the stack). >>>>> + >>>>> +proc do_fixed_varobj_test {} { >>>>> + global srcfile >>>>> + >>>>> + gdb_exit >>>>> + if {[mi_gdb_start]} then { >>>>> + continue >>>>> + } >>>>> + >>>>> + with_test_prefix "fixed" { >>>>> + mi_run_to_main >>>>> + >>>>> + # Run to the function 'callee3' so we have several frames. >>>>> + mi_create_breakpoint "basics.c:callee3" \ >>>>> + "insert breakpoint at basics.c:callee3" \ >>>>> + -number 2 -func callee3 -file ".*basics.c" >>>>> + >>>>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>>>> + "callee3" ".*" ".*${srcfile}" ".*" \ >>>>> + { "" "disp=\"keep\"" } "breakpoint hit" >>>>> + >>>>> + # At the breakpoint in callee3 there are 4 frames. Create a >>>>> + # varobj for the pc in each of frames 1 and above. >>>>> + set nframes 4 >>>>> + var_create_regs $nframes >>>>> + >>>>> + # Step one instruction to change the program counter. >>>>> + mi_execute_to "exec-next-instruction" "end-stepping-range" \ >>>>> + "callee3" ".*" ".*${srcfile}" ".*" "" \ >>>>> + "next instruction in callee3" >>>>> + >>>>> + # Check that -var-update reports that the values are unchanged. >>>>> + var_update_regs $nframes >>>>> + } >>>>> +} >>>>> + >>>>> +do_fixed_varobj_test >>>>> +do_floating_varobj_test >>>>> + >>>>> +mi_gdb_exit >>>>> +return 0 >>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>>> index 558cd6c..68e3cf8 100644 >>>>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \ >>>>> "assign same value to func (update)" >>>>> >>>>> mi_gdb_test "-var-create array_ptr * array_ptr" \ >>>>> - "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \ >>>>> + "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \ >>>>> "create global variable array_ptr" >>>>> >>>>> mi_gdb_test "-var-assign array_ptr array2" \ >>>>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee" >>>>> # A varobj we fail to read during -var-update should be considered >>>>> # out of scope. >>>>> mi_gdb_test "-var-create null_ptr * **0" \ >>>>> - {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \ >>>>> + {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \ >>>>> "create null_ptr" >>>>> >>>>> # Allow this to succeed, if address zero is readable, although it >>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>>> index 3bcb36c..a8cc76f 100644 >>>>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*" >>>>> # We use a explicit cast to (void *) as that is the >>>>> # type that caused the bug this testcase is testing for. >>>>> mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \ >>>>> - "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \ >>>>> + "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \ >>>>> "-var-create sp1 * \$sp" >>>>> gdb_exit >>>>> diff --git a/gdb/varobj.c b/gdb/varobj.c >>>>> index 6f56cba..1e3f192 100644 >>>>> --- a/gdb/varobj.c >>>>> +++ b/gdb/varobj.c >>>>> @@ -323,7 +323,7 @@ varobj_create (char *objname, >>>>> } >>>>> >>>>> p = expression; >>>>> - innermost_block = NULL; >>>>> + innermost_block = block_global_block (block); >>>>> /* Wrap the call to parse expression, so we can >>>>> return a sensible error. */ >>>>> TRY >>>>> @@ -2103,6 +2103,8 @@ new_root_variable (void) >>>>> var->root->floating = 0; >>>>> var->root->rootvar = NULL; >>>>> var->root->is_valid = 1; >>>>> + var->root->thread_id = 0; >>>>> + var->root->next = NULL; >>>>> >>>>> return var; >>>>> } >>>>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle) >>>>> back_to = make_cleanup_restore_current_thread (); >>>>> >>>>> /* Determine whether the variable is still around. */ >>>>> - if (var->root->valid_block == NULL || var->root->floating) >>>>> + if (var->root->valid_block == NULL >>>>> + || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL >>>>> + || var->root->floating) >>>>> within_scope = 1; >>>>> else if (var->root->thread_id == 0) >>>>> { >>>>> >>>> >>> >> > I've created PR mi/20395 for this issue and marked it with the Target > Milestone 7.12. Since it's an issue of returning incorrect data I'm > proposing that it's a blocker. > thanks > --Don >
Hi. This patch has been in the queue for quite a while. ping. Thanks! --Don >>>> >>>> On 6/20/2016 8:31 AM, Don Breazeal wrote: >>>>> On 6/13/2016 2:54 PM, Don Breazeal wrote: >>>>>> This is an updated version of a patch to fix access to registers >>>>>> for frames other than the current frame via var-update. The piece >>>>>> of the patch affecting GDB proper has been completely re-written. >>>>>> It changes the initialization of varobj->root->frame in varobj_create >>>>>> so that existing mechanisms (with minimal changes) select the correct >>>>>> frame and provide correct register values from -var-update. >>>>>> >>>>>> In addition, the new test for this functionality has been expanded >>>>>> to cover an analogous case using floating variable objects, and >>>>>> several tests have been updated to account for the fact that with >>>>>> the new initialization, -var-create and -var-list-children now >>>>>> provide a thread-id field where previously they didn't. >>>>>> >>>>>> Thanks >>>>>> --Don >>>>>> >>>>>> ------------ >>>>>> This patch fixes a problem with using the MI -var-update command >>>>>> to access the values of registers in frames other than the current >>>>>> frame. The patch includes a test that demonstrates the problem: >>>>>> >>>>>> * run so there are several frames on the stack >>>>>> * create a fixed varobj for $pc in each frame, #'s 1 and above >>>>>> * step one instruction, to modify the value of $pc >>>>>> * call -var-update for each of the previously created varobjs >>>>>> to verify that they are not reported as having changed. >>>>>> >>>>>> Without the patch, the -var-update command reported that $pc for all >>>>>> frames 1 and above had changed to the value of $pc in frame 0. >>>>>> >>>>>> When a varobj is created by -var-create, varobj->root->frame should >>>>>> be set to specified frame. Then for a subsequent -var-update, >>>>>> varobj.c:check_scope can use varobj->root->frame to select the >>>>>> right frame based on the type of varobj. >>>>>> >>>>>> The problem is that for register expressions varobj->root->frame is not >>>>>> set correctly. This frame tracking is done using the global >>>>>> 'innermost_block' which is set in the various parser files (for example >>>>>> c-exp.y). However, this is not set for register expressions. >>>>>> >>>>>> The fix is to change the initialization of innermost_block in >>>>>> varobj_create from NULL to the global block, as returned by >>>>>> block_global_block. Then varobj_create sets varobj->root->frame >>>>>> for register varobjs, and value_of_root_1 can check for the >>>>>> global block when determining whether the variable is in-scope >>>>>> and select the frame appropriately. >>>>>> >>>>>> A side-effect of this change is that for register varobjs and some >>>>>> global variable varobjs, the output of -var-create now includes the >>>>>> thread-id field. The rationale for this is as follow: if we ask for a >>>>>> particular expression in a particular frame, that implies a particular >>>>>> thread. Thus including the thread-id is correct behavior, and the >>>>>> test results have been updated accordingly. >>>>>> >>>>>> In addition there is a new test, gdb.mi/mi-frame-regs.exp, which >>>>>> performs the test described in the bullet list above as well as a >>>>>> similar test using a floating variable object to represent $pc. >>>>>> >>>>>> We attempted a couple alternative solutions: >>>>>> * a special case in varobj_update to select the frame was not ideal >>>>>> because it didn't use the existing mechanisms to select the frame. >>>>>> * setting innermost_block in write_dollar_variable had side-effects >>>>>> in the CLI that would have required special case code. >>>>>> >>>>>> Tested on x86_64 Linux native, no regressions. >>>>>> >>>>>> gdb/testsuite/ChangeLog: >>>>>> 2016-06-13 Don Breazeal <dbreazea@my.domain.org> >>>>>> >>>>>> * gdb.ada/mi_interface.exp: Add thread-id field to expected >>>>>> output of -var-create and -var-list-children. >>>>>> * gdb.ada/mi_var_array.exp: Add thread-id field to expected >>>>>> output of -var-list-children. >>>>>> * gdb.mi/mi-break.exp (test_error): Add thread-id field to >>>>>> expected output of -var-create. >>>>>> * gdb.mi/mi-frame-regs.exp: New test script. >>>>>> * gdb.mi/mi-var-cmd.exp: Add thread-id field to expected >>>>>> output of -var-create. >>>>>> * gdb.mi/mi-var-create-rtti.exp: Likewise. >>>>>> >>>>>> gdb/ChangeLog: >>>>>> 2016-06-13 Don Breazeal <donb@codesourcery.com> >>>>>> Andrew Burgess <andrew.burgess@embecosm.com> >>>>>> >>>>>> * varobj.c (varobj_create): Initialize innermost_block to >>>>>> the global block instead of NULL. >>>>>> (new_root_variable): Initialize the thread_id and next >>>>>> fields. >>>>>> (value_of_root_1): Set within_scope if the variable's >>>>>> valid_block field is the global block. >>>>>> >>>>>> --- >>>>>> gdb/testsuite/gdb.ada/mi_interface.exp | 4 +- >>>>>> gdb/testsuite/gdb.ada/mi_var_array.exp | 2 +- >>>>>> gdb/testsuite/gdb.mi/mi-break.exp | 2 +- >>>>>> gdb/testsuite/gdb.mi/mi-frame-regs.exp | 183 ++++++++++++++++++++++++++++ >>>>>> gdb/testsuite/gdb.mi/mi-var-cmd.exp | 4 +- >>>>>> gdb/testsuite/gdb.mi/mi-var-create-rtti.exp | 2 +- >>>>>> gdb/varobj.c | 8 +- >>>>>> 7 files changed, 196 insertions(+), 9 deletions(-) >>>>>> >>>>>> diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp >>>>>> index 6000ec8..b948cd5 100644 >>>>>> --- a/gdb/testsuite/gdb.ada/mi_interface.exp >>>>>> +++ b/gdb/testsuite/gdb.ada/mi_interface.exp >>>>>> @@ -44,9 +44,9 @@ mi_continue_to_line \ >>>>>> "stop at start of main Ada procedure" >>>>>> >>>>>> mi_gdb_test "-var-create ggg1 * ggg1" \ >>>>>> - "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \ >>>>>> + "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \ >>>>>> "Create ggg1 varobj" >>>>>> >>>>>> mi_gdb_test "-var-list-children 1 ggg1" \ >>>>>> - "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \ >>>>>> + "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \ >>>>>> "list ggg1's children" >>>>>> diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp >>>>>> index c648e7e..c02d4c9 100644 >>>>>> --- a/gdb/testsuite/gdb.ada/mi_var_array.exp >>>>>> +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp >>>>>> @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \ >>>>>> "Create bt varobj" >>>>>> >>>>>> mi_gdb_test "-var-list-children vta" \ >>>>>> - "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \ >>>>>> + "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \ >>>>>> "list vta's children" >>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp >>>>>> index 85f328d..bdd43e0 100644 >>>>>> --- a/gdb/testsuite/gdb.mi/mi-break.exp >>>>>> +++ b/gdb/testsuite/gdb.mi/mi-break.exp >>>>>> @@ -213,7 +213,7 @@ proc test_error {} { >>>>>> # containing function call, the internal breakpoint created to handle >>>>>> # function call would be reported, messing up MI output. >>>>>> mi_gdb_test "-var-create V * return_1()" \ >>>>>> - "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \ >>>>>> + "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \ >>>>>> "create varobj for function call" >>>>>> >>>>>> mi_gdb_test "-var-update *" \ >>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>>>>> new file mode 100644 >>>>>> index 0000000..45f81d6 >>>>>> --- /dev/null >>>>>> +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp >>>>>> @@ -0,0 +1,183 @@ >>>>>> +# Copyright 1999-2016 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/>. >>>>>> + >>>>>> +# Test essential Machine interface (MI) operations >>>>>> +# >>>>>> +# Verify that -var-update will provide the correct values for floating >>>>>> +# and fixed varobjs that represent the pc register. >>>>>> +# >>>>>> + >>>>>> +load_lib mi-support.exp >>>>>> +set MIFLAGS "-i=mi" >>>>>> + >>>>>> +standard_testfile basics.c >>>>>> + >>>>>> +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ >>>>>> + executable {debug}] != "" } then { >>>>>> + untested mi-frame-regs.exp >>>>>> + return -1 >>>>>> +} >>>>>> + >>>>>> +# Return the address of the specified breakpoint. >>>>>> + >>>>>> +proc breakpoint_address {bpnum} { >>>>>> + global hex >>>>>> + global expect_out >>>>>> + global mi_gdb_prompt >>>>>> + >>>>>> + send_gdb "info breakpoint $bpnum\n" >>>>>> + gdb_expect { >>>>>> + -re ".*($hex).*$mi_gdb_prompt$" { >>>>>> + return $expect_out(1,string) >>>>>> + } >>>>>> + -re ".*$mi_gdb_prompt$" { >>>>>> + return "" >>>>>> + } >>>>>> + timeout { >>>>>> + return "" >>>>>> + } >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +# Test that a floating varobj representing $pc will provide the >>>>>> +# correct value via -var-update as the program stops at >>>>>> +# breakpoints in different functions. >>>>>> + >>>>>> +proc do_floating_varobj_test {} { >>>>>> + global srcfile >>>>>> + global hex >>>>>> + global expect_out >>>>>> + >>>>>> + gdb_exit >>>>>> + if {[mi_gdb_start]} then { >>>>>> + continue >>>>>> + } >>>>>> + >>>>>> + with_test_prefix "floating" { >>>>>> + mi_run_to_main >>>>>> + >>>>>> + # Create a floating varobj for $pc. >>>>>> + mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \ >>>>>> + "\\^done,.*value=\"$hex.*" \ >>>>>> + "create varobj for pc in frame 0" >>>>>> + >>>>>> + set nframes 4 >>>>>> + for {set i 1} {$i < $nframes} {incr i} { >>>>>> + >>>>>> + # Run to a breakpoint in each callee function in succession. >>>>>> + # Note that we can't use mi_runto because we need the >>>>>> + # breakpoint to be persistent, so we can use its address. >>>>>> + set bpnum [expr $i + 1] >>>>>> + mi_create_breakpoint \ >>>>>> + "basics.c:callee$i" \ >>>>>> + "insert breakpoint at basics.c:callee$i" \ >>>>>> + -number $bpnum -func callee$i -file ".*basics.c" >>>>>> + >>>>>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>>>>> + "callee$i" ".*" ".*${srcfile}" ".*" \ >>>>>> + { "" "disp=\"keep\"" } "breakpoint hit" >>>>>> + >>>>>> + # Get the value of $pc from the floating varobj. >>>>>> + mi_gdb_test "-var-update 1 var1" \ >>>>>> + "\\^done,.*value=\"($hex) .*" \ >>>>>> + "-var-update for frame $i" >>>>>> + set pcval $expect_out(3,string) >>>>>> + >>>>>> + # Get the address of the current breakpoint. >>>>>> + set bpaddr [breakpoint_address $bpnum] >>>>>> + if {$bpaddr == ""} then { >>>>>> + unresolved "get address of breakpoint $bpnum" >>>>>> + return >>>>>> + } >>>>>> + >>>>>> + # Check that the addresses are the same. >>>>>> + if {[expr $bpaddr != $pcval]} then { >>>>>> + fail "\$pc does not equal address of breakpoint" >>>>>> + } else { >>>>>> + pass "\$pc equals address of breakpoint" >>>>>> + } >>>>>> + } >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +# Create a varobj for the pc register in each of the frames other >>>>>> +# than frame 0. >>>>>> + >>>>>> +proc var_create_regs {nframes} { >>>>>> + global hex >>>>>> + >>>>>> + for {set i 1} {$i < $nframes} {incr i} { >>>>>> + mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \ >>>>>> + "\\^done,.*value=\"$hex.*" \ >>>>>> + "create varobj for pc in frame $i" >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +# Check that -var-update reports that the value of the pc register >>>>>> +# for each of the frames 1 and above is reported as unchanged. >>>>>> + >>>>>> +proc var_update_regs {nframes} { >>>>>> + >>>>>> + for {set i 1} {$i < $nframes} {incr i} { >>>>>> + mi_gdb_test "-var-update 1 var$i" \ >>>>>> + "\\^done,(changelist=\\\[\\\])" \ >>>>>> + "pc unchanged in -var-update for frame $i" >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +# Test that fixed varobjs representing $pc in different stack frames >>>>>> +# will provide the correct value via -var-update after the program >>>>>> +# counter changes (without substantially changing the stack). >>>>>> + >>>>>> +proc do_fixed_varobj_test {} { >>>>>> + global srcfile >>>>>> + >>>>>> + gdb_exit >>>>>> + if {[mi_gdb_start]} then { >>>>>> + continue >>>>>> + } >>>>>> + >>>>>> + with_test_prefix "fixed" { >>>>>> + mi_run_to_main >>>>>> + >>>>>> + # Run to the function 'callee3' so we have several frames. >>>>>> + mi_create_breakpoint "basics.c:callee3" \ >>>>>> + "insert breakpoint at basics.c:callee3" \ >>>>>> + -number 2 -func callee3 -file ".*basics.c" >>>>>> + >>>>>> + mi_execute_to "exec-continue" "breakpoint-hit" \ >>>>>> + "callee3" ".*" ".*${srcfile}" ".*" \ >>>>>> + { "" "disp=\"keep\"" } "breakpoint hit" >>>>>> + >>>>>> + # At the breakpoint in callee3 there are 4 frames. Create a >>>>>> + # varobj for the pc in each of frames 1 and above. >>>>>> + set nframes 4 >>>>>> + var_create_regs $nframes >>>>>> + >>>>>> + # Step one instruction to change the program counter. >>>>>> + mi_execute_to "exec-next-instruction" "end-stepping-range" \ >>>>>> + "callee3" ".*" ".*${srcfile}" ".*" "" \ >>>>>> + "next instruction in callee3" >>>>>> + >>>>>> + # Check that -var-update reports that the values are unchanged. >>>>>> + var_update_regs $nframes >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +do_fixed_varobj_test >>>>>> +do_floating_varobj_test >>>>>> + >>>>>> +mi_gdb_exit >>>>>> +return 0 >>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>>>> index 558cd6c..68e3cf8 100644 >>>>>> --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp >>>>>> @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \ >>>>>> "assign same value to func (update)" >>>>>> >>>>>> mi_gdb_test "-var-create array_ptr * array_ptr" \ >>>>>> - "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \ >>>>>> + "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \ >>>>>> "create global variable array_ptr" >>>>>> >>>>>> mi_gdb_test "-var-assign array_ptr array2" \ >>>>>> @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee" >>>>>> # A varobj we fail to read during -var-update should be considered >>>>>> # out of scope. >>>>>> mi_gdb_test "-var-create null_ptr * **0" \ >>>>>> - {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \ >>>>>> + {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \ >>>>>> "create null_ptr" >>>>>> >>>>>> # Allow this to succeed, if address zero is readable, although it >>>>>> diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>>>> index 3bcb36c..a8cc76f 100644 >>>>>> --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>>>> +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp >>>>>> @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*" >>>>>> # We use a explicit cast to (void *) as that is the >>>>>> # type that caused the bug this testcase is testing for. >>>>>> mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \ >>>>>> - "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \ >>>>>> + "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \ >>>>>> "-var-create sp1 * \$sp" >>>>>> gdb_exit >>>>>> diff --git a/gdb/varobj.c b/gdb/varobj.c >>>>>> index 6f56cba..1e3f192 100644 >>>>>> --- a/gdb/varobj.c >>>>>> +++ b/gdb/varobj.c >>>>>> @@ -323,7 +323,7 @@ varobj_create (char *objname, >>>>>> } >>>>>> >>>>>> p = expression; >>>>>> - innermost_block = NULL; >>>>>> + innermost_block = block_global_block (block); >>>>>> /* Wrap the call to parse expression, so we can >>>>>> return a sensible error. */ >>>>>> TRY >>>>>> @@ -2103,6 +2103,8 @@ new_root_variable (void) >>>>>> var->root->floating = 0; >>>>>> var->root->rootvar = NULL; >>>>>> var->root->is_valid = 1; >>>>>> + var->root->thread_id = 0; >>>>>> + var->root->next = NULL; >>>>>> >>>>>> return var; >>>>>> } >>>>>> @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle) >>>>>> back_to = make_cleanup_restore_current_thread (); >>>>>> >>>>>> /* Determine whether the variable is still around. */ >>>>>> - if (var->root->valid_block == NULL || var->root->floating) >>>>>> + if (var->root->valid_block == NULL >>>>>> + || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL >>>>>> + || var->root->floating) >>>>>> within_scope = 1; >>>>>> else if (var->root->thread_id == 0) >>>>>> { >>>>>> >>>>> >>>> >>> >> I've created PR mi/20395 for this issue and marked it with the Target >> Milestone 7.12. Since it's an issue of returning incorrect data I'm >> proposing that it's a blocker. >> thanks >> --Don >> >
On 06/13/2016 10:54 PM, Don Breazeal wrote: > The fix is to change the initialization of innermost_block in > varobj_create from NULL to the global block, as returned by > block_global_block. Hmm, this sounds questionable to me. innermost_block is an output parameter, after all. parse_exp_1 already takes an input block parameter. > Then varobj_create sets varobj->root->frame for register varobjs, and > value_of_root_1 can check for the global block when determining > whether the variable is in-scope and select the frame appropriately. > > A side-effect of this change is that for register varobjs and some > global variable varobjs, the output of -var-create now includes the > thread-id field. The rationale for this is as follow: if we ask for > a particular expression in a particular frame, that implies a > particular thread. Thus including the thread-id is correct behavior, > and the test results have been updated accordingly. Sounds OK for register varobjs, but it's not as clear for global variable varobjs. I see no way to tell -var-create to create a global variable fixed varobj that is _not_ associated with a particular thread: -var-create {name | "-"} {frame-addr | "*" | "@"} expression The docs say: If an expression specified when creating a fixed variable object refers to a local variable, the variable object becomes bound to the thread and frame in which the variable object is created. When such variable object is updated, GDB makes sure that the thread/frame combination the variable object is bound to still exists, and re-evaluates the variable object in context of that thread/frame. So if the expression needed a frame, then it gets bound to the frame/thread. But if it didn't, then it won't, by my reading? I worry about whether this might break frontends. In principle, this sounds similar to watchpoints -- those also need to check whether the original expression is still in scope, for the case of watchpoints on local variables. See update_watchpoint. I'm still trying to wrap my head around all this, and I need to read the varobj code to understand how this all works. Thanks, Pedro Alves
diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp index 6000ec8..b948cd5 100644 --- a/gdb/testsuite/gdb.ada/mi_interface.exp +++ b/gdb/testsuite/gdb.ada/mi_interface.exp @@ -44,9 +44,9 @@ mi_continue_to_line \ "stop at start of main Ada procedure" mi_gdb_test "-var-create ggg1 * ggg1" \ - "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \ + "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \ "Create ggg1 varobj" mi_gdb_test "-var-list-children 1 ggg1" \ - "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \ + "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \ "list ggg1's children" diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp index c648e7e..c02d4c9 100644 --- a/gdb/testsuite/gdb.ada/mi_var_array.exp +++ b/gdb/testsuite/gdb.ada/mi_var_array.exp @@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \ "Create bt varobj" mi_gdb_test "-var-list-children vta" \ - "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \ + "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \ "list vta's children" diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp index 85f328d..bdd43e0 100644 --- a/gdb/testsuite/gdb.mi/mi-break.exp +++ b/gdb/testsuite/gdb.mi/mi-break.exp @@ -213,7 +213,7 @@ proc test_error {} { # containing function call, the internal breakpoint created to handle # function call would be reported, messing up MI output. mi_gdb_test "-var-create V * return_1()" \ - "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \ + "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \ "create varobj for function call" mi_gdb_test "-var-update *" \ diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp new file mode 100644 index 0000000..45f81d6 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp @@ -0,0 +1,183 @@ +# Copyright 1999-2016 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/>. + +# Test essential Machine interface (MI) operations +# +# Verify that -var-update will provide the correct values for floating +# and fixed varobjs that represent the pc register. +# + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +standard_testfile basics.c + +if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \ + executable {debug}] != "" } then { + untested mi-frame-regs.exp + return -1 +} + +# Return the address of the specified breakpoint. + +proc breakpoint_address {bpnum} { + global hex + global expect_out + global mi_gdb_prompt + + send_gdb "info breakpoint $bpnum\n" + gdb_expect { + -re ".*($hex).*$mi_gdb_prompt$" { + return $expect_out(1,string) + } + -re ".*$mi_gdb_prompt$" { + return "" + } + timeout { + return "" + } + } +} + +# Test that a floating varobj representing $pc will provide the +# correct value via -var-update as the program stops at +# breakpoints in different functions. + +proc do_floating_varobj_test {} { + global srcfile + global hex + global expect_out + + gdb_exit + if {[mi_gdb_start]} then { + continue + } + + with_test_prefix "floating" { + mi_run_to_main + + # Create a floating varobj for $pc. + mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \ + "\\^done,.*value=\"$hex.*" \ + "create varobj for pc in frame 0" + + set nframes 4 + for {set i 1} {$i < $nframes} {incr i} { + + # Run to a breakpoint in each callee function in succession. + # Note that we can't use mi_runto because we need the + # breakpoint to be persistent, so we can use its address. + set bpnum [expr $i + 1] + mi_create_breakpoint \ + "basics.c:callee$i" \ + "insert breakpoint at basics.c:callee$i" \ + -number $bpnum -func callee$i -file ".*basics.c" + + mi_execute_to "exec-continue" "breakpoint-hit" \ + "callee$i" ".*" ".*${srcfile}" ".*" \ + { "" "disp=\"keep\"" } "breakpoint hit" + + # Get the value of $pc from the floating varobj. + mi_gdb_test "-var-update 1 var1" \ + "\\^done,.*value=\"($hex) .*" \ + "-var-update for frame $i" + set pcval $expect_out(3,string) + + # Get the address of the current breakpoint. + set bpaddr [breakpoint_address $bpnum] + if {$bpaddr == ""} then { + unresolved "get address of breakpoint $bpnum" + return + } + + # Check that the addresses are the same. + if {[expr $bpaddr != $pcval]} then { + fail "\$pc does not equal address of breakpoint" + } else { + pass "\$pc equals address of breakpoint" + } + } + } +} + +# Create a varobj for the pc register in each of the frames other +# than frame 0. + +proc var_create_regs {nframes} { + global hex + + for {set i 1} {$i < $nframes} {incr i} { + mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \ + "\\^done,.*value=\"$hex.*" \ + "create varobj for pc in frame $i" + } +} + +# Check that -var-update reports that the value of the pc register +# for each of the frames 1 and above is reported as unchanged. + +proc var_update_regs {nframes} { + + for {set i 1} {$i < $nframes} {incr i} { + mi_gdb_test "-var-update 1 var$i" \ + "\\^done,(changelist=\\\[\\\])" \ + "pc unchanged in -var-update for frame $i" + } +} + +# Test that fixed varobjs representing $pc in different stack frames +# will provide the correct value via -var-update after the program +# counter changes (without substantially changing the stack). + +proc do_fixed_varobj_test {} { + global srcfile + + gdb_exit + if {[mi_gdb_start]} then { + continue + } + + with_test_prefix "fixed" { + mi_run_to_main + + # Run to the function 'callee3' so we have several frames. + mi_create_breakpoint "basics.c:callee3" \ + "insert breakpoint at basics.c:callee3" \ + -number 2 -func callee3 -file ".*basics.c" + + mi_execute_to "exec-continue" "breakpoint-hit" \ + "callee3" ".*" ".*${srcfile}" ".*" \ + { "" "disp=\"keep\"" } "breakpoint hit" + + # At the breakpoint in callee3 there are 4 frames. Create a + # varobj for the pc in each of frames 1 and above. + set nframes 4 + var_create_regs $nframes + + # Step one instruction to change the program counter. + mi_execute_to "exec-next-instruction" "end-stepping-range" \ + "callee3" ".*" ".*${srcfile}" ".*" "" \ + "next instruction in callee3" + + # Check that -var-update reports that the values are unchanged. + var_update_regs $nframes + } +} + +do_fixed_varobj_test +do_floating_varobj_test + +mi_gdb_exit +return 0 diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp index 558cd6c..68e3cf8 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp +++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp @@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \ "assign same value to func (update)" mi_gdb_test "-var-create array_ptr * array_ptr" \ - "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \ + "\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \ "create global variable array_ptr" mi_gdb_test "-var-assign array_ptr array2" \ @@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee" # A varobj we fail to read during -var-update should be considered # out of scope. mi_gdb_test "-var-create null_ptr * **0" \ - {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \ + {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \ "create null_ptr" # Allow this to succeed, if address zero is readable, although it diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp index 3bcb36c..a8cc76f 100644 --- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp +++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp @@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*" # We use a explicit cast to (void *) as that is the # type that caused the bug this testcase is testing for. mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \ - "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \ + "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \ "-var-create sp1 * \$sp" gdb_exit diff --git a/gdb/varobj.c b/gdb/varobj.c index 6f56cba..1e3f192 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -323,7 +323,7 @@ varobj_create (char *objname, } p = expression; - innermost_block = NULL; + innermost_block = block_global_block (block); /* Wrap the call to parse expression, so we can return a sensible error. */ TRY @@ -2103,6 +2103,8 @@ new_root_variable (void) var->root->floating = 0; var->root->rootvar = NULL; var->root->is_valid = 1; + var->root->thread_id = 0; + var->root->next = NULL; return var; } @@ -2268,7 +2270,9 @@ value_of_root_1 (struct varobj **var_handle) back_to = make_cleanup_restore_current_thread (); /* Determine whether the variable is still around. */ - if (var->root->valid_block == NULL || var->root->floating) + if (var->root->valid_block == NULL + || BLOCK_SUPERBLOCK (var->root->valid_block) == NULL + || var->root->floating) within_scope = 1; else if (var->root->thread_id == 0) {