diff mbox

[1/2] trace: allow trace events with string arguments

Message ID 1315237048-3498-1-git-send-email-stefanha@linux.vnet.ibm.com
State New
Headers show

Commit Message

Stefan Hajnoczi Sept. 5, 2011, 3:37 p.m. UTC
String arguments are useful for producing human-readable traces without
post-processing (e.g. stderr backend).  Although the simple backend
cannot handles strings all others can.  Strings should be allowed and
the simple backend can be extended to support them.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 docs/tracing.txt |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

Comments

Blue Swirl Sept. 5, 2011, 7:45 p.m. UTC | #1
On Mon, Sep 5, 2011 at 3:37 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> String arguments are useful for producing human-readable traces without
> post-processing (e.g. stderr backend).  Although the simple backend
> cannot handles strings all others can.  Strings should be allowed and
> the simple backend can be extended to support them.

I don't think this is possible in general. Yes if the string can be
found in the executable (assuming address space randomizations don't
make that impossible post run), but not if the string happens to be
constructed in the stack or in the data segment during run time.

So I'd still at least strongly discourage string use.

> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  docs/tracing.txt |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/docs/tracing.txt b/docs/tracing.txt
> index 4b27ab0..e130a61 100644
> --- a/docs/tracing.txt
> +++ b/docs/tracing.txt
> @@ -70,11 +70,6 @@ Trace events should use types as follows:
>    cannot include all user-defined struct declarations and it is therefore
>    necessary to use void * for pointers to structs.
>
> -   Pointers (including char *) cannot be dereferenced easily (or at all) in
> -   some trace backends.  If pointers are used, ensure they are meaningful by
> -   themselves and do not assume the data they point to will be traced.  Do
> -   not pass in string arguments.
> -
>  * For everything else, use primitive scalar types (char, int, long) with the
>    appropriate signedness.
>
> @@ -185,6 +180,9 @@ source tree.  It may not be as powerful as platform-specific or third-party
>  trace backends but it is portable.  This is the recommended trace backend
>  unless you have specific needs for more advanced backends.
>
> +The "simple" backend currently does not capture string arguments, it simply
> +records the char* pointer value instead of the string that is pointed to.
> +
>  ==== Monitor commands ====
>
>  * info trace
> --
> 1.7.5.4
>
>
>
Jan Kiszka Sept. 5, 2011, 9:17 p.m. UTC | #2
On 2011-09-05 21:45, Blue Swirl wrote:
> On Mon, Sep 5, 2011 at 3:37 PM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com> wrote:
>> String arguments are useful for producing human-readable traces without
>> post-processing (e.g. stderr backend).  Although the simple backend
>> cannot handles strings all others can.  Strings should be allowed and
>> the simple backend can be extended to support them.
> 
> I don't think this is possible in general. Yes if the string can be
> found in the executable (assuming address space randomizations don't
> make that impossible post run), but not if the string happens to be
> constructed in the stack or in the data segment during run time.

Strings can be addressed in tracers like simpletrace by storing a
fixed-size copy (e.g. 64 chars) in the log. That will work out for the
majority of use cases.

Jan
Stefan Hajnoczi Sept. 6, 2011, 2:24 p.m. UTC | #3
On Mon, Sep 05, 2011 at 07:45:36PM +0000, Blue Swirl wrote:
> On Mon, Sep 5, 2011 at 3:37 PM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com> wrote:
> > String arguments are useful for producing human-readable traces without
> > post-processing (e.g. stderr backend).  Although the simple backend
> > cannot handles strings all others can.  Strings should be allowed and
> > the simple backend can be extended to support them.
> 
> I don't think this is possible in general. Yes if the string can be
> found in the executable (assuming address space randomizations don't
> make that impossible post run), but not if the string happens to be
> constructed in the stack or in the data segment during run time.
> 
> So I'd still at least strongly discourage string use.

I don't like strings either because they introduce variable-length data
that can be expensive to fetch.  But it's perfectly doable: stderr and
ust are in-process tracers that have easy access to the string no matter
where it is located in memory.  In SystemTap you only have the char*
argument but can use the userspace string copy-in function to fetch the
actual string (again, no matter where it lives in the process' address
space).

