From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3221 invoked by alias); 28 Sep 2011 19:48:49 -0000 Received: (qmail 3213 invoked by uid 22791); 28 Sep 2011 19:48:47 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: sourceware.org Received: from qmta14.westchester.pa.mail.comcast.net (HELO qmta14.westchester.pa.mail.comcast.net) (76.96.59.212) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 28 Sep 2011 19:48:33 +0000 Received: from omta23.westchester.pa.mail.comcast.net ([76.96.62.74]) by qmta14.westchester.pa.mail.comcast.net with comcast id eHhY1h0041c6gX85EKoZLy; Wed, 28 Sep 2011 19:48:33 +0000 Received: from [10.127.238.91] ([65.206.2.68]) by omta23.westchester.pa.mail.comcast.net with comcast id eKoP1h00W1U2a2h3jKoReZ; Wed, 28 Sep 2011 19:48:31 +0000 Subject: Re: Python: fetch value when building gdb.Value object Mime-Version: 1.0 (Apple Message framework v1084) Content-Type: text/plain; charset=us-ascii From: Paul Koning In-Reply-To: Date: Wed, 28 Sep 2011 20:06:00 -0000 Cc: gdb-patches@sourceware.org Content-Transfer-Encoding: quoted-printable Message-Id: References: <36B29E9D-F2B3-446F-AF8A-97254A3AAEE2@comcast.net> To: pmuldoon@redhat.com Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-09/txt/msg00475.txt.bz2 On Sep 28, 2011, at 3:24 PM, Phil Muldoon wrote: > Paul Koning writes: >=20 >> GDB sometimes lazily evaluates operations on values, and py-value.c wasn= 't taking that into account. The result was that assigning a Value object = to a Python variable could assign a lazy value, so that any errors in acces= sing the data would occur at a later time, and sometimes would not be handl= ed right. (For example, the "nonzero" operation would fail without a Pytho= n traceback.) >>=20 >> The attached patch cures this by fetching any lazy values when the gdb.V= alue object is built, and adds a test in the testcases to verify this. >>=20 >> Ok to submit? >>=20 >> paul >>=20 >> ChangeLog: >>=20=09 >> 2011-09-21 Paul Koning >>=20 >> * python/py-value.c (valpy_get_address): Use Py_XINCREF. >> (value_to_value_object): Fetch value if it was lazy. >>=20 >> testsuite/ChangeLog: >>=20=09 >> 2011-09-21 Paul Koning >>=20 >> * gdb.python/py-value.exp: Add test for null pointer reference >> assigned to a variable. >>=20 >> Index: python/py-value.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> RCS file: /cvs/src/src/gdb/python/py-value.c,v >> retrieving revision 1.25 >> diff -u -r1.25 py-value.c >> --- python/py-value.c 27 Jun 2011 19:21:51 -0000 1.25 >> +++ python/py-value.c 21 Sep 2011 15:45:12 -0000 >> @@ -209,7 +209,7 @@ >> val_obj->address =3D value_to_value_object (res_val); >> } >>=20 >> - Py_INCREF (val_obj->address); >> + Py_XINCREF (val_obj->address); >>=20 >> return val_obj->address; >> } >=20 > This seems an unrelated change? With the next change, value_to_value_object can return NULL (if the value_f= etch_lazy generates an error). So we now need XINCREF since that works wit= h a null pointer, while INCREF will crash if handed a null pointer. This i= s standard Python API coding style when dealing with a pointer that might b= e null because of an earlier failure, or because of a code path that doesn'= t set it. >=20 >> @@ -1045,7 +1045,15 @@ >> value_to_value_object (struct value *val) >> { >> value_object *val_obj; >> + volatile struct gdb_exception except; >>=20 >> + TRY_CATCH (except, RETURN_MASK_ALL) >> + { >=20 > Something that Jan pointed out a few weeks ago, is our exception net is > too wide, and asked me to review usage of REVIEW_MASK_ALL. In this > case, this should probably be RETURN_MASK_ERROR.=20=20 I'll investigate. I'm not familiar with the GDB error catching machinery s= o I have some learning to do. No problem. >=20 >=20 >> # Test memory error. >> gdb_test "python print gdb.parse_and_eval('*(int*)0')" "gdb.MemoryErro= r: Cannot access memory at address 0x0.*" >> + gdb_test "python inval =3D gdb.parse_and_eval('*(int*)0')" "gdb.Memor= yError: Cannot access memory at address 0x0.*" >=20 > What scenario will this test catch that the previous test won't? I'm > not saying you are incorrect, I just don't understand. What > error-trigger does the assignment to "inval" trigger? If you execute that statement on the existing code, the statement will succ= eed with no error reported. However, when you then attempt to use the valu= e, you get an error at that time: (gdb) python inval =3D gdb.parse_and_eval('*(int*)0') (gdb) python foo=3Dinval+1 Traceback (most recent call last): File "", line 1, in ? gdb.MemoryError: Cannot access memory at address 0x0 Error while executing Python code. (gdb) python bar=3Dbool(inval) Cannot access memory at address 0x0 (gdb) quit The first of those two has a traceback because the gdb.Value arithmetic ope= rations have a TRY_CATCH in them. The second one has no traceback because = the "nonzero" method of gdb.Value doesn't do TRY_CATCH.=20=20 With the patch you get this: (gdb) python inval =3D gdb.parse_and_eval('*(int*)0') Traceback (most recent call last): File "", line 1, in ? gdb.MemoryError: Cannot access memory at address 0x0 Error while executing Python code. I.e., you get the error at the time you write the operation that performs t= he bad access. The reason the previous test (with the print) works as expe= cted is that it creates the gdb.Value object (which would work with no erro= r) and then asks for the value of that object to print it (that's when the = error is noticed). With the patch it still works the same way as far as th= e output you see, although now the error is detected earlier in the executi= on of that statement. paul