diff mbox series

[v2,1/4] scripts/tracetool: Fix dtrace generation for macOS

Message ID 20200717093517.73397-2-r.bolshakov@yadro.com (mailing list archive)
State New, archived
Headers show
Series Add dtrace support on macOS | expand

Commit Message

Roman Bolshakov July 17, 2020, 9:35 a.m. UTC
dtrace USDT is fully supported since OS X 10.6. There are a few
peculiarities compared to other dtrace flavors.

1. It doesn't accept empty files.
2. It doesn't recognize bool type but accepts C99 _Bool.
3. It converts int8_t * in probe points to char * in
   header files and introduces [-Wpointer-sign] warning.

Cc: Cameron Esfahani <dirty@apple.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 scripts/tracetool/format/d.py | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé July 19, 2020, 1:52 p.m. UTC | #1
On 7/17/20 11:35 AM, Roman Bolshakov wrote:
> dtrace USDT is fully supported since OS X 10.6. There are a few
> peculiarities compared to other dtrace flavors.
> 
> 1. It doesn't accept empty files.
> 2. It doesn't recognize bool type but accepts C99 _Bool.
> 3. It converts int8_t * in probe points to char * in
>    header files and introduces [-Wpointer-sign] warning.
> 
> Cc: Cameron Esfahani <dirty@apple.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  scripts/tracetool/format/d.py | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
> index 0afb5f3f47..353722f89c 100644
> --- a/scripts/tracetool/format/d.py
> +++ b/scripts/tracetool/format/d.py
> @@ -13,6 +13,7 @@ __email__      = "stefanha@redhat.com"
>  
>  
>  from tracetool import out
> +from sys import platform
>  
>  
>  # Reserved keywords from
> @@ -34,7 +35,8 @@ def generate(events, backend, group):
>  
>      # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is happy
>      # with an empty file.  Avoid the warning.
> -    if not events:
> +    # But dtrace on macOS can't deal with empty files.
> +    if not events and platform != "darwin":

or?

