From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30453 invoked by alias); 16 Mar 2012 15:50:54 -0000 Received: (qmail 30441 invoked by uid 22791); 16 Mar 2012 15:50:51 -0000 X-SWARE-Spam-Status: No, hits=-0.6 required=5.0 tests=AWL,BAYES_50,TW_QT,TW_TD,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO glazunov.sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 16 Mar 2012 15:50:36 +0000 Received: from glazunov.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3) with ESMTP id q2GFoSJU011640; Fri, 16 Mar 2012 16:50:28 +0100 (CET) Received: (from kettenis@localhost) by glazunov.sibelius.xs4all.nl (8.14.5/8.14.3/Submit) id q2GFoQhS029855; Fri, 16 Mar 2012 16:50:26 +0100 (CET) Date: Fri, 16 Mar 2012 15:50:00 -0000 Message-Id: <201203161550.q2GFoQhS029855@glazunov.sibelius.xs4all.nl> From: Mark Kettenis To: yao@codesourcery.com CC: gdb-patches@sourceware.org In-reply-to: <1331905618-2631-1-git-send-email-yao@codesourcery.com> (message from Yao Qi on Fri, 16 Mar 2012 21:46:58 +0800) Subject: Re: [PATCH] Use sized types in tracepoint. References: <1331905618-2631-1-git-send-email-yao@codesourcery.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: 2012-03/txt/msg00620.txt.bz2 > From: Yao Qi > Date: Fri, 16 Mar 2012 21:46:58 +0800 > > Hi, When discussing on the patches on IPA protocol, I start to > examine the size of each field of tracepoint when copying them to > `command buffer'. In the protocol, each field has its own size, > independent of other factors, such as machine type (32bit vs 64bit), > etc. I started to think about using 'sized' types in these fields, > because it is safe to copy data from one process to the other one. > This is what this patch does. > > These sized types were introduced in C99, and we are using C90 in > GDB, IIRC. Not sure using them in GDB is accpetable. We have gnulib/stdint.h so using typedefs like int32_t should be ok. But I don't think we have support for the PRIxxx macros, so the changes to the printing code aren't ok I'm afraid. > Regression tested on x86_64-linux. OK? > > gdb/gdbserver: > > 2012-03-16 Yao Qi > > * tracepoint.c: Include inttypes.h. > (struct collect_memory_action): Use sized types. > (struct tracepoint): Likewise. > (cmd_qtdp, stop_tracing): Update print specifiers. > (cmd_qtp, response_tracepoint): Likewise. > (collect_data_at_tracepoint): Likewise. > (collect_data_at_step): Likewise. > --- > gdb/gdbserver/tracepoint.c | 32 +++++++++++++++++--------------- > 1 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c > index 2144f6f..3662626 100644 > --- a/gdb/gdbserver/tracepoint.c > +++ b/gdb/gdbserver/tracepoint.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #if HAVE_STDINT_H > #include > #endif > @@ -504,7 +505,7 @@ struct collect_memory_action > > ULONGEST addr; > ULONGEST len; > - int basereg; > + int32_t basereg; > }; > > /* An 'R' (collect registers) action. */ > @@ -724,7 +725,7 @@ struct tracepoint > { > /* The number of the tracepoint, as specified by GDB. Several > tracepoint objects here may share a number. */ > - int number; > + uint32_t number; > > /* Address at which the tracepoint is supposed to trigger. Several > tracepoints may share an address. */ > @@ -734,30 +735,30 @@ struct tracepoint > enum tracepoint_type type; > > /* True if the tracepoint is currently enabled. */ > - int enabled; > + int8_t enabled; > > /* The number of single steps that will be performed after each > tracepoint hit. */ > - long step_count; > + uint64_t step_count; > > /* The number of times the tracepoint may be hit before it will > terminate the entire tracing run. */ > - long pass_count; > + uint64_t pass_count; > > /* Pointer to the agent expression that is the tracepoint's > conditional, or NULL if the tracepoint is unconditional. */ > struct agent_expr *cond; > > /* The list of actions to take when the tracepoint triggers. */ > - int numactions; > + uint32_t numactions; > struct tracepoint_action **actions; > > /* Count of the times we've hit this tracepoint during the run. > Note that while-stepping steps are not counted as "hits". */ > - long hit_count; > + uint64_t hit_count; > > /* Cached sum of the sizes of traceframes created by this point. */ > - long traceframe_usage; > + uint64_t traceframe_usage; > > CORE_ADDR compiled_cond; > > @@ -777,7 +778,7 @@ struct tracepoint > /* The number of bytes displaced by fast tracepoints. It may subsume > multiple instructions, for multi-byte fast tracepoints. This > field is only valid for fast tracepoints. */ > - int orig_size; > + uint32_t orig_size; > > /* Only for fast tracepoints. */ > CORE_ADDR obj_addr_on_target; > @@ -2555,7 +2556,7 @@ cmd_qtdp (char *own_buf) > } > > trace_debug ("Defined %stracepoint %d at 0x%s, " > - "enabled %d step %ld pass %ld", > + "enabled %d step %" PRIu64 " pass %" PRIu64, > tpoint->type == fast_tracepoint ? "fast " > : tpoint->type == static_tracepoint ? "static " : "", > tpoint->number, paddress (tpoint->address), tpoint->enabled, > @@ -3404,7 +3405,7 @@ stop_tracing (void) > if (stopping_tracepoint) > { > trace_debug ("Stopping the trace because " > - "tracepoint %d was hit %ld times", > + "tracepoint %d was hit %" PRIu64 " times", > stopping_tracepoint->number, > stopping_tracepoint->pass_count); > tracing_stop_reason = "tpasscount"; > @@ -3682,7 +3683,8 @@ cmd_qtp (char *own_buf) > return; > } > > - sprintf (own_buf, "V%lx:%lx", tpoint->hit_count, tpoint->traceframe_usage); > + sprintf (own_buf, "V%" PRIu64 ":%" PRIu64 "", tpoint->hit_count, > + tpoint->traceframe_usage); > } > > /* State variables to help return all the tracepoint bits. */ > @@ -3700,7 +3702,7 @@ response_tracepoint (char *packet, struct tracepoint *tpoint) > { > char *buf; > > - sprintf (packet, "T%x:%s:%c:%lx:%lx", tpoint->number, > + sprintf (packet, "T%x:%s:%c:%" PRIx64 ":%" PRIx64, tpoint->number, > paddress (tpoint->address), > (tpoint->enabled ? 'E' : 'D'), tpoint->step_count, > tpoint->pass_count); > @@ -4537,7 +4539,7 @@ collect_data_at_tracepoint (struct tracepoint_hit_ctx *ctx, CORE_ADDR stop_pc, > && stopping_tracepoint == NULL) > stopping_tracepoint = tpoint; > > - trace_debug ("Making new traceframe for tracepoint %d at 0x%s, hit %ld", > + trace_debug ("Making new traceframe for tracepoint %d at 0x%s, hit %" PRIu64, > tpoint->number, paddress (tpoint->address), tpoint->hit_count); > > tframe = add_traceframe (tpoint); > @@ -4574,7 +4576,7 @@ collect_data_at_step (struct tracepoint_hit_ctx *ctx, > int acti; > > trace_debug ("Making new step traceframe for " > - "tracepoint %d at 0x%s, step %d of %ld, hit %ld", > + "tracepoint %d at 0x%s, step %d of %" PRIu64 ", hit %" PRIu64, > tpoint->number, paddress (tpoint->address), > current_step, tpoint->step_count, > tpoint->hit_count); > -- > 1.7.0.4 > >