Re: [PATCH] Add CTF support to GDB [1/4] Add "-ctf" to tsave command

[Hui Zhu <teawater@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 22500 bytes --]

The old patch have some build issues with upstream.  So I post a new
version to handle it.

Thanks,
Hui

2013-02-19  Hui Zhu  <hui_zhu@mentor.com>

	* Makefile.in (REMOTE_OBS): Add ctf.o.
	(SFILES): Add ctf.c.
	(HFILES_NO_SRCDIR): Add ctf.h.
	* ctf.c, ctf.h: New files.
	* breakpoint.c (tracepoint_count): Remove static.
	* mi/mi-main.c (ctf.h): New include.
	(mi_cmd_trace_save): Add "-ctf".
	* tracepoint.c (ctf.h): New include.
	(while_stepping_pseudocommand,
	collect_pseudocommand): Remove static.
	(trace_save_command): Add "-ctf".
	(_initialize_tracepoint): Ditto.
	* tracepoint.h (stack.h): New include.
	(while_stepping_pseudocommand,
	collect_pseudocommand): Add extern.

On Mon, Feb 11, 2013 at 8:53 PM, Hui Zhu <teawater@gmail.com> wrote:
> On Tue, Feb 5, 2013 at 6:51 AM, Hui Zhu <teawater@gmail.com> wrote:
>> On Mon, Feb 4, 2013 at 11:33 PM, Abid, Hafiz <Hafiz_Abid@mentor.com> wrote:
>>> Hi Hui,
>>> I tested the latest patch. I get some build error due to uninitialized local variables.
>>> ../../gdb/gdb/ctf.c: In function ‘ctf_save_collect_get_1’:
>>> ../../gdb/gdb/ctf.c:636:21: error: ‘type’ may be used uninitialised in this function [-Werror=uninitialized]
>>> ../../gdb/gdb/ctf.c: In function ‘ctf_save_collect_get’:
>>> ../../gdb/gdb/ctf.c:734:28: error: ‘pc’ may be used uninitialised in this function [-Werror=uninitialized]
>>> ../../gdb/gdb/ctf.c: In function ‘ctf_save_tp_find’:
>>> ../../gdb/gdb/ctf.c:823:7: error: ‘pc’ may be used uninitialised in this function [-Werror=uninitialized]
>>> ../../gdb/gdb/ctf.c: In function ‘ctf_save’:
>>> ../../gdb/gdb/ctf.c:1323:33: error: ‘content’ may be used uninitialised in this function [-Werror=uninitialized]
>>> ../../gdb/gdb/ctf.c:1307:56: error: ‘val’ may be used uninitialised in this function [-Werror=uninitialized]
>>>
>>> After fixing that, I can see that array and while-stepping are working OK. As I understand, bitfields are not yet supported in babeltrace. So that takes care of most of the issues I reported.
>>>
>>> Regards,
>>> Abid
>>
>>
>> Hi Abid,
>>
>> Thanks for your help.  I just post a new version that fixed these issues.
>>
>> Best,
>> Hui
>
> Hi,
>
> Ping.
>
> Thanks,
> Hui
>
>>
>> 2013-02-05  Hui Zhu  <hui_zhu@mentor.com>
>>
>>         * Makefile.in (REMOTE_OBS): Add ctf.o.
>>         (SFILES): Add ctf.c.
>>         (HFILES_NO_SRCDIR): Add ctf.h.
>>         * ctf.c, ctf.h: New files.
>>         * breakpoint.c (tracepoint_count): Remove static.
>>         * mi/mi-main.c (ctf.h): New include.
>>         (mi_cmd_trace_save): Add "-ctf".
>>         * tracepoint.c (ctf.h): New include.
>>         (collect_pseudocommand): Remove static.
>>         (trace_save_command): Add "-ctf".
>>         (_initialize_tracepoint): Ditto.
>>         * tracepoint.h (stack.h): New include.
>>         (collect_pseudocommand): Add extern.
>>
>>> ________________________________________
>>> From: Hui Zhu [teawater@gmail.com]
>>> Sent: Wednesday, January 23, 2013 1:32 PM
>>> To: Abid, Hafiz
>>> Cc: Tom Tromey; Zhu, Hui; gdb-patches@sourceware.org
>>> Subject: Re: [PATCH] Add CTF support to GDB [1/4] Add "-ctf" to tsave command
>>>
>>> Hi Abid,
>>>
>>> I post a new version according to your comments.
>>>
>>> Following part have the reply for your comments.
>>>
>>> Thanks,
>>> Hui
>>>
>>> 2013-01-23  Hui Zhu  <hui_zhu@mentor.com>
>>>
>>>         * Makefile.in (REMOTE_OBS): Add ctf.o.
>>>         (SFILES): Add ctf.c.
>>>         (HFILES_NO_SRCDIR): Add ctf.h.
>>>         * ctf.c, ctf.h: New files.
>>>         * breakpoint.c (tracepoint_count): Remove static.
>>>         * mi/mi-main.c (ctf.h): New include.
>>>         (mi_cmd_trace_save): Add "-ctf".
>>>         * tracepoint.c (ctf.h): New include.
>>>         (collect_pseudocommand): Remove static.
>>>         (trace_save_command): Add "-ctf".
>>>         (_initialize_tracepoint): Ditto.
>>>         * tracepoint.h (stack.h): New include.
>>>         (collect_pseudocommand): Add extern.
>>>
>>> On Fri, Jan 18, 2013 at 10:29 PM, Hafiz Abid Qadeer
>>> <hafiz_abid@mentor.com> wrote:
>>>> On 18/01/13 01:16:24, Hui Zhu wrote:
>>>>>
>>>>> Hi Abid,
>>>>>
>>>>> Thanks for your review.
>>>>>
>>>>> On Mon, Jan 14, 2013 at 10:27 PM, Abid, Hafiz <Hafiz_Abid@mentor.com>
>>>>> wrote:
>>>>> > Hi Hui,
>>>>> > I tested your patch and found a few problems. I used 'tsave -ctf output'
>>>>> > and then used babeltrace to get a text dump of the output.
>>>>> >
>>>>> > 1. In case of array, the tracing results are off by one.
>>>>> > 2. Struct members values are not shown correctly in case of bitfields.
>>>>>
>>>>> Could you give me some example about this 2 issues?
>>>>> And I just fixed some type issue with while-stepping.  I think maybe
>>>>> they were fixed in the new patch.
>>>>>
>>>> I made an array of size 5 and gave it elements values from 5 to 9. I
>>>> collected this array in trace. After trace was finished, GDB will show
>>>> correct values of all the array elements. But in babeltrace, the first
>>>> element would have value of 6 and last will have a garbage value. So it
>>>> looked that values are off by one index.
>>>>
>>>> For bitfield, I had a structure like this and I observed that value of b was
>>>> not correct in babeltrace.
>>>> struct test_main
>>>> {
>>>>         int a;
>>>>         int b: 16;
>>>>         int c: 16;
>>>> };
>>>>
>>>> I will send you my test application offline.
>>>
>>> Thanks.  This issue is because old patch doesn't support bitfields.  I
>>> add them in the new patch.  But babeltrace doesn't support gcc
>>> bitfields.  So I didn't update test for bitfields.
>>>
>>>>
>>>>
>>>>> > 3. When I use while-stepping on tracepoints actions, I see some error in
>>>>> > the babeltrace.
>>>>>
>>>>> Fixed.  And I think it is a good idea for test.  So I updated test for
>>>>> this issue.
>>>>>
>>>>> > 4. It looks that TYPE_CODE_FLT is not supported which cause the
>>>>> > following warning when I use collect $reg on the tracepoint actions.
>>>>> > "warning: error saving tracepoint 2 "$st0" to CTF file: type is not
>>>>> > support."
>>>>>
>>>>> Yes.  current patch is still not support all the type of GDB.
>>>>>
>>>>> >
>>>>> > Below are some comments on the code. I see many tab characters in the
>>>>> > patch. It may be problem in my editor but something to keep an eye on.
>>>>> >
>>>>> >>+#define CTF_PACKET_SIZE               4096
>>>>> > It may be my ignorance but is this size sufficient? Should it be
>>>>> > possible to increase the limit using some command?
>>>>>
>>>>> Yes, add a command to change current ctf_packet_size is a good idea.
>>>>> Do you mind I add it after CTF patch get commit?  Then we can keep
>>>>> focus on the current function of CTF patch.
>>>>
>>>> I dont have any problem with fixed size. I was just giving an idea that you
>>>> may want to implement in future.
>>>>
>>>>
>>>>>
>>>>> >
>>>>> >>+  /* This is the content size of current packet.  */
>>>>> >>+  size_t content_size;
>>>>> > ...
>>>>> >>+  /* This is the content size of current packet and event that is
>>>>> >>+     being written to file.
>>>>> >>+     Check size use it.  */
>>>>> >>+  size_t current_content_size;
>>>>> > I don't fully understand the difference between these 2 variables.
>>>>> > Probably they need a more helpful comment.
>>>>> >
>>>>>
>>>>> I update it to:
>>>>>   /* This is the temp value of CONTENT_SIZE when GDB write a event to
>>>>>      CTF file.
>>>>>      If this event save success, CURRENT_CONTENT_SIZE will set to
>>>>>      CONTENT_SIZE.  */
>>>>>   size_t current_content_size;
>>>>>
>>>>> >> +error saving tracepoint %d \"%s\" to CTF file: type is not support."),
>>>>> > 'supported' instead of 'support'.
>>>>>
>>>>> Fixed.
>>>>>
>>>>> >
>>>>> >>+            sprintf (regname, "$%s", name);
>>>>> >>+  sprintf (file_name, "%s/%s", dirname, CTF_METADATA_NAME);
>>>>> >>+  sprintf (file_name, "%s/%s", dirname, CTF_DATASTREAM_NAME);
>>>>> > Please use xsnprintf. There are also a bunch of snprintf calls in this
>>>>> > file.
>>>>>
>>>>> The size of file_name is alloca as the right size for both this
>>>>> string.  So I think this part doesn't need xsnprintf.
>>>>>   file_name = alloca (strlen (dirname) + 1
>>>>>                       + strlen (CTF_DATASTREAM_NAME) + 1);
>>>>> >
>>>>> >>+                  case '$':
>>>>> >>+                    collect->ctf_str
>>>>> >>+                        = ctf_save_metadata_change_char
>>>>> >> (collect->ctf_str,
>>>>> >>+                                                         i, "dollar");
>>>>> > This will change expression like $eip in gdb to dollar_eip in ctf. Does
>>>>> > CTF forbid these characters?
>>>>>
>>>>> No.
>>>>
>>>> In that case, the question will be why we do this change from $eip to
>>>> dollar_eip.
>>>
>>> Oops,  sorry for my mistake.  CTF doesn't support this char like $ or
>>> something else.
>>>
>>>>
>>>>
>>>>>
>>>>> >
>>>>> >>+static void
>>>>> >>+tsv_save_do_loc_arg_collect (const char *print_name,
>>>>> >>+                           struct symbol *sym,
>>>>> >>+                           void *cb_data)
>>>>> >>+{
>>>>> >>+  struct loc_arg_collect_data *p = cb_data;
>>>>> >>+  char *name;
>>>>> >>+
>>>>> >>+  name = alloca (strlen (print_name) + 1);
>>>>> >>+  strcpy (name, print_name);
>>>>> >>+  ctf_save_collect_get_1 (p->tcsp, p->tps, name);
>>>>> >>+}
>>>>> > Is there any real need to make a copy of the print_name? I think it can
>>>>> > be passed directly to the ctf_save_collect_get_1.
>>>>>
>>>>> This is because print_name is a const but ctf_save_collect_get_1's
>>>>> argument name need to be a string that is not a const.
>>>>> Added comments for that.
>>>>
>>>> You probably would have done a cast or perhaps ctf_save_collect_get_1's
>>>> argument can be changed to const.
>>>>
>>>
>>> Fixed.
>>>
>>>>
>>>>>
>>>>> >
>>>>> >>+ tmp = alloca (strlen (collect->ctf_str) + 30);
>>>>> >>+        strcpy (tmp, collect->ctf_str);
>>>>> >>+        while (1)
>>>>> >>+          {
>>>>> >>+            struct ctf_save_collect_s *collect2;
>>>>> >>+            int i = 0;
>>>>> >>+
>>>>> >>+            for (collect2 = tps->collect; collect2;
>>>>> >>+                 collect2 = collect2->next)
>>>>> >>+              {
>>>>> >>+                if (collect2->ctf_str
>>>>> >>+                    && strcmp (collect2->ctf_str, tmp) == 0)
>>>>> >>+                  break;
>>>>> >>+              }
>>>>> >>+            if (collect2 == NULL)
>>>>> >>+              break;
>>>>> >>+
>>>>> >>+            snprintf (tmp, strlen (collect->ctf_str) + 30,
>>>>> >>+                      "%s_%d", collect->ctf_str, i++);
>>>>> >>+          }
>>>>> > What is the purpose of this loop? It only writes a new string in the tmp
>>>>> > local variable which is not used after the loop.
>>>>>
>>>>> Fixed.
>>>>>
>>>>> >
>>>>> >>+\"%s\" of tracepoint %d rename to \"%s\" in CTF file."),
>>>>> > I think 'is renamed' will be better instead of rename here.
>>>>>
>>>>> Fixed.
>>>>>
>>>>> >
>>>>> >>+        if (try_count > 1 || 4 + 4 + 4 == tcs.content_size)
>>>>> > what is the significance of this 4 + 4 + 4
>>>>>
>>>>> Change it to CONTENT_HEADER_SIZE
>>>>>
>>>>> >
>>>>> >>+traceframe %d of tracepoint %d need save data that bigger than packet
>>>>> >> size %d.\n\
>>>>> > should be "needs to save data that is bigger than the packet size"
>>>>>
>>>>> Fixed.
>>>>>
>>>>> >
>>>>> >>+traceframe %d is dropped because try to get the value of \"%s\" got
>>>>> >> error: %s"),
>>>>> > This probably needs to re-phrased.
>>>>>
>>>>> Fixed.
>>>>>
>>>>> >
>>>>> > Also many comments can be improved grammatically. This will make them
>>>>> > easier to understand. Please let me know if I need any help there.
>>>>> >
>>>>> > Thanks,
>>>>> > Abid
>>>>>
>>>>> Post a new version according to your comments.
>>>>>
>>>>> Thanks,
>>>>> Hui
>>>>>
>>>>> 2013-01-18  Hui Zhu  <hui_zhu@mentor.com>
>>>>>
>>>>>         * Makefile.in (REMOTE_OBS): Add ctf.o.
>>>>>         (SFILES): Add ctf.c.
>>>>>         (HFILES_NO_SRCDIR): Add ctf.h.
>>>>>         * ctf.c, ctf.h: New files.
>>>>>         * breakpoint.c (tracepoint_count): Remove static.
>>>>>         * mi/mi-main.c (ctf.h): New include.
>>>>>         (mi_cmd_trace_save): Add "-ctf".
>>>>>         * tracepoint.c (ctf.h): New include.
>>>>>         (collect_pseudocommand): Remove static.
>>>>>         (trace_save_command): Add "-ctf".
>>>>>         (_initialize_tracepoint): Ditto.
>>>>>         * tracepoint.h (stack.h): New include.
>>>>>         (collect_pseudocommand): Add extern.
>>>>>
>>>>> >
>>>>> > ________________________________________
>>>>> > From: gdb-patches-owner@sourceware.org
>>>>> > [gdb-patches-owner@sourceware.org] on behalf of Hui Zhu [teawater@gmail.com]
>>>>> > Sent: Monday, January 14, 2013 5:18 AM
>>>>> > To: Tom Tromey
>>>>> > Cc: Zhu, Hui; gdb-patches@sourceware.org
>>>>> > Subject: Re: [PATCH] Add CTF support to GDB [1/4] Add "-ctf" to tsave
>>>>> > command
>>>>> >
>>>>> > Hi Tom,
>>>>> >
>>>>> > I found a bug when I use test to test this patch.
>>>>> > So I post a new version to fix this bug.
>>>>> > The change of this patch is change the same type check to:
>>>>> > static void
>>>>> > ctf_save_type_define_write (struct ctf_save_s *tcsp, struct type *type)
>>>>> > {
>>>>> >   struct ctf_save_type_s *t;
>>>>> >
>>>>> >   for (t = tcsp->type; t; t = t->next)
>>>>> >     {
>>>>> >       if (t->type == type
>>>>> >           || (TYPE_NAME (t->type) && TYPE_NAME (type)
>>>>> >               && strcmp (TYPE_NAME (t->type), TYPE_NAME (type)) == 0))
>>>>> >         return;
>>>>> >     }
>>>>> >
>>>>> > Thanks,
>>>>> > Hui
>>>>> >
>>>>> > On Tue, Jan 8, 2013 at 9:40 AM, Hui Zhu <teawater@gmail.com> wrote:
>>>>> >> Hi Tom,
>>>>> >>
>>>>> >> Thanks for your review.
>>>>> >>
>>>>> >> On Fri, Jan 4, 2013 at 5:36 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>> >>>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>>>>> >>>
>>>>> >>> Hui> +struct ctf_save_collect_s
>>>>> >>> Hui> +{
>>>>> >>> Hui> +  struct ctf_save_collect_s *next;
>>>>> >>> Hui> +  char *str;
>>>>> >>> Hui> +  char *ctf_str;
>>>>> >>> Hui> +  int align_size;
>>>>> >>> Hui> +  struct expression *expr;
>>>>> >>> Hui> +  struct type *type;
>>>>> >>> Hui> +  int is_ret;
>>>>> >>> Hui> +};
>>>>> >>>
>>>>> >>>>> Like Hafiz said -- comments would be nice.
>>>>> >>>
>>>>> >>> Hui> I added some comments in the new patches.
>>>>> >>>
>>>>> >>> I looked at the new patches and did not see comments.  For example, I
>>>>> >>> looked at this struct I quoted above.
>>>>> >>>
>>>>> >>> Every new structure, field, and function ought to have a comment.
>>>>> >>
>>>>> >> OK.  I added comments for them in the new patch.
>>>>> >>
>>>>> >>>
>>>>> >>>
>>>>> >>> Hui> +    case TYPE_CODE_ARRAY:
>>>>> >>> Hui> +      for (; TYPE_CODE (type) == TYPE_CODE_ARRAY;
>>>>> >>> Hui> +     type = TYPE_TARGET_TYPE (type))
>>>>> >>> Hui> +  ;
>>>>> >>>
>>>>> >>> Tom> You probably want some check_typedef calls in there.
>>>>> >>>
>>>>> >>> Hui> Because typedef will be handle as a type in this part, so this
>>>>> >>> part
>>>>> >>> Hui> doesn't need check_typedef.
>>>>> >>>
>>>>> >>> That seems peculiar to me, but I don't really know CTF.
>>>>> >>> In this case you need a comment, since the result will be non-obvious
>>>>> >>> to
>>>>> >>> gdb developers.
>>>>> >>>
>>>>> >>> Tom> check_typedef; though if your intent is to peel just a single
>>>>> >>> layer,
>>>>> >>> Tom> then it is a bit trickier -- I think the best you can do is
>>>>> >>> always call
>>>>> >>> Tom> it, then use TYPE_TARGET_TYPE if it is non-NULL or the result of
>>>>> >>> Tom> check_typedef otherwise.
>>>>> >>>
>>>>> >>> Hui> If use check_typedef, this part will generate the define that
>>>>> >>> Hui> different with the type descriptor of the code.
>>>>> >>>
>>>>> >>> You need to call check_typedef before you can even examine
>>>>> >>> TYPE_TARGET_TYPE of a typedef.  This is what I meant by using it
>>>>> >>> before
>>>>> >>> using TYPE_TARGET_TYPE.  Otherwise with stubs I think you will see
>>>>> >>> crashes -- check_typedef is what sets this field.
>>>>> >>>
>>>>> >>> If you then use TYPE_TARGET_TYPE and get NULL, you ought to instead
>>>>> >>> use
>>>>> >>> the result of check_typedef.  This means the stub had to resolve to a
>>>>> >>> typedef in a different objfile.
>>>>> >>
>>>>> >> I change it to following part:
>>>>> >>     case TYPE_CODE_ARRAY:
>>>>> >>       /* This part just to get the real name of this array.
>>>>> >>          This part should keep typedef if it can.  */
>>>>> >>       for (; TYPE_CODE (type) == TYPE_CODE_ARRAY;
>>>>> >>            type = TYPE_TARGET_TYPE (type) ? TYPE_TARGET_TYPE (type)
>>>>> >>                                           : check_typedef (type))
>>>>> >>         ;
>>>>> >>
>>>>> >>>
>>>>> >>> Hui> If use TYPE_TARGET_TYPE, it will generate following metadata:
>>>>> >>> Hui> typedef char test_t1;
>>>>> >>> Hui> typedef test_t1 test_t2;
>>>>> >>> Hui> typedef test_t2 test_t3;
>>>>> >>>
>>>>> >>> I suppose there should be a test case doing this.
>>>>> >>
>>>>> >> OK. I will write a test for all this function.
>>>>> >>
>>>>> >>>
>>>>> >>> Hui> +      case TYPE_CODE_PTR:
>>>>> >>> Hui> +  align_size = TYPE_LENGTH (type);
>>>>> >>> Hui> +  break;
>>>>> >>>
>>>>> >>> Tom> Surely the alignment rules are ABI dependent.
>>>>> >>> Tom> I would guess that what you have will work in many cases, but
>>>>> >>> definitely
>>>>> >>> Tom> not all of them.
>>>>> >>>
>>>>> >>> Hui> All the type will be handle and record in function
>>>>> >>> Hui> ctf_save_type_check_and_write.
>>>>> >>> Hui> The size align will be handle in this function too.
>>>>> >>>
>>>>> >>> I don't think this really addresses the issue.
>>>>> >>> Not all platforms use the alignment rules currently coded in
>>>>> >>> ctf_save_type_check_and_write.  But maybe it doesn't matter.
>>>>> >>>
>>>>> >>> Hui> +    frame = get_current_frame ();
>>>>> >>> Hui> +    if (!frame)
>>>>> >>> Hui> +      error (_("get current frame fail"));
>>>>> >>> Hui> +    frame = get_prev_frame (frame);
>>>>> >>> Hui> +    if (!frame)
>>>>> >>> Hui> +      error (_("get prev frame fail"));
>>>>> >>> Tom>
>>>>> >>> Tom> These messages could be improved.
>>>>> >>>
>>>>> >>> Actually, I don't think get_current_frame can return NULL, can it?
>>>>> >>>
>>>>> >>> For the second error, how about "could not find previous frame"?
>>>>> >>
>>>>> >> Fixed.
>>>>> >>
>>>>> >>>
>>>>> >>> Hui> +      warning (_("\
>>>>> >>> Hui> +Not save \"%s\" of tracepoint %d to ctf file because get its
>>>>> >>> Hui> value fail: %s"),
>>>>> >>> Hui> +         str, tps->tp->base.number, e.message);
>>>>> >>> Tom>
>>>>> >>> Tom> Likewise.
>>>>> >>>
>>>>> >>> Hui> Could you help me with this part?  :)
>>>>> >>>
>>>>> >>> How about "error saving tracepoint %d to CTF file %s: %s".
>>>>> >>
>>>>> >> It is more better.  I updated them all.
>>>>> >>
>>>>> >>>
>>>>> >>> Tom> Although, this approach just seems weird, since it seems like you
>>>>> >>> Tom> already have the symbol and you want its value; constructing and
>>>>> >>> parsing
>>>>> >>> Tom> an expression to get this is very roundabout.
>>>>> >>> Tom>
>>>>> >>> Tom> I'm not sure I really understand the goal here; but the parsing
>>>>> >>> approach
>>>>> >>> Tom> is particularly fragile if you have shadowing.
>>>>> >>>
>>>>> >>> Hui> Function ctf_save_collect_get will parse the collect string and
>>>>> >>> add
>>>>> >>> Hui> them to struct.
>>>>> >>> Hui> Each tracepoint will call this function just once.
>>>>> >>>
>>>>> >>> Ok, I don't know the answer here.
>>>>> >>
>>>>> >> I am sorry that this part is not very clear.  So I update the comments
>>>>> >> of ctf_save_collect_get to:
>>>>> >> /* Get var that want to collect from STR and put them to TPS->collect.
>>>>> >>    This function will not be call when GDB add a new TP.  */
>>>>> >>
>>>>> >> static void
>>>>> >> ctf_save_collect_get (struct ctf_save_s *tcsp, struct ctf_save_tp_s
>>>>> >> *tps,
>>>>> >>                       char *str)
>>>>> >>
>>>>> >> How about this?
>>>>> >>
>>>>> >>>
>>>>> >>> Tom> Hmm, a lot of this code looks like code from tracepoint.c.
>>>>> >>> Tom> I think it would be better to share the code if that is possible.
>>>>> >>>
>>>>> >>> Hui> I tried to share code with function add_local_symbols.  But it is
>>>>> >>> not
>>>>> >>> Hui> a big function and use different way to get block.
>>>>> >>>
>>>>> >>> I wonder why, and whether this means that the different ways of saving
>>>>> >>> will in fact write out different data.
>>>>> >>
>>>>> >> I added function add_local_symbols_1 for that.
>>>>> >>
>>>>> >>>
>>>>> >>> Hui> +    if (collect->expr)
>>>>> >>> Hui> +      free_current_contents (&collect->expr);
>>>>> >>>
>>>>> >>> Tom> Why free_current_contents here?
>>>>> >>> Tom> That seems weird.
>>>>> >>>
>>>>> >>> Hui> If this collect is $_ret, it will not have collect->expr.  Or
>>>>> >>> maybe
>>>>> >>> Hui> this collect will be free because when setup this collect get
>>>>> >>> Hui> error.  So check it before free it.
>>>>> >>>
>>>>> >>> You can just write  xfree (collect->expr).
>>>>> >>> You don't need a NULL check here.
>>>>> >>> This applies to all those xfree calls.
>>>>> >>>
>>>>> >>
>>>>> >> OK.  Fixed.
>>>>> >>
>>>>> >> I post a new version.  Please help me review it.
>>>>> >>
>>>>> >> Thanks,
>>>>> >> Hui
>>>>> >>
>>>>> >> 2013-01-08  Hui Zhu  <hui_zhu@mentor.com>
>>>>> >>
>>>>> >>         * Makefile.in (REMOTE_OBS): Add ctf.o.
>>>>> >>         (SFILES): Add ctf.c.
>>>>> >>         (HFILES_NO_SRCDIR): Add ctf.h.
>>>>> >>         * ctf.c, ctf.h: New files.
>>>>> >>         * mi/mi-main.c (ctf.h): New include.
>>>>> >>         (mi_cmd_trace_save): Add "-ctf".
>>>>> >>         * tracepoint.c (ctf.h): New include.
>>>>> >>         (collect_pseudocommand): Remove static.
>>>>> >>         (trace_save_command): Add "-ctf".
>>>>> >>         (_initialize_tracepoint): Ditto.
>>>>> >>         * tracepoint.h (stack.h): New include.
>>>>> >>         (collect_pseudocommand): Add extern.
>>>>>
>>>>

[-- Attachment #2: tsave-ctf.txt --]
[-- Type: text/plain, Size: 43595 bytes --]

--- a/Makefile.in
+++ b/Makefile.in
@@ -510,7 +510,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o
+	remote-notif.o ctf.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -759,7 +759,7 @@ SFILES = ada-exp.y ada-lang.c ada-typepr
 	regset.c sol-thread.c windows-termcap.c \
 	common/gdb_vecs.c common/common-utils.c common/xml-utils.c \
 	common/ptid.c common/buffer.c gdb-dlfcn.c common/agent.c \
-	common/format.c
+	common/format.c ctf.c
 
 LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 
@@ -835,7 +835,7 @@ gnulib/import/stddef.in.h gnulib/import/
 common/common-utils.h common/xml-utils.h common/buffer.h common/ptid.h \
 common/format.h common/host-defs.h utils.h common/queue.h \
 common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h \
-gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h
+gdb_bfd.h sparc-ravenscar-thread.h ppc-ravenscar-thread.h ctf.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
--- a/breakpoint.c
+++ b/breakpoint.c
@@ -610,7 +610,7 @@ static int prev_breakpoint_count;
 
 /* Number of last tracepoint made.  */
 
-static int tracepoint_count;
+int tracepoint_count;
 
 static struct cmd_list_element *breakpoint_set_cmdlist;
 static struct cmd_list_element *breakpoint_show_cmdlist;
--- /dev/null
+++ b/ctf.c
@@ -0,0 +1,1344 @@
+/* CTF format support.
+
+   Copyright (C) 2012-2013 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 <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "ctf.h"
+#include "tracepoint.h"
+#include "regcache.h"
+#include "gdbcmd.h"
+#include "exceptions.h"
+#include "stack.h"
+
+#include <ctype.h>
+
+/* Following part is for tsave.  */
+
+#define CTF_MAGIC		0xC1FC1FC1
+#define CTF_SAVE_MAJOR		1
+#define CTF_SAVE_MINOR		8
+
+#define CTF_METADATA_NAME	"metadata"
+#define CTF_DATASTREAM_NAME	"datastream"
+
+#define CTF_PACKET_SIZE		4096
+
+#define ALIGN_SIZE(size, align) \
+	((((align) + (size) - 1) & (~((align) - 1))) - (size))
+
+/* The entry of collect list of a TP.  */
+
+struct ctf_save_collect_s
+{
+  struct ctf_save_collect_s *next;
+
+  /* Point the original collect string.  */
+
+  char *str;
+
+  /* Point string the convert from STR to the format
+     that can save to CTF. */
+
+  char *ctf_str;
+
+  /* Align size of this collect.  */
+
+  int align_size;
+
+  /* The expression that get from STR. */
+
+  struct expression *expr;
+
+  /* The type of this collect.  */
+
+  struct type *type;
+
+  /* If true, this collect is $_ret.  */
+
+  int is_ret;
+};
+
+/* The entry of tracepoint list that will save to CTF.  */
+
+struct ctf_save_tp_s
+{
+  struct ctf_save_tp_s *next;
+
+  struct tracepoint *tp;
+
+  /* If true, this is the step collect of this TP.
+     Divide TP to non-step collect and step collect
+     because they collect different value.  */
+
+  int stepping_frame;
+
+  /* This is the id that will save to CTF file.
+     Doesn't use TPS->TP->BASE.NUMBER directly because stepping_frame
+     event need use different with original event.  */
+  int id;
+
+  /* The collect list for this TP.  */
+
+  struct ctf_save_collect_s *collect;
+
+  /* Each traceframe entry of a tracepoint will save as a data format
+     like a struct.
+     This is the align size of this struct.  */
+
+  int align_size;
+
+  /* If true, the size of this struct is variable.  */
+
+  int is_variable_length;
+
+  /* If true, all the traceframe entry of this TP will not save to CTF.
+     Add a flag instead of just remove this struct because GDB just can
+     get tracepoint information through traceframe entry.
+     If just remove this struct, GDB will add a new struct when GDB get
+     another traceframe of this TP.
+     Use a flag, when GDB get another traceframe of this TP, GDB will
+     know this traceframe need to be dropped when it get it through
+     this flag.  */
+
+  int is_dropped;
+};
+
+/* The entry of type list that will save to CTF.  */
+
+struct ctf_save_type_s
+{
+  struct ctf_save_type_s *next;
+
+  struct type *type;
+};
+
+/* The data struct for CTF_SAVE.  */
+
+struct ctf_save_s
+{
+  FILE *metadata_fd;
+  FILE *datastream_fd;
+
+  /* This is the content size of current packet.  */
+  size_t content_size;
+
+  /* This is the begin offset of current packet.  */
+  long packet_begin;
+
+  /* If true, need check if content_size bigger than CTF_PACKET_SIZE.  */
+  int check_size;
+
+  /* This is the temp value of CONTENT_SIZE when GDB write a event to
+     CTF file.
+     If this event save success, CURRENT_CONTENT_SIZE will set to
+     CONTENT_SIZE.  */
+  size_t current_content_size;
+
+  /* Save the number of trace frame before call ctf_save.
+     Set it back when return ctf_save.  */
+  int old_traceframe_num;
+
+  /* Tracepoint list.  */
+  struct ctf_save_tp_s *tp;
+
+  /* Type list.  */
+  struct ctf_save_type_s *type;
+
+  const char *tab;
+};
+
+static void
+ctf_save_fwrite (FILE *fd, const gdb_byte *buf, size_t size)
+{
+  if (fwrite (buf, size, 1, fd) != 1)
+    error (_("Unable to write file for saving trace data (%s)"),
+	   safe_strerror (errno));
+}
+
+static void
+ctf_save_fwrite_format_1 (FILE *fd, const char *format, va_list args)
+{
+  char *linebuffer;
+  struct cleanup *old_cleanups;
+
+  linebuffer = xstrvprintf (format, args);
+  old_cleanups = make_cleanup (xfree, linebuffer);
+  ctf_save_fwrite (fd, linebuffer, strlen (linebuffer));
+  do_cleanups (old_cleanups);
+}
+
+/* Write data in FORMAT to FD.  */
+
+static void
+ctf_save_fwrite_format (FILE *fd, const char *format, ...)
+{
+  va_list args;
+
+  va_start (args, format);
+  ctf_save_fwrite_format_1 (fd, format, args);
+  va_end (args);
+}
+
+/* Write BUF that size is SIZE to datastream file.  */
+
+static int
+ctf_save_write (struct ctf_save_s *tcsp, const gdb_byte *buf, size_t size)
+{
+  if (tcsp->check_size)
+    {
+      if (tcsp->current_content_size + size > CTF_PACKET_SIZE)
+        return -1;
+    }
+
+  ctf_save_fwrite (tcsp->datastream_fd, buf, size);
+
+  tcsp->current_content_size += size;
+
+  return 0;
+}
+
+/* Set datastream file position indicator according to OFFSET
+   and WHENCE.  */
+
+static int
+ctf_save_fseek (struct ctf_save_s *tcsp, long offset, int whence)
+{
+  if (whence == SEEK_CUR && tcsp->check_size)
+    {
+      if (tcsp->current_content_size + offset > CTF_PACKET_SIZE)
+        return -1;
+    }
+
+  if (fseek (tcsp->datastream_fd, offset, whence))
+    error (_("Unable to seek file for saving trace data (%s)"),
+	   safe_strerror (errno));
+
+  if (whence == SEEK_CUR)
+    tcsp->current_content_size += offset;
+
+  return 0;
+}
+
+/* Aligned ALIGN_SIZE write BUF that size is SIZE
+   to datastream file.  */
+
+static int
+ctf_save_align_write (struct ctf_save_s *tcsp, const gdb_byte *buf,
+		      size_t size, size_t align_size)
+{
+  if (ctf_save_fseek (tcsp,
+		      ALIGN_SIZE (tcsp->current_content_size, align_size),
+		      SEEK_CUR))
+    return -1;
+
+  if (ctf_save_write (tcsp, buf, size))
+    return -1;
+
+  return 0;
+}
+
+static void ctf_save_type_define_write (struct ctf_save_s *tcsp,
+					struct type *type, int local);
+
+/* Write the type part of a var define to CTF metadata file.  */
+
+static void
+ctf_save_type_name_write (struct ctf_save_s *tcsp, struct type *type)
+{
+  switch (TYPE_CODE (type))
+    {
+    case TYPE_CODE_ARRAY:
+      /* This part just to get the real name of this array.
+         This part should keep typedef if it can.  */
+      for (; TYPE_CODE (type) == TYPE_CODE_ARRAY;
+	   type = TYPE_TARGET_TYPE (type) ? TYPE_TARGET_TYPE (type)
+					  : check_typedef (type))
+	;
+      if (TYPE_NAME (type))
+        ctf_save_fwrite_format (tcsp->metadata_fd, "%s", TYPE_NAME (type));
+      else
+	ctf_save_type_define_write (tcsp, type, 1);
+      break;
+
+    case TYPE_CODE_PTR:
+      if (TYPE_LENGTH (type) == 8)
+	ctf_save_fwrite_format (tcsp->metadata_fd, "uint64_t");
+      else
+	ctf_save_fwrite_format (tcsp->metadata_fd, "uint32_t");
+      break;
+
+    case TYPE_CODE_STRUCT:
+    case TYPE_CODE_ENUM:
+      if (TYPE_TAG_NAME (type))
+        ctf_save_fwrite_format (tcsp->metadata_fd, "%s %s",
+				TYPE_CODE (type) == TYPE_CODE_STRUCT ?
+				"struct" : "enum",
+				TYPE_TAG_NAME (type));
+      else
+	ctf_save_type_define_write (tcsp, type, 1);
+      break;
+
+    case TYPE_CODE_UNION:
+      {
+	int i, biggest_id, biggest_size = 0;
+
+	for (i = 0; i < TYPE_NFIELDS (type); ++i)
+	  {
+	    if (biggest_size < TYPE_LENGTH (TYPE_FIELD_TYPE (type, i)))
+	      {
+		biggest_size = TYPE_LENGTH (TYPE_FIELD_TYPE (type, i));
+		biggest_id = i;
+	      }
+	  }
+	ctf_save_type_name_write (tcsp, TYPE_FIELD_TYPE (type, biggest_id));
+	break;
+      }
+
+    default:
+      ctf_save_fwrite_format (tcsp->metadata_fd, "%s", TYPE_NAME (type));
+      break;
+    }
+}
+
+/* Write the size part of a var define to CTF metadata file.  */
+
+static void
+ctf_save_type_size_write (struct ctf_save_s *tcsp, struct type *type)
+{
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY)
+    {
+      /* This part just to get the real name of this array.
+         This part should keep typedef if it can.  */
+      for (; TYPE_CODE (type) == TYPE_CODE_ARRAY;
+	   type = TYPE_TARGET_TYPE (type) ? TYPE_TARGET_TYPE (type)
+					  : check_typedef (type))
+	ctf_save_fwrite_format (tcsp->metadata_fd, "[%d]",
+				TYPE_LENGTH (type) /
+				TYPE_LENGTH (TYPE_TARGET_TYPE (type)));
+    }
+}
+
+/* Write a var define to CTF metadata file.  */
+
+static void
+ctf_save_var_define_write (struct ctf_save_s *tcsp, struct type *type,
+			   const char *name, int bitsize)
+{
+  ctf_save_type_name_write (tcsp, type);
+  ctf_save_fwrite_format (tcsp->metadata_fd, " %s", name);
+  if (bitsize)
+    ctf_save_fwrite_format (tcsp->metadata_fd, ":%d", bitsize);
+  ctf_save_type_size_write (tcsp, type);
+  ctf_save_fwrite_format (tcsp->metadata_fd, ";\n");
+}
+
+/* Return true if T1 and T2 is same.  */
+
+static int
+ctf_save_type_is_same (struct type *t1, struct type *t2)
+{
+  const char *name1, *name2;
+
+  if (t1 == t2)
+    return 1;
+
+  if (TYPE_CODE (t1) != TYPE_CODE (t2))
+    return 0;
+
+  if (TYPE_CODE (t1) == TYPE_CODE_STRUCT || TYPE_CODE (t1) == TYPE_CODE_ENUM)
+    {
+      name1 = TYPE_TAG_NAME (t1);
+      name2 = TYPE_TAG_NAME (t2);
+    }
+  else
+    {
+      name1 = TYPE_NAME (t1);
+      name2 = TYPE_NAME (t2);
+    }
+
+  if (name1 && name2 && strcmp (name1, name2) == 0)
+    return 1;
+
+  return 0;
+}
+
+/* Check if TYPE in TCSP->TYPE.
+   If not, write TYPE to TCSP->metadata_fd.
+   If LOCAL is true, this type define just define with a var.  */
+
+static void
+ctf_save_type_define_write (struct ctf_save_s *tcsp, struct type *type,
+			    int local)
+{
+  struct ctf_save_type_s *t;
+
+  /* Ignore type uint32_t and uint64_t because they have written
+     in function ctf_save_metadata_header.  */
+  if (TYPE_NAME (type) && (strcmp (TYPE_NAME (type), "uint32_t") == 0
+			   || strcmp (TYPE_NAME (type), "uint64_t") == 0))
+    return;
+
+  if (!local)
+    {
+      for (t = tcsp->type; t; t = t->next)
+	{
+	  if (ctf_save_type_is_same (type, t->type))
+	    return;
+	}
+
+      t = (struct ctf_save_type_s *) xzalloc (sizeof (*t));
+      t->type = type;
+      t->next = tcsp->type;
+      tcsp->type = t;
+    }
+
+  switch (TYPE_CODE (type))
+    {
+      case TYPE_CODE_TYPEDEF:
+	ctf_save_fwrite_format (tcsp->metadata_fd, "typedef ");
+	ctf_save_var_define_write (tcsp, TYPE_TARGET_TYPE (type),
+				   TYPE_NAME (type), 0);
+	break;
+
+      case TYPE_CODE_INT:
+	ctf_save_fwrite_format (tcsp->metadata_fd, "\
+typealias integer { size = %d; align = %d; signed = %s; } := %s;\n",
+				TYPE_LENGTH (type) * TARGET_CHAR_BIT,
+				TYPE_LENGTH (type) * TARGET_CHAR_BIT,
+				!TYPE_UNSIGNED (type) ? "true" : "false",
+				TYPE_NAME (type));
+	break;
+
+      case TYPE_CODE_STRUCT:
+      case TYPE_CODE_ENUM:
+        {
+	  int i;
+	  char tab[256];
+	  const char *old_tab;
+
+	  ctf_save_fwrite_format (tcsp->metadata_fd, "%s %s {\n",
+				  TYPE_CODE (type) == TYPE_CODE_STRUCT ?
+				  "struct" : "enum",
+				  TYPE_TAG_NAME (type) ? TYPE_TAG_NAME (type)
+						       : "");
+
+	  old_tab = tcsp->tab;
+	  snprintf (tab, 256, "%s\t", old_tab);
+	  tcsp->tab = tab;
+	  for (i = 0; i < TYPE_NFIELDS (type); ++i)
+	    {
+	      if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
+	        {
+		  ctf_save_fwrite_format (tcsp->metadata_fd, tcsp->tab);
+		  ctf_save_var_define_write (tcsp, TYPE_FIELD_TYPE (type, i),
+					     TYPE_FIELD_NAME (type, i),
+					     TYPE_FIELD_BITSIZE (type, i));
+		}
+	      else
+		ctf_save_fwrite_format (tcsp->metadata_fd, "%s%s = %s,\n",
+					tcsp->tab, TYPE_FIELD_NAME (type, i),
+					plongest (TYPE_FIELD_ENUMVAL (type, i)));
+	    }
+	  tcsp->tab = old_tab;
+	  ctf_save_fwrite_format (tcsp->metadata_fd, "%s}", tcsp->tab);
+	  if (TYPE_TAG_NAME (type))
+	    ctf_save_fwrite_format (tcsp->metadata_fd, ";\n");
+	  break;
+	}
+    }
+}
+
+/* Check if this type is supported by GDB.
+   Return the align size.  */
+
+static int
+ctf_save_type_check_and_write (struct ctf_save_s *tcsp, struct type *type)
+{
+  int align_size = 0;
+
+  switch (TYPE_CODE (type))
+    {
+      case TYPE_CODE_TYPEDEF:
+	align_size = ctf_save_type_check_and_write (tcsp,
+						    TYPE_TARGET_TYPE (type));
+	if (align_size < 0)
+	  return align_size;
+	ctf_save_type_define_write (tcsp, type, 0);
+	break;
+
+      case TYPE_CODE_ARRAY:
+	align_size = ctf_save_type_check_and_write (tcsp,
+						    TYPE_TARGET_TYPE (type));
+	if (align_size < 0)
+	  return align_size;
+	break;
+
+      case TYPE_CODE_INT:
+	ctf_save_type_define_write (tcsp, type, 0);
+	align_size = TYPE_LENGTH (type);
+	break;
+
+      case TYPE_CODE_PTR:
+	align_size = TYPE_LENGTH (type);
+	break;
+
+      case TYPE_CODE_STRUCT:
+        {
+	  int i, s_align_size;
+
+	  for (i = 0; i < TYPE_NFIELDS (type); ++i)
+	    {
+	      s_align_size
+		  = ctf_save_type_check_and_write (tcsp,
+						   TYPE_FIELD_TYPE (type, i));
+	      if (s_align_size < 0)
+		return s_align_size;
+
+	      if (align_size < s_align_size)
+		align_size = s_align_size;
+	    }
+	  if (TYPE_TAG_NAME (type))
+	    ctf_save_type_define_write (tcsp, type, 0);
+	  break;
+	}
+
+      case TYPE_CODE_ENUM:
+	align_size = TYPE_LENGTH (type);
+	if (TYPE_TAG_NAME (type))
+	  ctf_save_type_define_write (tcsp, type, 0);
+	break;
+
+      case TYPE_CODE_UNION:
+        {
+	  int i, s_align_size;
+
+	  for (i = 0; i < TYPE_NFIELDS (type); ++i)
+	    {
+	      s_align_size
+		  = ctf_save_type_check_and_write (tcsp,
+						   TYPE_FIELD_TYPE (type, i));
+	      if (s_align_size < 0)
+		return s_align_size;
+
+	      if (align_size < s_align_size)
+		align_size = s_align_size;
+	    }
+	  break;
+	}
+
+      default:
+	align_size = -1;
+	break;
+    }
+
+  return align_size;
+}
+
+static void
+ctf_save_collect_get_1 (struct ctf_save_s *tcsp, struct ctf_save_tp_s *tps,
+			const char *name)
+{
+  struct expression *expr;
+  struct ctf_save_collect_s *collect;
+  volatile struct gdb_exception e;
+  struct type *type = NULL;
+  int is_ret = 0;
+  int align_size;
+  char *str;
+
+  /* Alloca STR because PARSE_EXPRESSION need argument
+     is not a const string.  */
+  str = alloca (strlen (name) + 1);
+  strcpy (str, name);
+
+  /* Check if action_exp is already exist in tps->collect.  */
+  for (collect = tps->collect; collect; collect = collect->next)
+    {
+      if (strcmp (collect->str, str) == 0)
+	return;
+    }
+
+  if (0 == strncasecmp (str, "$_ret", 5))
+    is_ret = 1;
+
+  TRY_CATCH (e, RETURN_MASK_ERROR)
+    {
+      if (is_ret)
+        {
+	  CORE_ADDR pc;
+	  struct frame_info *frame;
+
+	  frame = get_prev_frame (get_current_frame ());
+	  if (!frame)
+	    error (_("could not find previous frame"));
+
+	  if (!get_frame_pc_if_available (frame, &pc))
+	    error (_("PC unavailable"));
+	}
+      else
+        {
+	  struct cleanup *old_chain;
+	  struct value *val;
+
+	  expr = parse_expression (str);
+	  old_chain = make_cleanup (free_current_contents, &expr);
+	  type = value_type (evaluate_expression (expr));
+	  do_cleanups (old_chain);
+	}
+    }
+  if (e.reason < 0)
+    {
+      warning (_("error saving tracepoint %d \"%s\" to CTF file: %s"),
+	       tps->tp->base.number, str, e.message);
+      return;
+    }
+
+  if (is_ret)
+    align_size = 8;
+  else
+    {
+      align_size = ctf_save_type_check_and_write (tcsp, type);
+      if (align_size < 0)
+	{
+	  warning (_("\
+error saving tracepoint %d \"%s\" to CTF file: type is not supported."),
+		   tps->tp->base.number, str);
+	  return;
+	}
+    }
+
+  collect = (struct ctf_save_collect_s *) xzalloc (sizeof (*collect));
+
+  /* Add tp to the list.  */
+  collect->next = tps->collect;
+  tps->collect = collect;
+
+  collect->str = xstrdup (str);
+  collect->is_ret = is_ret;
+
+  if (!is_ret)
+    {
+      collect->type = type;
+
+      collect->align_size = align_size;
+      if (collect->align_size > tps->align_size)
+        tps->align_size = collect->align_size;
+
+      collect->expr = parse_expression (str);
+    }
+}
+
+struct loc_arg_collect_data
+{
+  struct ctf_save_s *tcsp;
+  struct ctf_save_tp_s *tps;
+};
+
+static void
+tsv_save_do_loc_arg_collect (const char *print_name,
+			     struct symbol *sym,
+			     void *cb_data)
+{
+  struct loc_arg_collect_data *p = cb_data;
+
+  ctf_save_collect_get_1 (p->tcsp, p->tps, print_name);
+}
+
+/* worker function (cleanup) */
+static void
+replace_comma (void *data)
+{
+  char *comma = data;
+  *comma = ',';
+}
+
+/* Get var that want to collect from STR and put them to TPS->collect.
+   This function will not be call when GDB add a new TP.  */
+
+static void
+ctf_save_collect_get (struct ctf_save_s *tcsp, struct ctf_save_tp_s *tps,
+		      char *str)
+{
+  char *action_exp = str, *next_comma;
+  struct cleanup *old_chain;
+
+  do
+    {
+      if (*action_exp == ',')
+	action_exp++;
+      while (isspace ((int) *action_exp))
+	action_exp++;
+
+      next_comma = strchr (action_exp, ',');
+      if (next_comma)
+	{
+	  old_chain = make_cleanup (replace_comma, next_comma);
+	  *next_comma = '\0';
+	}
+
+      if (0 == strncasecmp (action_exp, "$reg", 4))
+        {
+	  int i;
+	  struct gdbarch *arch = tps->tp->base.loc->gdbarch;
+
+	  for (i = 0; i < gdbarch_num_regs (arch); i++)
+	    {
+	      const char *name = gdbarch_register_name (arch, i);
+	      int name_size = strlen (name);
+	      char regname[1 + name_size + 1];
+
+	      if (name_size == 0)
+		continue;
+
+	      sprintf (regname, "$%s", name);
+
+	      ctf_save_collect_get_1 (tcsp, tps, regname);
+	    }
+	}
+      else if (0 == strncasecmp (action_exp, "$loc", 4)
+	       || 0 == strncasecmp (action_exp, "$arg", 4))
+	{
+	  CORE_ADDR pc = 0;
+	  struct loc_arg_collect_data cb_data;
+	  volatile struct gdb_exception e;
+
+	  cb_data.tcsp = tcsp;
+	  cb_data.tps = tps;
+
+	  TRY_CATCH (e, RETURN_MASK_ERROR)
+	    {
+	      pc = regcache_read_pc (get_current_regcache ());
+	    }
+	  if (e.reason < 0)
+	    {
+	      warning (_("error saving tracepoint %d \"%s\" to CTF file: %s"),
+		       tps->tp->base.number, action_exp, e.message);
+	      continue;
+	    }
+
+	  if (add_local_symbols_1 ((0 == strncasecmp (action_exp, "$loc", 4)
+					? 'L' : 'A'),
+				   pc, tsv_save_do_loc_arg_collect,
+				   &cb_data))
+	    {
+	      warning (_("\
+error saving tracepoint %d \"%s\" to CTF file: no symbol table info available."),
+		       tps->tp->base.number, action_exp);
+	      continue;
+	    }
+	}
+      else
+	ctf_save_collect_get_1 (tcsp, tps, action_exp);
+
+      if (next_comma)
+	do_cleanups (old_chain);
+      action_exp = next_comma;
+    }
+  while (action_exp && *action_exp == ',');
+}
+
+/* Call function ctf_save_collect_get Add action list ACTION of TPS
+   to TPS->collect.
+   STEPPING_FRAME is true if it is step action.  */
+
+static void
+ctf_save_tp_actions (struct ctf_save_s *tcsp, struct ctf_save_tp_s *tps,
+		     struct command_line *action, int stepping_frame)
+{
+  for (; action != NULL; action = action->next)
+    {
+      char *action_exp;
+      struct cmd_list_element *cmd;
+
+      QUIT;
+      action_exp = action->line;
+      while (isspace ((int) *action_exp))
+	action_exp++;
+      if (*action_exp == '#')
+	continue;
+
+      cmd = lookup_cmd (&action_exp, cmdlist, "", -1, 1);
+      if (cmd == 0)
+	error (_("Bad action list item: %s"), action_exp);
+
+      if (cmd_cfunc_eq (cmd, while_stepping_pseudocommand))
+	{
+	  int i;
+
+	  for (i = 0; i < action->body_count; ++i)
+	    ctf_save_tp_actions (tcsp, tps, action->body_list[i], 1);
+	}
+      else if (cmd_cfunc_eq (cmd, collect_pseudocommand))
+	{
+	  if (stepping_frame != tps->stepping_frame)
+	    continue;
+	  if (*action_exp == '/')
+	    action_exp = decode_agent_options (action_exp);
+	  ctf_save_collect_get (tcsp, tps, action_exp);
+	}
+    }
+}
+
+
+/* Try to find the ctf_save_tp_s struct in the TCSP->tp.
+   If cannot find it in the TCSP->tp, make a new one for TP
+   and add it to TCSP->tp.  */
+
+static struct ctf_save_tp_s *
+ctf_save_tp_find (struct ctf_save_s *tcsp, struct tracepoint *tp)
+{
+  struct ctf_save_tp_s *ret;
+  struct bp_location *loc;
+  struct regcache *regcache;
+  int stepping_frame = 0;
+  struct command_line *action;
+  volatile struct gdb_exception e;
+  CORE_ADDR pc = 0;
+  extern int tracepoint_count;
+
+  TRY_CATCH (e, RETURN_MASK_ERROR)
+    {
+      pc = regcache_read_pc (get_current_regcache ());
+    }
+  if (e.reason >= 0)
+    {
+      stepping_frame = 1;
+      for (loc = tp->base.loc; loc; loc = loc->next)
+	{
+	  if (loc->address == pc)
+            {
+	      stepping_frame = 0;
+	      break;
+	    }
+        }
+    }
+
+  for (ret = tcsp->tp; ret; ret = ret->next)
+    {
+      if (ret->tp == tp && ret->stepping_frame == stepping_frame)
+	return ret;
+    }
+
+  ret = (struct ctf_save_tp_s *) xzalloc (sizeof (*ret));
+
+  /* Add tp to the list.  */
+  ret->next = tcsp->tp;
+  tcsp->tp = ret;
+
+  ret->stepping_frame = stepping_frame;
+  if (stepping_frame)
+    ret->id = tracepoint_count + tp->base.number;
+  else
+    ret->id = tp->base.number;
+  ret->tp = tp;
+
+  if (!stepping_frame && *default_collect)
+    ctf_save_collect_get (tcsp, ret, default_collect);
+
+  ctf_save_tp_actions (tcsp, ret, breakpoint_commands (&tp->base), 0);
+
+  return ret;
+}
+
+/* Let TCSP->datastream_fd point to the part for next packet.  */
+
+static void
+ctf_save_next_packet (struct ctf_save_s *tcsp)
+{
+  tcsp->packet_begin += CTF_PACKET_SIZE;
+  ctf_save_fseek (tcsp, tcsp->packet_begin, SEEK_SET);
+  tcsp->content_size = 0;
+}
+
+/* Write the content size to packet header context. */
+
+static void
+ctf_save_write_content_size (struct ctf_save_s *tcsp)
+{
+  uint32_t u32;
+
+  ctf_save_fseek (tcsp, tcsp->packet_begin + 4, SEEK_SET);
+  u32 = tcsp->content_size * TARGET_CHAR_BIT;
+  ctf_save_write (tcsp, (void *)&u32, 4);
+}
+
+/* Write the CTF data packet header context.  */
+
+#define PACKET_HEADER_SIZE	(4 + 4 + 4)
+
+static void
+ctf_save_write_packet_header_context (struct ctf_save_s *tcsp)
+{
+  uint32_t u32;
+
+  /* magic.  */
+  u32 = CTF_MAGIC;
+  ctf_save_write (tcsp, (void *)&u32, 4);
+  tcsp->content_size += 4;
+
+  /* content_size.  We still don't know the size, write it later.  */
+  ctf_save_fseek (tcsp, 4, SEEK_CUR);
+  tcsp->content_size += 4;
+
+  /* packet_size */
+  u32 = CTF_PACKET_SIZE * TARGET_CHAR_BIT;
+  ctf_save_write (tcsp, (void *)&u32, 4);
+  tcsp->content_size += 4;
+
+  tcsp->current_content_size = tcsp->content_size;
+
+  /* Make this packet all into file.  */
+  ctf_save_fseek (tcsp, tcsp->packet_begin + CTF_PACKET_SIZE - 4, SEEK_SET);
+  u32 = 0;
+  ctf_save_write (tcsp, (void *)&u32, 4);
+  ctf_save_fseek (tcsp, tcsp->packet_begin + tcsp->content_size, SEEK_SET);
+}
+
+/* Change I-th char in CTF_STR to STR.  */
+
+static char *
+ctf_save_metadata_change_char (char *ctf_str, int i, const char *str)
+{
+  char *new;
+
+  ctf_str[i] = '\0';
+  new = xstrprintf ("%s%s%s_%s", ctf_str, (i ? "_" : ""),
+		    str, ctf_str + i + 1);
+  xfree (ctf_str);
+
+  return new;
+}
+
+/* Write the header of CTF's metadata to TCSP->METADATA_FD.  */
+
+static void
+ctf_save_metadata_header (struct ctf_save_s *tcsp)
+{
+  const char metadata_fmt[] =
+  "\ntrace {\n"
+  "	major = %u;\n"
+  "	minor = %u;\n"
+  "	byte_order = %s;\n"		/* be or le */
+  "	packet.header := struct {\n"
+  "		uint32_t magic;\n"
+  "	};\n"
+  "};\n"
+  "\n"
+  "stream {\n"
+  "	packet.context := struct {\n"
+  "		uint32_t content_size;\n"
+  "		uint32_t packet_size;\n"
+  "	};\n"
+  "	event.header := struct {\n"
+  "		uint32_t id;\n"
+  "	};\n"
+  "};\n";
+
+  ctf_save_fwrite_format (tcsp->metadata_fd, "/* CTF %d.%d */\n",
+			  CTF_SAVE_MAJOR, CTF_SAVE_MINOR);
+  ctf_save_fwrite_format (tcsp->metadata_fd, "\
+typealias integer { size = 32; align = 32; signed = false; } := uint32_t;\n");
+  ctf_save_fwrite_format (tcsp->metadata_fd, "\
+typealias integer { size = 64; align = 64; signed = false; } := uint64_t;\n");
+  ctf_save_fwrite_format (tcsp->metadata_fd, "\n");
+
+  ctf_save_fwrite_format (tcsp->metadata_fd, metadata_fmt,
+			  CTF_SAVE_MAJOR, CTF_SAVE_MINOR,
+			  BYTE_ORDER == LITTLE_ENDIAN ? "le" : "be");
+  ctf_save_fwrite_format (tcsp->metadata_fd, "\n");
+}
+
+/* Write the body of CTF's metadata to TCSP->METADATA_FD.  */
+
+static void
+ctf_save_metadata (struct ctf_save_s *tcsp)
+{
+  struct ctf_save_tp_s *tps;
+  struct ctf_save_collect_s *collect;
+  struct ctf_save_type_s *t;
+
+  ctf_save_fwrite_format (tcsp->metadata_fd, "\n");
+
+  /* Write event.  */
+  for (tps = tcsp->tp; tps; tps = tps->next)
+    {
+      ctf_save_fwrite_format (tcsp->metadata_fd,
+			      "event {\n\tname = \"%s%s\";\n\tid = %u;\n"
+			      "\tfields := struct { \n",
+			      tps->tp->base.addr_string,
+			      tps->stepping_frame ? " while-stepping" : "",
+			      tps->id);
+      for (collect = tps->collect; collect; collect = collect->next)
+        {
+	  char *tmp;
+	  const char *old_tab;
+	  int find_same = 0;
+
+	  if (collect->is_ret)
+	    collect->ctf_str = xstrdup ("ret_pc");
+	  else
+	    {
+	      char *new;
+	      int need_recheck;
+	      int i;
+
+	      collect->ctf_str = xstrdup (collect->str);
+	      for (i = 0; collect->ctf_str[i] != '\0'; ++i)
+	        {
+		  switch (collect->ctf_str[i])
+		    {
+		    case '$':
+		      collect->ctf_str
+			  = ctf_save_metadata_change_char (collect->ctf_str,
+							   i, "dollar");
+		      break;
+		    case '*':
+		      collect->ctf_str
+			  = ctf_save_metadata_change_char (collect->ctf_str,
+							   i, "star");
+		      break;
+		    case ' ':
+		      collect->ctf_str[i] = '_';
+		      break;
+		    }
+		}
+	    }
+
+	  /* Check if TPS include a collect that CTF_STR same
+	     with COLLECT->CTF_STR.
+	     If so, rename it and keep check.  */
+	  tmp = alloca (strlen (collect->ctf_str) + 30);
+	  strcpy (tmp, collect->ctf_str);
+	  while (1)
+	    {
+	      struct ctf_save_collect_s *collect2;
+	      int i = 0;
+
+	      for (collect2 = tps->collect; collect2;
+		   collect2 = collect2->next)
+		{
+		  if (collect != collect2 && collect2->ctf_str
+		      && strcmp (collect2->ctf_str, tmp) == 0)
+		    break;
+		}
+	      if (collect2 == NULL)
+		break;
+
+	      find_same = 1;
+	      xsnprintf (tmp, strlen (collect->ctf_str) + 30,
+			 "%s_%d", collect->ctf_str, i++);
+	    }
+	  if (find_same)
+	    {
+	      xfree (collect->ctf_str);
+	      collect->ctf_str = xstrdup (tmp);
+	    }
+
+	  if (strcmp (collect->ctf_str, collect->str))
+	    warning (_("\
+\"%s\" of tracepoint %d renamed to \"%s\" in CTF file."),
+		       collect->str, tps->tp->base.number,
+		       collect->ctf_str);
+
+	  old_tab = tcsp->tab;
+	  tcsp->tab = "\t\t";
+	  ctf_save_fwrite_format (tcsp->metadata_fd, tcsp->tab);
+	  if (collect->is_ret)
+	    ctf_save_fwrite_format (tcsp->metadata_fd, "uint64_t %s;\n",
+				    collect->ctf_str);
+	  else
+	    ctf_save_var_define_write (tcsp, collect->type,
+				       collect->ctf_str, 0);
+	  tcsp->tab = old_tab;
+	}
+      ctf_save_fwrite_format (tcsp->metadata_fd, "\t};\n};\n");
+    }
+}
+
+/* Clean function for ctf_save.  */
+
+static void
+ctf_save_cleanup (void *p)
+{
+  struct ctf_save_s *tcsp = p;
+  struct ctf_save_tp_s *tp, *tmp_tp;
+  struct ctf_save_type_s *type, *tmp_type;
+
+  if (tcsp->metadata_fd)
+    {
+      ctf_save_metadata (tcsp);
+      fclose (tcsp->metadata_fd);
+    }
+
+  if (tcsp->datastream_fd)
+    fclose (tcsp->datastream_fd);
+
+  for (tp = tcsp->tp; tp; tp = tmp_tp)
+    {
+      struct ctf_save_collect_s *collect, *tmp_collect;
+
+      for (collect = tp->collect; collect; collect = tmp_collect)
+        {
+	  xfree (collect->expr);
+	  xfree (collect->str);
+	  xfree (collect->ctf_str);
+	  tmp_collect = collect->next;
+	  xfree (collect);
+        }
+      tmp_tp = tp->next;
+      xfree (tp);
+    }
+
+  for (type = tcsp->type; type; type = tmp_type)
+    {
+      tmp_type = type->next;
+      xfree (type);
+    }
+
+  reinit_frame_cache ();
+  target_dcache_invalidate ();
+  set_current_traceframe (tcsp->old_traceframe_num);
+}
+
+/* Save current trace frame to DIRNAME.  */
+
+void
+ctf_save (const char *dirname)
+{
+  struct ctf_save_s tcs;
+  struct cleanup *old_chain;
+  int frame_num;
+  char *file_name;
+  struct ctf_save_type_s *t;
+  char tab[] = "";
+
+  /* Create DIRNAME.  */
+  file_name = alloca (strlen (dirname) + 1
+		      + strlen (CTF_DATASTREAM_NAME) + 1);
+  if (mkdir (dirname, S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH)
+      && errno != EEXIST)
+    error (_("Unable to open directory '%s' for saving trace data (%s)"),
+	   dirname, safe_strerror (errno));
+
+  memset (&tcs, '\0', sizeof (tcs));
+  tcs.old_traceframe_num = get_traceframe_number ();
+  old_chain = make_cleanup (ctf_save_cleanup, &tcs);
+  tcs.tab = tab;
+
+  sprintf (file_name, "%s/%s", dirname, CTF_METADATA_NAME);
+  tcs.metadata_fd = fopen (file_name, "w");
+  if (!tcs.metadata_fd)
+    error (_("Unable to open file '%s' for saving trace data (%s)"),
+	   file_name, safe_strerror (errno));
+  ctf_save_metadata_header (&tcs);
+
+  sprintf (file_name, "%s/%s", dirname, CTF_DATASTREAM_NAME);
+  tcs.datastream_fd = fopen (file_name, "w");
+  if (!tcs.datastream_fd)
+    error (_("Unable to open file '%s' for saving trace data (%s)"),
+	   file_name, safe_strerror (errno));
+
+  ctf_save_write_packet_header_context (&tcs);
+
+  for (frame_num = 0; 1; frame_num ++)
+    {
+      int tnum;
+      struct tracepoint *tp;
+      int try_count;
+      uint32_t u32;
+      struct ctf_save_tp_s *tps;
+      struct ctf_save_collect_s *collect;
+      int traceframe_is_dropped = 0;
+
+      /* Allow user to bail out with ^C.  */
+      QUIT;
+
+      /* Goto traceframe frame_num and set tp.  */
+      frame_num = target_trace_find (tfind_number, frame_num, 0, 0, &tnum);
+      if (frame_num < 0)
+	break;
+      tp = get_tracepoint_by_number_on_target (tnum);
+      if (!tp)
+        {
+	  warning (_("drop traceframe %d because cannot find tracepoint %d."),
+		   frame_num, tnum);
+	  continue;
+	}
+      if (!tp->base.loc)
+        {
+	  warning (_("drop traceframe %d because tracepoint %d is pending."),
+		   frame_num, tp->base.number);
+	  continue;
+	}
+      reinit_frame_cache ();
+      target_dcache_invalidate ();
+      set_current_traceframe (frame_num);
+
+      tps = ctf_save_tp_find (&tcs, tp);
+
+      /* The tp is not variable-length and bigger than CTF_PACKET_SIZE.
+         So drop it.  */
+      if (tps->is_dropped)
+	continue;
+
+      /* Try count for current tp write.
+         If try second time, the event size bigger than a packet.
+         Then drop this event.  */
+      try_count = 0;
+
+re_write_tp:
+      if (try_count > 0)
+        {
+	  /* Handle retry.  */
+	  if (try_count > 1 || PACKET_HEADER_SIZE == tcs.content_size)
+	    {
+	      /* Second retry or packet just write that packet means
+	         this TP is too big.  So drop it.  */
+	      if (tps->is_variable_length)
+	        {
+		  /* The tp is variable-length.  */
+		  warning (_("\
+traceframe %d of tracepoint %d needs to save data that is bigger than packet size %d.\n\
+So it will be dropped."), frame_num, tps->tp->base.number, CTF_PACKET_SIZE);
+		}
+	      else
+	        {
+		  /* The tp is not variable-length.  */
+		  warning (_("\
+tracepoint %d need save data that bigger than packet size %d.\n\
+So all of its traceframes will be dropped."),
+			   tps->tp->base.number, CTF_PACKET_SIZE);
+		  /* Mark this tp to let GDB drop its traceframes.  */
+		  tps->is_dropped = 1;
+		}
+	      continue;
+	    }
+
+	  tcs.check_size = 0;
+	  ctf_save_write_content_size (&tcs);
+          ctf_save_next_packet (&tcs);
+	  ctf_save_write_packet_header_context (&tcs);
+	}
+
+      try_count ++;
+
+      tcs.current_content_size = tcs.content_size;
+      tcs.check_size = 1;
+
+      u32 = (uint32_t) tps->id;
+      if (ctf_save_align_write (&tcs, (void *)&u32, sizeof (u32),
+				sizeof (u32)))
+	goto re_write_tp;
+
+      /* Align.  */
+      if (tps->align_size)
+        {
+	  if (ctf_save_fseek (&tcs,
+			      ALIGN_SIZE (tcs.current_content_size,
+					  tps->align_size),
+			      SEEK_CUR))
+	    goto re_write_tp;
+	}
+
+      for (collect = tps->collect; collect; collect = collect->next)
+        {
+	  volatile struct gdb_exception e;
+
+	  if (collect->is_ret)
+	    {
+	      CORE_ADDR pc;
+	      uint64_t u64;
+
+	      TRY_CATCH (e, RETURN_MASK_ERROR)
+		{
+		  struct frame_info *frame;
+
+		  frame = get_current_frame ();
+		  if (!frame)
+		    error (_("get current frame fail"));
+		  frame = get_prev_frame (frame);
+		  if (!frame)
+		    error (_("get prev frame fail"));
+
+		  if (!get_frame_pc_if_available (frame, &pc))
+		    error (_("PC unavailable"));
+		}
+	      if (e.reason < 0)
+		{
+		  warning (_("\
+traceframe %d is dropped because GDB try to get the value of \"%s\" got error: %s"),
+			   frame_num, collect->str, e.message);
+		  traceframe_is_dropped = 1;
+		  break;
+		}
+
+	      u64 = pc;
+	      if (ctf_save_align_write (&tcs, (gdb_byte *) &u64,
+					sizeof (u64), sizeof (u64)))
+		goto re_write_tp;
+	    }
+	  else
+            {
+	      struct value *val = NULL;
+	      struct cleanup *back_chain;
+	      const gdb_byte *content = NULL;
+
+	      back_chain = make_cleanup (null_cleanup, NULL);
+	      TRY_CATCH (e, RETURN_MASK_ERROR)
+	        {
+		  val = evaluate_expression (collect->expr);
+		  content = value_contents (val);
+		}
+	      if (e.reason < 0 || collect->type != value_type (val))
+		{
+		  if (e.reason < 0)
+		    warning (_("\
+traceframe %d is dropped because try to get the value of \"%s\" got error: %s"),
+			     frame_num, collect->str, e.message);
+		  else
+		    warning (_("\
+traceframe %d is dropped because type of \"%s\" is wrong."),
+			     frame_num, collect->str);
+		  traceframe_is_dropped = 1;
+		  do_cleanups (back_chain);
+		  break;
+		}
+
+	      /* Write this val according to type.  */
+	      if (ctf_save_align_write (&tcs, content,
+					TYPE_LENGTH (collect->type),
+					collect->align_size))
+		{
+		  do_cleanups (back_chain);
+		  goto re_write_tp;
+		}
+
+	      /* Free the memory that alloc by evaluate_expression.  */
+	      do_cleanups (back_chain);
+	    }
+	}
+
+      if (traceframe_is_dropped)
+	continue;
+
+      tcs.content_size = tcs.current_content_size;
+    }
+  ctf_save_write_content_size (&tcs);
+
+  do_cleanups (old_chain);
+}
--- /dev/null
+++ b/ctf.h
@@ -0,0 +1,25 @@
+/* CTF format support.
+
+   Copyright (C) 2012 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 <http://www.gnu.org/licenses/>.  */
+
+#ifndef CTF_H
+#define CTF_H
+
+extern void ctf_save (const char *dirname);
+
+#endif
--- a/mi/mi-main.c
+++ b/mi/mi-main.c
@@ -49,6 +49,7 @@
 #include "osdata.h"
 #include "splay-tree.h"
 #include "tracepoint.h"
+#include "ctf.h"
 #include "ada-lang.h"
 #include "linespec.h"
 
@@ -2477,16 +2478,19 @@ void
 mi_cmd_trace_save (char *command, char **argv, int argc)
 {
   int target_saves = 0;
+  int generate_ctf = 0;
   char *filename;
 
   if (argc != 1 && argc != 2)
-    error (_("Usage: -trace-save [-r] filename"));
+    error (_("Usage: -trace-save [-r] [-ctf] filename"));
 
   if (argc == 2)
     {
       filename = argv[1];
       if (strcmp (argv[0], "-r") == 0)
 	target_saves = 1;
+      if (strcmp (argv[0], "-ctf") == 0)
+	generate_ctf = 1;
       else
 	error (_("Invalid option: %s"), argv[0]);
     }
@@ -2495,7 +2499,10 @@ mi_cmd_trace_save (char *command, char *
       filename = argv[0];
     }
 
-  trace_save (filename, target_saves);
+  if (generate_ctf)
+    ctf_save (filename);
+  else
+    trace_save (filename, target_saves);
 }
 
 void
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -53,6 +53,7 @@
 #include "exceptions.h"
 #include "cli/cli-utils.h"
 #include "probe.h"
+#include "ctf.h"
 
 /* readline include files */
 #include "readline/readline.h"
@@ -588,13 +589,13 @@ end_actions_pseudocommand (char *args, i
   error (_("This command cannot be used at the top level."));
 }
 
-static void
+void
 while_stepping_pseudocommand (char *args, int from_tty)
 {
   error (_("This command can only be used in a tracepoint actions list."));
 }
 
-static void
+void
 collect_pseudocommand (char *args, int from_tty)
 {
   error (_("This command can only be used in a tracepoint actions list."));
@@ -1168,6 +1169,34 @@ do_collect_symbol (const char *print_nam
   p->count++;
 }
 
+int
+add_local_symbols_1 (int type, CORE_ADDR pc,
+		     iterate_over_block_arg_local_vars_cb cb,
+		     void *cb_data)
+{
+  struct block *block;
+
+  if (type == 'L')
+    {
+      block = block_for_pc (pc);
+      if (block == NULL)
+	return -1;
+
+      iterate_over_block_local_vars (block, cb, cb_data);
+    }
+  else
+    {
+      pc = get_pc_function_start (pc);
+      block = block_for_pc (pc);
+      if (block == NULL)
+	return -1;
+
+      iterate_over_block_arg_vars (block, cb, cb_data);
+    }
+
+  return 0;
+}
+
 /* Add all locals (or args) symbols to collection list.  */
 static void
 add_local_symbols (struct collection_list *collect,
@@ -1176,6 +1205,7 @@ add_local_symbols (struct collection_lis
 {
   struct block *block;
   struct add_local_symbols_data cb_data;
+  const char *name;
 
   cb_data.collect = collect;
   cb_data.gdbarch = gdbarch;
@@ -1185,33 +1215,19 @@ add_local_symbols (struct collection_lis
   cb_data.count = 0;
 
   if (type == 'L')
-    {
-      block = block_for_pc (pc);
-      if (block == NULL)
-	{
-	  warning (_("Can't collect locals; "
-		     "no symbol table info available.\n"));
-	  return;
-	}
-
-      iterate_over_block_local_vars (block, do_collect_symbol, &cb_data);
-      if (cb_data.count == 0)
-	warning (_("No locals found in scope."));
-    }
+    name = "locals";
   else
-    {
-      pc = get_pc_function_start (pc);
-      block = block_for_pc (pc);
-      if (block == NULL)
-	{
-	  warning (_("Can't collect args; no symbol table info available."));
-	  return;
-	}
+    name = "args";
 
-      iterate_over_block_arg_vars (block, do_collect_symbol, &cb_data);
-      if (cb_data.count == 0)
-	warning (_("No args found in scope."));
+  if (add_local_symbols_1 (type, pc, do_collect_symbol, &cb_data))
+    {
+      warning (_("Can't collect %s; no symbol table info available.\n"),
+	       name);
+      return;
     }
+
+  if (cb_data.count == 0)
+    warning (_("No %s found in scope."), name);
 }
 
 static void
@@ -3182,6 +3198,7 @@ static void
 trace_save_command (char *args, int from_tty)
 {
   int target_does_save = 0;
+  int generate_ctf = 0;
   char **argv;
   char *filename = NULL;
   struct cleanup *back_to;
@@ -3196,6 +3213,8 @@ trace_save_command (char *args, int from
     {
       if (strcmp (*argv, "-r") == 0)
 	target_does_save = 1;
+      if (strcmp (*argv, "-ctf") == 0)
+	generate_ctf = 1;
       else if (**argv == '-')
 	error (_("unknown option `%s'"), *argv);
       else
@@ -3205,10 +3224,18 @@ trace_save_command (char *args, int from
   if (!filename)
     error_no_arg (_("file in which to save trace data"));
 
-  trace_save (filename, target_does_save);
+  if (generate_ctf)
+    {
+      if (target_does_save)
+	error_no_arg (_("-r cannot be used with -ctf."));
+      ctf_save (filename);
+    }
+  else
+    trace_save (filename, target_does_save);
 
   if (from_tty)
-    printf_filtered (_("Trace data saved to file '%s'.\n"), filename);
+    printf_filtered (_("Trace data saved to %s '%s'.\n"),
+		     generate_ctf ? "directory" : "file", filename);
 
   do_cleanups (back_to);
 }
@@ -5251,6 +5278,7 @@ _initialize_tracepoint (void)
   add_com ("tsave", class_trace, trace_save_command, _("\
 Save the trace data to a file.\n\
 Use the '-r' option to direct the target to save directly to the file,\n\
+Use the '-ctf' option to save the data to CTF format,\n\
 using its own filesystem."));
 
   c = add_com ("tvariable", class_trace, trace_variable_command,_("\
--- a/tracepoint.h
+++ b/tracepoint.h
@@ -23,6 +23,7 @@
 #include "target.h"
 #include "memrange.h"
 #include "gdb_vecs.h"
+#include "stack.h"
 
 /* A trace state variable is a value managed by a target being
    traced.  A trace state variable (or tsv for short) can be accessed
@@ -245,6 +246,9 @@ extern void encode_actions (struct break
 extern void validate_actionline (char **, struct breakpoint *);
 extern void validate_trace_state_variable_name (const char *name);
 
+extern void while_stepping_pseudocommand (char *args, int from_tty);
+extern void collect_pseudocommand (char *args, int from_tty);
+
 extern struct trace_state_variable *find_trace_state_variable (const char *name);
 extern struct trace_state_variable *create_trace_state_variable (const char *name);
 
@@ -288,4 +292,7 @@ extern struct traceframe_info *parse_tra
 extern int traceframe_available_memory (VEC(mem_range_s) **result,
 					CORE_ADDR memaddr, ULONGEST len);
 
+extern int add_local_symbols_1 (int type, CORE_ADDR pc,
+				iterate_over_block_arg_local_vars_cb cb,
+				void *cb_data);
 #endif	/* TRACEPOINT_H */