>          return
>  
>      out('/* This file is autogenerated by tracetool, do not edit. */'
> @@ -44,6 +46,17 @@ def generate(events, backend, group):
>      for e in events:
>          args = []
>          for type_, name in e.args:
> +            if platform == "darwin":
> +                # macOS dtrace accepts only C99 _Bool

Why not do that for all platforms?

> +                if type_ == 'bool':
> +                    type_ = '_Bool'
> +                if type_ == 'bool *':
> +                    type_ = '_Bool *'
> +                # It converts int8_t * in probe points to char * in header
> +                # files and introduces [-Wpointer-sign] warning.
> +                # Avoid it by changing probe type to signed char * beforehand.
> +                if type_ == 'int8_t *':
> +                    type_ = 'signed char *'
>              if name in RESERVED_WORDS:
>                  name += '_'
>              args.append(type_ + ' ' + name)
>
Roman Bolshakov July 20, 2020, 10:50 a.m. UTC | #2
On Sun, Jul 19, 2020 at 03:52:08PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/17/20 11:35 AM, Roman Bolshakov wrote:
> > dtrace USDT is fully supported since OS X 10.6. There are a few
> > peculiarities compared to other dtrace flavors.
> > 
> > 1. It doesn't accept empty files.
> > 2. It doesn't recognize bool type but accepts C99 _Bool.
> > 3. It converts int8_t * in probe points to char * in
> >    header files and introduces [-Wpointer-sign] warning.
> > 
> > Cc: Cameron Esfahani <dirty@apple.com>
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> > ---
> >  scripts/tracetool/format/d.py | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
> > index 0afb5f3f47..353722f89c 100644
> > --- a/scripts/tracetool/format/d.py
> > +++ b/scripts/tracetool/format/d.py
> > @@ -13,6 +13,7 @@ __email__      = "stefanha@redhat.com"
> >  
> >  
> >  from tracetool import out
> > +from sys import platform
> >  
> >  
> >  # Reserved keywords from
> > @@ -34,7 +35,8 @@ def generate(events, backend, group):
> >  
> >      # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is happy
> >      # with an empty file.  Avoid the warning.
> > -    if not events:
> > +    # But dtrace on macOS can't deal with empty files.
> > +    if not events and platform != "darwin":
> 
> or?

no, the event list is empty for some files where all events are
disabled (e.g. hppa/trace-events), so it should have an "and" here. This
limits early exit only on macOS. The precendence looks correct:

https://docs.python.org/3/reference/expressions.html#operator-precedence

> 
> >          return
> >  
> >      out('/* This file is autogenerated by tracetool, do not edit. */'
> > @@ -44,6 +46,17 @@ def generate(events, backend, group):
> >      for e in events:
> >          args = []
> >          for type_, name in e.args:
> > +            if platform == "darwin":
> > +                # macOS dtrace accepts only C99 _Bool
> 
> Why not do that for all platforms?
> 

Because I can only test the changes on darwin :)
I don't know how other dtrace flavors behave and whether it is an issue
for dtrace on Linux/Solaris/FreeBSD/etc.

Thanks,
Roman

> > +                if type_ == 'bool':
> > +                    type_ = '_Bool'
> > +                if type_ == 'bool *':
> > +                    type_ = '_Bool *'
> > +                # It converts int8_t * in probe points to char * in header
> > +                # files and introduces [-Wpointer-sign] warning.
> > +                # Avoid it by changing probe type to signed char * beforehand.
> > +                if type_ == 'int8_t *':
> > +                    type_ = 'signed char *'
> >              if name in RESERVED_WORDS:
> >                  name += '_'
> >              args.append(type_ + ' ' + name)
> > 
>
Philippe Mathieu-Daudé July 20, 2020, 10:53 a.m. UTC | #3
On 7/20/20 12:50 PM, Roman Bolshakov wrote:
> On Sun, Jul 19, 2020 at 03:52:08PM +0200, Philippe Mathieu-Daudé wrote:
>> On 7/17/20 11:35 AM, Roman Bolshakov wrote:
>>> dtrace USDT is fully supported since OS X 10.6. There are a few
>>> peculiarities compared to other dtrace flavors.
>>>
>>> 1. It doesn't accept empty files.
>>> 2. It doesn't recognize bool type but accepts C99 _Bool.
>>> 3. It converts int8_t * in probe points to char * in
>>>    header files and introduces [-Wpointer-sign] warning.
>>>
>>> Cc: Cameron Esfahani <dirty@apple.com>
>>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
>>> ---
>>>  scripts/tracetool/format/d.py | 15 ++++++++++++++-
>>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
>>> index 0afb5f3f47..353722f89c 100644
>>> --- a/scripts/tracetool/format/d.py
>>> +++ b/scripts/tracetool/format/d.py
>>> @@ -13,6 +13,7 @@ __email__      = "stefanha@redhat.com"
>>>  
>>>  
>>>  from tracetool import out
>>> +from sys import platform
>>>  
>>>  
>>>  # Reserved keywords from
>>> @@ -34,7 +35,8 @@ def generate(events, backend, group):
>>>  
>>>      # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is happy
>>>      # with an empty file.  Avoid the warning.
>>> -    if not events:
>>> +    # But dtrace on macOS can't deal with empty files.
>>> +    if not events and platform != "darwin":
>>
>> or?
> 
> no, the event list is empty for some files where all events are
> disabled (e.g. hppa/trace-events), so it should have an "and" here. This
> limits early exit only on macOS.

Ah yes you are right, I misread it.

> The precendence looks correct:
> 
> https://docs.python.org/3/reference/expressions.html#operator-precedence
> 
>>
>>>          return
>>>  
>>>      out('/* This file is autogenerated by tracetool, do not edit. */'
>>> @@ -44,6 +46,17 @@ def generate(events, backend, group):
>>>      for e in events:
>>>          args = []
>>>          for type_, name in e.args:
>>> +            if platform == "darwin":
>>> +                # macOS dtrace accepts only C99 _Bool
>>
>> Why not do that for all platforms?
>>
> 
> Because I can only test the changes on darwin :)
> I don't know how other dtrace flavors behave and whether it is an issue
> for dtrace on Linux/Solaris/FreeBSD/etc.
> 
> Thanks,
> Roman
> 
>>> +                if type_ == 'bool':
>>> +                    type_ = '_Bool'
>>> +                if type_ == 'bool *':
>>> +                    type_ = '_Bool *'
>>> +                # It converts int8_t * in probe points to char * in header
>>> +                # files and introduces [-Wpointer-sign] warning.
>>> +                # Avoid it by changing probe type to signed char * beforehand.
>>> +                if type_ == 'int8_t *':
>>> +                    type_ = 'signed char *'
>>>              if name in RESERVED_WORDS:
>>>                  name += '_'
>>>              args.append(type_ + ' ' + name)
>>>
>>
>
Stefan Hajnoczi July 21, 2020, 12:33 p.m. UTC | #4
On Fri, Jul 17, 2020 at 12:35:14PM +0300, Roman Bolshakov wrote:
> dtrace USDT is fully supported since OS X 10.6. There are a few
> peculiarities compared to other dtrace flavors.
> 
> 1. It doesn't accept empty files.
> 2. It doesn't recognize bool type but accepts C99 _Bool.
> 3. It converts int8_t * in probe points to char * in
>    header files and introduces [-Wpointer-sign] warning.
> 
> Cc: Cameron Esfahani <dirty@apple.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  scripts/tracetool/format/d.py | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox series

Patch

diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
index 0afb5f3f47..353722f89c 100644
--- a/scripts/tracetool/format/d.py
+++ b/scripts/tracetool/format/d.py
@@ -13,6 +13,7 @@  __email__      = "stefanha@redhat.com"
 
 
 from tracetool import out
+from sys import platform
 
 
 # Reserved keywords from
@@ -34,7 +35,8 @@  def generate(events, backend, group):
 
     # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is happy
     # with an empty file.  Avoid the warning.
-    if not events:
+    # But dtrace on macOS can't deal with empty files.
+    if not events and platform != "darwin":
         return
 
     out('/* This file is autogenerated by tracetool, do not edit. */'
@@ -44,6 +46,17 @@  def generate(events, backend, group):
     for e in events:
         args = []
         for type_, name in e.args:
+            if platform == "darwin":
+                # macOS dtrace accepts only C99 _Bool
+                if type_ == 'bool':
+                    type_ = '_Bool'
+                if type_ == 'bool *':
+                    type_ = '_Bool *'
+                # It converts int8_t * in probe points to char * in header
+                # files and introduces [-Wpointer-sign] warning.
+                # Avoid it by changing probe type to signed char * beforehand.
+                if type_ == 'int8_t *':
+                    type_ = 'signed char *'
             if name in RESERVED_WORDS:
                 name += '_'
             args.append(type_ + ' ' + name)