From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1813 invoked by alias); 11 Apr 2011 03:52:06 -0000 Received: (qmail 1805 invoked by uid 22791); 11 Apr 2011 03:52:06 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 11 Apr 2011 03:52:02 +0000 Received: (qmail 26059 invoked from network); 11 Apr 2011 03:52:01 -0000 Received: from unknown (HELO ?192.168.0.102?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 11 Apr 2011 03:52:01 -0000 Message-ID: <4DA27ADA.9050305@codesourcery.com> Date: Mon, 11 Apr 2011 03:52:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8 MIME-Version: 1.0 To: Sergio Durigan Junior CC: gdb-patches@sourceware.org, Tom Tromey Subject: Re: [PATCH 4/6] Implement support for SystemTap probes References: <4D9D243A.3090505@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-04/txt/msg00146.txt.bz2 On 04/08/2011 08:37 PM, Sergio Durigan Junior wrote: > +static void > +stap_compile_to_ax_1 (struct objfile *objfile, > + const struct stap_probe *probe, > + struct agent_expr *expr, > + struct axs_value *value, > + int n) > +{ > + struct stap_evaluation_info eval_info; > + struct gdbarch *gdbarch = expr->gdbarch; > + char *s = (char *) probe->parsed_args->arg[n].arg_str; > + > + /* Filling necessary information for evaluation function. */ > + eval_info.saved_expr = s; > + eval_info.exp_buf = s; > + eval_info.gdbarch = expr->gdbarch; > + eval_info.frame = NULL; > + eval_info.bitness = probe->parsed_args->arg[n].bitness; > + /* We are compiling to an agent expression. */ > + eval_info.aexpr = expr; > + eval_info.avalue = value; > + > + /* We can always use this kind. */ > + value->kind = axs_rvalue; > + > + /* Figuring out the correct type for this axs_value. */ > + switch (eval_info.bitness) > + { > + case STAP_ARG_BITNESS_UNDEFINED: > + if (gdbarch_addr_bit (gdbarch) == 32) > + value->type = builtin_type (gdbarch)->builtin_uint32; > + else > + value->type = builtin_type (gdbarch)->builtin_uint64; > + break; Do we have to consider the case when gdbarch_addr_bit returns 16? > +static void > +compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr, > + struct axs_value *value, void *data) > +{ > + CORE_ADDR pc = expr->scope; > + int sel = (int) (uintptr_t) data; > + struct objfile *objfile; > + const struct stap_probe *pc_probe; > + int n_probes; > + > + /* SEL==10 means "_probe_argc". */ > + gdb_assert (sel >= 0 && sel <= 10); `10' can be replaced by STAP_MAX_ARGS, as you fixed in compute_probe_arg. > + > + pc_probe = find_probe_by_pc (pc, &objfile); > + if (pc_probe == NULL) > + error (_("No SystemTap probe at PC %s"), core_addr_to_string (pc)); > + > + n_probes > + = objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile, > + pc_probe); > + if (sel == 10) > + { > + value->kind = axs_rvalue; > + value->type = builtin_type (expr->gdbarch)->builtin_int; > + ax_const_l (expr, n_probes); > + return; > + } > + > + gdb_assert (sel >= 0); It is a redundant check. The whole patch looks pretty nice to me, and I have no other comments. -- Yao (齐尧)