This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] D: Fix crash when expression debugging


On 01/04/2017 02:38 PM, Iain Buclaw wrote:
On 4 January 2017 at 20:48, Luis Machado <lgustavo@codesourcery.com> wrote:
On 01/04/2017 01:38 PM, Iain Buclaw wrote:

On 4 January 2017 at 17:08, Luis Machado <lgustavo@codesourcery.com>
wrote:

On 01/03/2017 06:02 PM, Iain Buclaw wrote:


This patch fixes a crash found on "p *(type *)0x1234" when using "set
debug expression 1".

While casting works as expected with expression debugging turned off,
this seems to be an indication that I'm infact doing something wrong
in the building of the expression.

(gdb) print *(int*)(0x0)
Dump of expression @ 0x12db320, before conversion to prefix form:
    Language d, 11 elements, 16 bytes each.
    Index                Opcode         Hex Value  String Value
        0               OP_TYPE  87  W...............
        1    <unknown 20114800>  20114800  p.2.............
        2               OP_TYPE  87  W...............
        3               OP_LONG  38  &...............
        4    <unknown 19696640>  19696640  ..,.............
        5               OP_NULL  0  ................
        6               OP_LONG  38  &...............
        7             UNOP_CAST  51  3...............
        8    <unknown 20114800>  20114800  p.2.............
        9             UNOP_CAST  51  3...............
       10              UNOP_IND  61  =...............
Dump of expression @ 0x12db320, after conversion to prefix form:
Expression: `*(int *) 0'
    Language d, 11 elements, 16 bytes each.


        0  UNOP_IND
        1    UNOP_CAST             Type @0x132ed70 (int *)
        4      OP_LONG               Type @0x12c8c00 (int), value 0
(0x0)
        8  <unknown 20114800>    Unknown format
        9  UNOP_CAST             Type @0x3d (Segmentation fault


Looks like using UNOP_CAST_TYPE is the right thing to do here, as the
TypeExp handler has wrapped the type around a pair of OP_TYPE opcodes.


dlang-debug-expr.patch


2017-01-04  Iain Buclaw  <ibuclaw@gdcproject.org>

gdb/ChangeLog:

        * d-exp.y (CastExpression): Emit UNOP_CAST_TYPE.

gdb/testsuite/ChangeLog:

        * gdb.dlang/debug-expr.exp: New file.

---
diff --git a/gdb/d-exp.y b/gdb/d-exp.y
index 077e645..91d15f2 100644
--- a/gdb/d-exp.y
+++ b/gdb/d-exp.y
@@ -321,15 +321,12 @@ UnaryExpression:

 CastExpression:
        CAST_KEYWORD '(' TypeExp ')' UnaryExpression
-               { write_exp_elt_opcode (pstate, UNOP_CAST);
-                 write_exp_elt_type (pstate, $3);
-                 write_exp_elt_opcode (pstate, UNOP_CAST); }
+               { write_exp_elt_opcode (pstate, UNOP_CAST_TYPE); }
        /* C style cast is illegal D, but is still recognised in
           the grammar, so we keep this around for convenience.  */
 |      '(' TypeExp ')' UnaryExpression
-               { write_exp_elt_opcode (pstate, UNOP_CAST);
-                 write_exp_elt_type (pstate, $2);
-                 write_exp_elt_opcode (pstate, UNOP_CAST); }
+               { write_exp_elt_opcode (pstate, UNOP_CAST_TYPE); }
+
 ;

 PowExpression:
diff --git a/gdb/testsuite/gdb.dlang/debug-expr.exp
b/gdb/testsuite/gdb.dlang/debug-expr.exp
new file mode 100644
index 0000000..3bb2c09
--- /dev/null
+++ b/gdb/testsuite/gdb.dlang/debug-expr.exp
@@ -0,0 +1,40 @@
+# Copyright 2017 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 "set debug expr 1" on d expressions.
+
+if { [skip_d_tests] } { continue }



We should output a message:

untested "skipping d language tests"

It may be more reasonable to just return instead of continuing? The
effect
will probably be the same, but it is a bit confusing to read "continue"
without a visible loop.


I did a quick grep, and it seems like everyone is skippingtests in
this way except for gdb.ada and gdb.btrace which are doing { return -1
}



That's a bit of a stretch. Take, for example, a few of the examples in
gdb.base. You will see a number of them returning.

The problem here is inheriting past confusing practices when we use some
existing files to create new ones, which is not your fault really. I'm
guilty myself. :-)


Yes indeed. I wasn't disagreeing, just questioning the two competing
ways of returning.

I will update to use return and push this in then if there's no
disagreement. :-)


Hopefully someone will chime in for a second opinion. :-)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]