If your primary trace backend is stderr or ust it makes sense to use
them.  If you use SystemTap or simpletrace where it is relatively easy
to do post-processing, then it's lighter weight and nicer (IMO) to
record just the address and leave pretty-printing to a post-processing
step.

We can add string support to simpletrace by creating a new file format
version (or moving to a standarized trace file format).  The format
needs to support variable-length trace records, which the current format
cannot support.  Then tracetool can detect char* type arguments and add
strcpy code into the generated trace_*().

Anyway, tracing should be an aid to developers and users.  It should
make USB development easier for Gerd.  We need to compete with fprintf
here :).

I don't think there is a technical reason why strings are not possible,
it turns out only simpletrace can't do them and that's my fault.

Stefan
Blue Swirl Sept. 7, 2011, 7:21 p.m. UTC | #4
On Tue, Sep 6, 2011 at 2:24 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Sep 05, 2011 at 07:45:36PM +0000, Blue Swirl wrote:
>> On Mon, Sep 5, 2011 at 3:37 PM, Stefan Hajnoczi
>> <stefanha@linux.vnet.ibm.com> wrote:
>> > String arguments are useful for producing human-readable traces without
>> > post-processing (e.g. stderr backend).  Although the simple backend
>> > cannot handles strings all others can.  Strings should be allowed and
>> > the simple backend can be extended to support them.
>>
>> I don't think this is possible in general. Yes if the string can be
>> found in the executable (assuming address space randomizations don't
>> make that impossible post run), but not if the string happens to be
>> constructed in the stack or in the data segment during run time.
>>
>> So I'd still at least strongly discourage string use.
>
> I don't like strings either because they introduce variable-length data
> that can be expensive to fetch.

This is still a valid point to discourage their use albeit not so strongly.

>  But it's perfectly doable: stderr and
> ust are in-process tracers that have easy access to the string no matter
> where it is located in memory.  In SystemTap you only have the char*
> argument but can use the userspace string copy-in function to fetch the
> actual string (again, no matter where it lives in the process' address
> space).
>
> If your primary trace backend is stderr or ust it makes sense to use
> them.  If you use SystemTap or simpletrace where it is relatively easy
> to do post-processing, then it's lighter weight and nicer (IMO) to
> record just the address and leave pretty-printing to a post-processing
> step.
>
> We can add string support to simpletrace by creating a new file format
> version (or moving to a standarized trace file format).  The format
> needs to support variable-length trace records, which the current format
> cannot support.  Then tracetool can detect char* type arguments and add
> strcpy code into the generated trace_*().
>
> Anyway, tracing should be an aid to developers and users.  It should
> make USB development easier for Gerd.  We need to compete with fprintf
> here :).

It was pretty easy to avoid most uses of strings for those DPRINTF
conversions that I have made. The benefits of tracing aren't in the
replacement process (or even development ease), but actual use of the
trace points. There fprintf can't compete.

> I don't think there is a technical reason why strings are not possible,
> it turns out only simpletrace can't do them and that's my fault.

Nobody came up with a better design at that time, so it was the state
of the art. Maybe it still is since you plan to improve it instead of
scrapping.
diff mbox

Patch

diff --git a/docs/tracing.txt b/docs/tracing.txt
index 4b27ab0..e130a61 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -70,11 +70,6 @@  Trace events should use types as follows:
    cannot include all user-defined struct declarations and it is therefore
    necessary to use void * for pointers to structs.
 
-   Pointers (including char *) cannot be dereferenced easily (or at all) in
-   some trace backends.  If pointers are used, ensure they are meaningful by
-   themselves and do not assume the data they point to will be traced.  Do
-   not pass in string arguments.
-
  * For everything else, use primitive scalar types (char, int, long) with the
    appropriate signedness.
 
@@ -185,6 +180,9 @@  source tree.  It may not be as powerful as platform-specific or third-party
 trace backends but it is portable.  This is the recommended trace backend
 unless you have specific needs for more advanced backends.
 
+The "simple" backend currently does not capture string arguments, it simply
+records the char* pointer value instead of the string that is pointed to.
+
 ==== Monitor commands ====
 
 * info trace