From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11979 invoked by alias); 18 Feb 2011 21:11:35 -0000 Received: (qmail 11967 invoked by uid 22791); 18 Feb 2011 21:11:32 -0000 X-SWARE-Spam-Status: No, hits=-6.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 Feb 2011 21:11:23 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p1ILBFcv026218 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 18 Feb 2011 16:11:15 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p1ILBEdX005253; Fri, 18 Feb 2011 16:11:14 -0500 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id p1ILBDNW017064; Fri, 18 Feb 2011 16:11:13 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id CD3A1378BE1; Fri, 18 Feb 2011 14:11:12 -0700 (MST) From: Tom Tromey To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: RFC: move agent opcodes to common file References: <201102181856.05577.pedro@codesourcery.com> Date: Fri, 18 Feb 2011 23:35:00 -0000 In-Reply-To: <201102181856.05577.pedro@codesourcery.com> (Pedro Alves's message of "Fri, 18 Feb 2011 18:56:05 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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-02/txt/msg00502.txt.bz2 >>>>> "Pedro" == Pedro Alves writes: Pedro> Looks good to me. Pedro> Hmm. I think it'd be a good idea to wrap the direct accesses Pedro> to the gdb_agent_op_names array (there are a few Pedro> `gdb_agent_op_names[op]' in the code, in a function that Pedro> checks that "op" doesn't overflow the array length before Pedro> dereferencing it, and returns "?undef?" or something like that Pedro> if it does over- or underflow. It look like it'll happen if Pedro> you connect a new gdb to an older gdbserver with --debug enabled, Pedro> and send it the new opcodes. I implemented this. I fixed gdbserver/Makefile.in, too. Here is what I am checking in. Tom 2011-02-18 Tom Tromey * common/ax.def: New file. * ax.h (enum agent_op): Use ax.def. * ax-general.c (aop_map): Use ax.def. 2011-02-18 Tom Tromey * Makefile.in (tracepoint-ipa.o): Depend on ax.def. (tracepoint.o): Likewise. * tracepoint.c (enum gdb_agent_op): Use ax.def. (gdb_agent_op_names): Likewise. Index: ax-general.c =================================================================== RCS file: /cvs/src/src/gdb/ax-general.c,v retrieving revision 1.24 diff -u -r1.24 ax-general.c --- ax-general.c 18 Feb 2011 20:55:43 -0000 1.24 +++ ax-general.c 18 Feb 2011 21:10:10 -0000 @@ -338,58 +338,11 @@ struct aop_map aop_map[] = { - {0, 0, 0, 0, 0}, - {"float", 0, 0, 0, 0}, /* 0x01 */ - {"add", 0, 0, 2, 1}, /* 0x02 */ - {"sub", 0, 0, 2, 1}, /* 0x03 */ - {"mul", 0, 0, 2, 1}, /* 0x04 */ - {"div_signed", 0, 0, 2, 1}, /* 0x05 */ - {"div_unsigned", 0, 0, 2, 1}, /* 0x06 */ - {"rem_signed", 0, 0, 2, 1}, /* 0x07 */ - {"rem_unsigned", 0, 0, 2, 1}, /* 0x08 */ - {"lsh", 0, 0, 2, 1}, /* 0x09 */ - {"rsh_signed", 0, 0, 2, 1}, /* 0x0a */ - {"rsh_unsigned", 0, 0, 2, 1}, /* 0x0b */ - {"trace", 0, 0, 2, 0}, /* 0x0c */ - {"trace_quick", 1, 0, 1, 1}, /* 0x0d */ - {"log_not", 0, 0, 1, 1}, /* 0x0e */ - {"bit_and", 0, 0, 2, 1}, /* 0x0f */ - {"bit_or", 0, 0, 2, 1}, /* 0x10 */ - {"bit_xor", 0, 0, 2, 1}, /* 0x11 */ - {"bit_not", 0, 0, 1, 1}, /* 0x12 */ - {"equal", 0, 0, 2, 1}, /* 0x13 */ - {"less_signed", 0, 0, 2, 1}, /* 0x14 */ - {"less_unsigned", 0, 0, 2, 1}, /* 0x15 */ - {"ext", 1, 0, 1, 1}, /* 0x16 */ - {"ref8", 0, 8, 1, 1}, /* 0x17 */ - {"ref16", 0, 16, 1, 1}, /* 0x18 */ - {"ref32", 0, 32, 1, 1}, /* 0x19 */ - {"ref64", 0, 64, 1, 1}, /* 0x1a */ - {"ref_float", 0, 0, 1, 1}, /* 0x1b */ - {"ref_double", 0, 0, 1, 1}, /* 0x1c */ - {"ref_long_double", 0, 0, 1, 1}, /* 0x1d */ - {"l_to_d", 0, 0, 1, 1}, /* 0x1e */ - {"d_to_l", 0, 0, 1, 1}, /* 0x1f */ - {"if_goto", 2, 0, 1, 0}, /* 0x20 */ - {"goto", 2, 0, 0, 0}, /* 0x21 */ - {"const8", 1, 8, 0, 1}, /* 0x22 */ - {"const16", 2, 16, 0, 1}, /* 0x23 */ - {"const32", 4, 32, 0, 1}, /* 0x24 */ - {"const64", 8, 64, 0, 1}, /* 0x25 */ - {"reg", 2, 0, 0, 1}, /* 0x26 */ - {"end", 0, 0, 0, 0}, /* 0x27 */ - {"dup", 0, 0, 1, 2}, /* 0x28 */ - {"pop", 0, 0, 1, 0}, /* 0x29 */ - {"zero_ext", 1, 0, 1, 1}, /* 0x2a */ - {"swap", 0, 0, 2, 2}, /* 0x2b */ - {"getv", 2, 0, 0, 1}, /* 0x2c */ - {"setv", 2, 0, 0, 1}, /* 0x2d */ - {"tracev", 2, 0, 0, 1}, /* 0x2e */ - {0, 0, 0, 0, 0}, /* 0x2f */ - {"trace16", 2, 0, 1, 1}, /* 0x30 */ - {0, 0, 0, 0, 0}, /* 0x31 */ - {"pick", 1, 0, 0, 1}, /* 0x32 */ - {"rot", 0, 0, 3, 3}, /* 0x33 */ + {0, 0, 0, 0, 0} +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE) \ + , { # NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED } +#include "ax.def" +#undef DEFOP }; Index: ax.h =================================================================== RCS file: /cvs/src/src/gdb/ax.h,v retrieving revision 1.16 diff -u -r1.16 ax.h --- ax.h 18 Feb 2011 20:55:43 -0000 1.16 +++ ax.h 18 Feb 2011 21:10:10 -0000 @@ -145,67 +145,14 @@ unsigned char *reg_mask; }; -/* The actual values of the various bytecode operations. - - Other independent implementations of the agent bytecode engine will - rely on the exact values of these enums, and may not be recompiled - when we change this table. The numeric values should remain fixed - whenever possible. Thus, we assign them values explicitly here (to - allow gaps to form safely), and the disassembly table in - agentexpr.h behaves like an opcode map. If you want to see them - grouped logically, see doc/agentexpr.texi. */ +/* The actual values of the various bytecode operations. */ enum agent_op { - aop_float = 0x01, - aop_add = 0x02, - aop_sub = 0x03, - aop_mul = 0x04, - aop_div_signed = 0x05, - aop_div_unsigned = 0x06, - aop_rem_signed = 0x07, - aop_rem_unsigned = 0x08, - aop_lsh = 0x09, - aop_rsh_signed = 0x0a, - aop_rsh_unsigned = 0x0b, - aop_trace = 0x0c, - aop_trace_quick = 0x0d, - aop_log_not = 0x0e, - aop_bit_and = 0x0f, - aop_bit_or = 0x10, - aop_bit_xor = 0x11, - aop_bit_not = 0x12, - aop_equal = 0x13, - aop_less_signed = 0x14, - aop_less_unsigned = 0x15, - aop_ext = 0x16, - aop_ref8 = 0x17, - aop_ref16 = 0x18, - aop_ref32 = 0x19, - aop_ref64 = 0x1a, - aop_ref_float = 0x1b, - aop_ref_double = 0x1c, - aop_ref_long_double = 0x1d, - aop_l_to_d = 0x1e, - aop_d_to_l = 0x1f, - aop_if_goto = 0x20, - aop_goto = 0x21, - aop_const8 = 0x22, - aop_const16 = 0x23, - aop_const32 = 0x24, - aop_const64 = 0x25, - aop_reg = 0x26, - aop_end = 0x27, - aop_dup = 0x28, - aop_pop = 0x29, - aop_zero_ext = 0x2a, - aop_swap = 0x2b, - aop_getv = 0x2c, - aop_setv = 0x2d, - aop_tracev = 0x2e, - aop_trace16 = 0x30, - aop_pick = 0x32, - aop_rot = 0x33, +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE) \ + aop_ ## NAME = VALUE, +#include "ax.def" +#undef DEFOP aop_last }; Index: common/ax.def =================================================================== RCS file: common/ax.def diff -N common/ax.def --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ common/ax.def 18 Feb 2011 21:10:10 -0000 @@ -0,0 +1,97 @@ +/* Definition of agent opcode values. -*- c -*- + Copyright (C) 1998, 1999, 2000, 2007, 2008, 2009, 2010, 2011 + Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +/* The actual values of the various bytecode operations. + + Other independent implementations of the agent bytecode engine will + rely on the exact values of these enums, and may not be recompiled + when we change this table. The numeric values should remain fixed + whenever possible. Thus, we assign them values explicitly here (to + allow gaps to form safely), and the disassembly table in + agentexpr.h behaves like an opcode map. If you want to see them + grouped logically, see doc/agentexpr.texi. + + Each line is of the form: + + DEFOP (name, size, data_size, consumed, produced, opcode) + + NAME is the name of the operation. + SIZE is the number of argument bytes that the operation takes from + the bytecode stream. + DATA_SIZE is the size of data operated on, in bits, for operations + that care (ref and const). It is zero otherwise. + CONSUMED is the number of stack elements consumed. + PRODUCED is the number of stack elements produced. + OPCODE is the operation's encoding. */ + +DEFOP (float, 0, 0, 0, 0, 0x01) +DEFOP (add, 0, 0, 2, 1, 0x02) +DEFOP (sub, 0, 0, 2, 1, 0x03) +DEFOP (mul, 0, 0, 2, 1, 0x04) +DEFOP (div_signed, 0, 0, 2, 1, 0x05) +DEFOP (div_unsigned, 0, 0, 2, 1, 0x06) +DEFOP (rem_signed, 0, 0, 2, 1, 0x07) +DEFOP (rem_unsigned, 0, 0, 2, 1, 0x08) +DEFOP (lsh, 0, 0, 2, 1, 0x09) +DEFOP (rsh_signed, 0, 0, 2, 1, 0x0a) +DEFOP (rsh_unsigned, 0, 0, 2, 1, 0x0b) +DEFOP (trace, 0, 0, 2, 0, 0x0c) +DEFOP (trace_quick, 1, 0, 1, 1, 0x0d) +DEFOP (log_not, 0, 0, 1, 1, 0x0e) +DEFOP (bit_and, 0, 0, 2, 1, 0x0f) +DEFOP (bit_or, 0, 0, 2, 1, 0x10) +DEFOP (bit_xor, 0, 0, 2, 1, 0x11) +DEFOP (bit_not, 0, 0, 1, 1, 0x12) +DEFOP (equal, 0, 0, 2, 1, 0x13) +DEFOP (less_signed, 0, 0, 2, 1, 0x14) +DEFOP (less_unsigned, 0, 0, 2, 1, 0x15) +DEFOP (ext, 1, 0, 1, 1, 0x16) +DEFOP (ref8, 0, 8, 1, 1, 0x17) +DEFOP (ref16, 0, 16, 1, 1, 0x18) +DEFOP (ref32, 0, 32, 1, 1, 0x19) +DEFOP (ref64, 0, 64, 1, 1, 0x1a) +DEFOP (ref_float, 0, 0, 1, 1, 0x1b) +DEFOP (ref_double, 0, 0, 1, 1, 0x1c) +DEFOP (ref_long_double, 0, 0, 1, 1, 0x1d) +DEFOP (l_to_d, 0, 0, 1, 1, 0x1e) +DEFOP (d_to_l, 0, 0, 1, 1, 0x1f) +DEFOP (if_goto, 2, 0, 1, 0, 0x20) +DEFOP (goto, 2, 0, 0, 0, 0x21) +DEFOP (const8, 1, 8, 0, 1, 0x22) +DEFOP (const16, 2, 16, 0, 1, 0x23) +DEFOP (const32, 4, 32, 0, 1, 0x24) +DEFOP (const64, 8, 64, 0, 1, 0x25) +DEFOP (reg, 2, 0, 0, 1, 0x26) +DEFOP (end, 0, 0, 0, 0, 0x27) +DEFOP (dup, 0, 0, 1, 2, 0x28) +DEFOP (pop, 0, 0, 1, 0, 0x29) +DEFOP (zero_ext, 1, 0, 1, 1, 0x2a) +DEFOP (swap, 0, 0, 2, 2, 0x2b) +DEFOP (getv, 2, 0, 0, 1, 0x2c) +DEFOP (setv, 2, 0, 0, 1, 0x2d) +DEFOP (tracev, 2, 0, 0, 1, 0x2e) +/* We need something here just to make the tables come out ok. */ +DEFOP (invalid, 0, 0, 0, 0, 0x2f) +DEFOP (trace16, 2, 0, 1, 1, 0x30) +/* We need something here just to make the tables come out ok. */ +DEFOP (invalid2, 0, 0, 0, 0, 0x2f) +/* The "consumed" number for pick is wrong, but there's no way to + express the right thing. */ +DEFOP (pick, 1, 0, 0, 1, 0x32) +DEFOP (rot, 0, 0, 3, 3, 0x33) Index: gdbserver/Makefile.in =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/Makefile.in,v retrieving revision 1.102 diff -u -r1.102 Makefile.in --- gdbserver/Makefile.in 11 Feb 2011 09:57:25 -0000 1.102 +++ gdbserver/Makefile.in 18 Feb 2011 21:10:10 -0000 @@ -382,7 +382,7 @@ -fvisibility=hidden # In-process agent object rules -tracepoint-ipa.o: tracepoint.c $(server_h) +tracepoint-ipa.o: tracepoint.c $(server_h) $(srcdir)/../common/ax.def $(CC) -c $(IPAGENT_CFLAGS) $< -o tracepoint-ipa.o utils-ipa.o: utils.c $(server_h) $(CC) -c $(IPAGENT_CFLAGS) $< -o utils-ipa.o @@ -410,7 +410,7 @@ server.o: server.c $(server_h) target.o: target.c $(server_h) thread-db.o: thread-db.c $(server_h) $(linux_low_h) $(gdb_proc_service_h) -tracepoint.o: tracepoint.c $(server_h) +tracepoint.o: tracepoint.c $(server_h) $(srcdir)/../common/ax.def utils.o: utils.c $(server_h) gdbreplay.o: gdbreplay.c config.h Index: gdbserver/tracepoint.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/tracepoint.c,v retrieving revision 1.18 diff -u -r1.18 tracepoint.c --- gdbserver/tracepoint.c 18 Feb 2011 20:55:45 -0000 1.18 +++ gdbserver/tracepoint.c 18 Feb 2011 21:10:14 -0000 @@ -470,112 +470,19 @@ enum gdb_agent_op { - gdb_agent_op_float = 0x01, - gdb_agent_op_add = 0x02, - gdb_agent_op_sub = 0x03, - gdb_agent_op_mul = 0x04, - gdb_agent_op_div_signed = 0x05, - gdb_agent_op_div_unsigned = 0x06, - gdb_agent_op_rem_signed = 0x07, - gdb_agent_op_rem_unsigned = 0x08, - gdb_agent_op_lsh = 0x09, - gdb_agent_op_rsh_signed = 0x0a, - gdb_agent_op_rsh_unsigned = 0x0b, - gdb_agent_op_trace = 0x0c, - gdb_agent_op_trace_quick = 0x0d, - gdb_agent_op_log_not = 0x0e, - gdb_agent_op_bit_and = 0x0f, - gdb_agent_op_bit_or = 0x10, - gdb_agent_op_bit_xor = 0x11, - gdb_agent_op_bit_not = 0x12, - gdb_agent_op_equal = 0x13, - gdb_agent_op_less_signed = 0x14, - gdb_agent_op_less_unsigned = 0x15, - gdb_agent_op_ext = 0x16, - gdb_agent_op_ref8 = 0x17, - gdb_agent_op_ref16 = 0x18, - gdb_agent_op_ref32 = 0x19, - gdb_agent_op_ref64 = 0x1a, - gdb_agent_op_ref_float = 0x1b, - gdb_agent_op_ref_double = 0x1c, - gdb_agent_op_ref_long_double = 0x1d, - gdb_agent_op_l_to_d = 0x1e, - gdb_agent_op_d_to_l = 0x1f, - gdb_agent_op_if_goto = 0x20, - gdb_agent_op_goto = 0x21, - gdb_agent_op_const8 = 0x22, - gdb_agent_op_const16 = 0x23, - gdb_agent_op_const32 = 0x24, - gdb_agent_op_const64 = 0x25, - gdb_agent_op_reg = 0x26, - gdb_agent_op_end = 0x27, - gdb_agent_op_dup = 0x28, - gdb_agent_op_pop = 0x29, - gdb_agent_op_zero_ext = 0x2a, - gdb_agent_op_swap = 0x2b, - gdb_agent_op_getv = 0x2c, - gdb_agent_op_setv = 0x2d, - gdb_agent_op_tracev = 0x2e, - gdb_agent_op_trace16 = 0x30, - gdb_agent_op_pick = 0x32, - gdb_agent_op_rot = 0x33, +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE) \ + gdb_agent_op_ ## NAME = VALUE, +#include "ax.def" +#undef DEFOP gdb_agent_op_last }; static const char *gdb_agent_op_names [gdb_agent_op_last] = { - "?undef?", - "float", - "add", - "sub", - "mul", - "div_signed", - "div_unsigned", - "rem_signed", - "rem_unsigned", - "lsh", - "rsh_signed", - "rsh_unsigned", - "trace", - "trace_quick", - "log_not", - "bit_and", - "bit_or", - "bit_xor", - "bit_not", - "equal", - "less_signed", - "less_unsigned", - "ext", - "ref8", - "ref16", - "ref32", - "ref64", - "ref_float", - "ref_double", - "ref_long_double", - "l_to_d", - "d_to_l", - "if_goto", - "goto", - "const8", - "const16", - "const32", - "const64", - "reg", - "end", - "dup", - "pop", - "zero_ext", - "swap", - "getv", - "setv", - "tracev", - "?undef?", - "trace16", - "?undef?", - "pick", - "rot" + "?undef?" +#define DEFOP(NAME, SIZE, DATA_SIZE, CONSUMED, PRODUCED, VALUE) , # NAME +#include "ax.def" +#undef DEFOP }; struct agent_expr @@ -4297,6 +4204,16 @@ #endif +/* A wrapper for gdb_agent_op_names that does some bounds-checking. */ + +static const char * +gdb_agent_op_name (int op) +{ + if (op < 0 || op >= gdb_agent_op_last || gdb_agent_op_names[op] == NULL) + return "?undef?"; + return gdb_agent_op_names[op]; +} + /* The agent expression evaluator, as specified by the GDB docs. It returns 0 if everything went OK, and a nonzero error code otherwise. */ @@ -4690,7 +4607,7 @@ } trace_debug ("Op %s -> sp=%d, top=0x%s", - gdb_agent_op_names[op], sp, pulongest (top)); + gdb_agent_op_name (op), sp, pulongest (top)); } } @@ -6000,11 +5917,11 @@ if (emit_error) { trace_debug ("Error %d while emitting code for %s\n", - emit_error, gdb_agent_op_names[op]); + emit_error, gdb_agent_op_name (op)); return expr_eval_unhandled_opcode; } - trace_debug ("Op %s compiled\n", gdb_agent_op_names[op]); + trace_debug ("Op %s compiled\n", gdb_agent_op_name (op)); } /* Now fill in real addresses as goto destinations. */