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

[Philippe Mathieu-Daudé]

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)
>>>
>>
>