From 7fffc7d16d383d900f79f3632ebbccb2a2a7ff9e Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Thu, 3 Sep 2020 21:06:03 -0400 Subject: [PATCH] config: replace trace type's default BO with configuration's target BO This patch removes the `default_byte_order` property from the `barectf.TraceType` class. Instead of having this helper, a configuration now has a mandatory `target_byte_order` property. Because of the trace type's default byte order role in `barectf-bitfield.h`, it's more fundamental than a mere "default": it's actually the byte order you expect the target system to have. In other words, you could not make the trace type's default byte order big-endian to when your target system is a little-endian machine: the bit array serialization process will fail at tracing time. Therefore I found that the "default byte order" term was misleading. This change brings a challenge, however: when you don't specify a group of stream type or trace type features, or when you specify `barectf.DEFAULT_FIELD_TYPE` for a given feature field type, which byte order should the automatically created integer field type(s) have? My solution is to add an optional default byte order parameter to each constructor which _could_ create an integer field type (recursively). As soon as this is going to happen, the default byte order must not be `None`. For example, when you create a `barectf.StreamTypeEventFeatures` object, you can use `barectf.DEFAULT_FIELD_TYPE`, but you must also pass a default byte order: features = barectf.StreamTypeEventFeatures(barectf.DEFAULT_FIELD_TYPE, default_byte_order=barectf.ByteOrder.LITTLE_ENDIAN) This can also be when you create a stream type or a trace type, as if you don't pass any feature object, then the constructor creates one for you, which eventually creates default field types. For example: trace_type = barectf.TraceType(my_stream_types, default_byte_order=barectf.ByteOrder.BIG_ENDIAN) This design is not super elegant, but it's a compromise for the constructors to have everything they need to create full integer field types. This is also why the `byte_order` property of a bit array field type is now mandatory; there's no (peculiar) temporary state where it remains `None` until the complete trace type exists. I thought about making the target byte order little-endian by default, for example, but considering that barectf is used to generate tracers for embedded and bare-metal systems, I found that solution too indiscriminate. In a barectf 3 YAML configuration file: * The trace type's `$default-byte-order` is removed. * There's a new, mandatory `target-byte-order` configuration property. * You can still not specify any bit array field type byte order: the parser uses the configuration's target byte order in that case. A barectf 2 YAML trace type's `byte-order` property (which is already mandatory) is copied as the v3 configuration node's `target-byte-order` property. `bitfield.h.j2` and `metadata.j2` are changed to use the configuration's target byte order instead of the trace type's default byte order. Signed-off-by: Philippe Proulx --- barectf/config.py | 140 ++++++++++++------------- barectf/config_parse_v2.py | 5 +- barectf/config_parse_v3.py | 47 +++------ barectf/schemas/config/3/config.yaml | 6 +- barectf/templates/c/bitfield.h.j2 | 6 +- barectf/templates/metadata/metadata.j2 | 2 +- 6 files changed, 94 insertions(+), 112 deletions(-) diff --git a/barectf/config.py b/barectf/config.py index 6772de9..09f102e 100644 --- a/barectf/config.py +++ b/barectf/config.py @@ -22,7 +22,7 @@ # SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. import barectf.version as barectf_version -from typing import Optional, Any, FrozenSet, Mapping, Iterator, Set, Union +from typing import Optional, Any, FrozenSet, Mapping, Iterator, Set, Union, Callable import typing from barectf.typing import Count, Alignment, _OptStr, Id import collections.abc @@ -49,8 +49,7 @@ class _FieldType: class _BitArrayFieldType(_FieldType): - def __init__(self, size: Count, byte_order: Optional[ByteOrder] = None, - alignment: Alignment = Alignment(1)): + def __init__(self, size: Count, byte_order: ByteOrder, alignment: Alignment = Alignment(1)): self._size = size self._byte_order = byte_order self._alignment = alignment @@ -60,7 +59,7 @@ class _BitArrayFieldType(_FieldType): return self._size @property - def byte_order(self) -> Optional[ByteOrder]: + def byte_order(self) -> ByteOrder: return self._byte_order @property @@ -76,8 +75,7 @@ class DisplayBase(enum.Enum): class _IntegerFieldType(_BitArrayFieldType): - def __init__(self, size: Count, byte_order: Optional[ByteOrder] = None, - alignment: Optional[Alignment] = None, + def __init__(self, size: Count, byte_order: ByteOrder, alignment: Optional[Alignment] = None, preferred_display_base: DisplayBase = DisplayBase.DECIMAL): if alignment is None: alignment = Alignment(8 if size % 8 == 0 else 1) @@ -156,8 +154,7 @@ class EnumerationFieldTypeMappings(collections.abc.Mapping): class _EnumerationFieldType(_IntegerFieldType): - def __init__(self, size: Count, byte_order: Optional[ByteOrder] = None, - alignment: Optional[Alignment] = None, + def __init__(self, size: Count, byte_order: ByteOrder, alignment: Optional[Alignment] = None, preferred_display_base: DisplayBase = DisplayBase.DECIMAL, mappings: Optional[_EnumFtMappings] = None): super().__init__(size, byte_order, alignment, preferred_display_base) @@ -415,10 +412,12 @@ class StreamTypePacketFeatures: content_size_field_type: _DefaultableUIntFt = DEFAULT_FIELD_TYPE, beginning_time_field_type: _OptDefaultableUIntFt = None, end_time_field_type: _OptDefaultableUIntFt = None, - discarded_events_counter_field_type: _OptDefaultableUIntFt = None): + discarded_events_counter_field_type: _OptDefaultableUIntFt = None, + default_byte_order: Optional[ByteOrder] = None): def get_ft(user_ft: _OptDefaultableUIntFt) -> _OptUIntFt: if user_ft == DEFAULT_FIELD_TYPE: - return UnsignedIntegerFieldType(64) + assert default_byte_order is not None + return UnsignedIntegerFieldType(64, typing.cast(ByteOrder, default_byte_order)) return typing.cast(_OptUIntFt, user_ft) @@ -451,10 +450,12 @@ class StreamTypePacketFeatures: class StreamTypeEventFeatures: def __init__(self, type_id_field_type: _OptDefaultableUIntFt = DEFAULT_FIELD_TYPE, - time_field_type: _OptDefaultableUIntFt = None): + time_field_type: _OptDefaultableUIntFt = None, + default_byte_order: Optional[ByteOrder] = None): def get_ft(user_ft: _OptDefaultableUIntFt) -> _OptUIntFt: if user_ft == DEFAULT_FIELD_TYPE: - return UnsignedIntegerFieldType(64) + assert default_byte_order is not None + return UnsignedIntegerFieldType(64, typing.cast(ByteOrder, default_byte_order)) return typing.cast(_OptUIntFt, user_ft) @@ -472,15 +473,17 @@ class StreamTypeEventFeatures: class StreamTypeFeatures: def __init__(self, packet_features: Optional[StreamTypePacketFeatures] = None, - event_features: Optional[StreamTypeEventFeatures] = None): - self._packet_features = StreamTypePacketFeatures() - - if packet_features is not None: + event_features: Optional[StreamTypeEventFeatures] = None, + default_byte_order: Optional[ByteOrder] = None): + if packet_features is None: + self._packet_features = StreamTypePacketFeatures(default_byte_order=default_byte_order) + else: self._packet_features = packet_features - self._event_features = StreamTypeEventFeatures() - if event_features is not None: + if event_features is None: + self._event_features = StreamTypeEventFeatures(default_byte_order=default_byte_order) + else: self._event_features = event_features @property @@ -496,6 +499,7 @@ class StreamType(_UniqueByName): def __init__(self, name: str, event_types: Set[EventType], default_clock_type: Optional[ClockType] = None, features: Optional[StreamTypeFeatures] = None, + default_feature_field_type_byte_order: Optional[ByteOrder] = None, packet_context_field_type_extra_members: Optional[_StructFtMembers] = None, event_common_context_field_type: _OptStructFt = None): self._id: Optional[Id] = None @@ -509,7 +513,7 @@ class StreamType(_UniqueByName): assert ev_type._id is None ev_type._id = Id(index) - self._set_features(features) + self._set_features(features, default_feature_field_type_byte_order) self._packet_context_field_type_extra_members = StructureFieldTypeMembers({}) if packet_context_field_type_extra_members is not None: @@ -518,7 +522,8 @@ class StreamType(_UniqueByName): self._set_pkt_ctx_ft() self._set_ev_header_ft() - def _set_features(self, features: Optional[StreamTypeFeatures]): + def _set_features(self, features: Optional[StreamTypeFeatures], + default_byte_order: Optional[ByteOrder]): if features is not None: self._features = features return None @@ -535,8 +540,10 @@ class StreamType(_UniqueByName): pkt_end_time_ft = DEFAULT_FIELD_TYPE self._features = StreamTypeFeatures(StreamTypePacketFeatures(beginning_time_field_type=pkt_beginning_time_ft, - end_time_field_type=pkt_end_time_ft), - StreamTypeEventFeatures(time_field_type=ev_time_ft)) + end_time_field_type=pkt_end_time_ft, + default_byte_order=default_byte_order), + StreamTypeEventFeatures(time_field_type=ev_time_ft, + default_byte_order=default_byte_order)) def _set_ft_mapped_clk_type_name(self, ft: Optional[UnsignedIntegerFieldType]): if ft is None: @@ -631,20 +638,33 @@ _OptUuidFt = Optional[Union[str, StaticArrayFieldType]] class TraceTypeFeatures: def __init__(self, magic_field_type: _OptDefaultableUIntFt = DEFAULT_FIELD_TYPE, uuid_field_type: _OptUuidFt = None, - stream_type_id_field_type: _OptDefaultableUIntFt = DEFAULT_FIELD_TYPE): - def get_field_type(user_ft: Optional[Union[str, _FieldType]], default_ft: _FieldType) -> _OptFt: + stream_type_id_field_type: _OptDefaultableUIntFt = DEFAULT_FIELD_TYPE, + default_byte_order: Optional[ByteOrder] = None): + def get_field_type(user_ft: Optional[Union[str, _FieldType]], + create_default_ft: Callable[[], _FieldType]) -> _OptFt: if user_ft == DEFAULT_FIELD_TYPE: - return default_ft + return create_default_ft() return typing.cast(_OptFt, user_ft) - self._magic_field_type = typing.cast(_OptUIntFt, get_field_type(magic_field_type, - UnsignedIntegerFieldType(32))) - self._uuid_field_type = typing.cast(Optional[StaticArrayFieldType], get_field_type(uuid_field_type, - StaticArrayFieldType(Count(16), - UnsignedIntegerFieldType(8)))) - self._stream_type_id_field_type = typing.cast(_OptUIntFt, get_field_type(stream_type_id_field_type, - UnsignedIntegerFieldType(64))) + def create_default_magic_ft(): + assert default_byte_order is not None + return UnsignedIntegerFieldType(32, default_byte_order) + + def create_default_uuid_ft(): + assert default_byte_order is not None + return StaticArrayFieldType(Count(16), UnsignedIntegerFieldType(8, default_byte_order)) + + def create_default_stream_type_id_ft(): + assert default_byte_order is not None + return UnsignedIntegerFieldType(64, default_byte_order) + + self._magic_field_type = typing.cast(_OptUIntFt, get_field_type(magic_field_type, create_default_magic_ft)) + self._uuid_field_type = typing.cast(Optional[StaticArrayFieldType], + get_field_type(uuid_field_type, create_default_uuid_ft)) + self._stream_type_id_field_type = typing.cast(_OptUIntFt, + get_field_type(stream_type_id_field_type, + create_default_stream_type_id_ft)) @property def magic_field_type(self) -> _OptUIntFt: @@ -660,9 +680,9 @@ class TraceTypeFeatures: class TraceType: - def __init__(self, stream_types: Set[StreamType], default_byte_order: ByteOrder, - uuid: _OptUuid = None, features: Optional[TraceTypeFeatures] = None): - self._default_byte_order = default_byte_order + def __init__(self, stream_types: Set[StreamType], uuid: _OptUuid = None, + features: Optional[TraceTypeFeatures] = None, + default_feature_field_type_byte_order: Optional[ByteOrder] = None): self._stream_types = frozenset(stream_types) # assign unique IDs @@ -671,18 +691,19 @@ class TraceType: stream_type._id = Id(index) self._uuid = uuid - self._set_features(features) + self._set_features(features, default_feature_field_type_byte_order) self._set_pkt_header_ft() - self._set_fts_effective_byte_order() - def _set_features(self, features: Optional[TraceTypeFeatures]): + def _set_features(self, features: Optional[TraceTypeFeatures], + default_byte_order: Optional[ByteOrder]): if features is not None: self._features = features return # automatic UUID field type because the trace type has a UUID uuid_ft = None if self._uuid is None else DEFAULT_FIELD_TYPE - self._features = TraceTypeFeatures(uuid_field_type=uuid_ft) + self._features = TraceTypeFeatures(uuid_field_type=uuid_ft, + default_byte_order=default_byte_order) def _set_pkt_header_ft(self): members = collections.OrderedDict() @@ -698,39 +719,6 @@ class TraceType: add_member_if_exists('stream_id', self._features.stream_type_id_field_type) self._pkt_header_ft = StructureFieldType(8, members) - def _set_fts_effective_byte_order(self): - def set_ft_effective_byte_order(ft: _OptFt): - if ft is None: - return - - if isinstance(ft, _BitArrayFieldType): - if ft._byte_order is None: - assert self._default_byte_order is not None - ft._byte_order = self._default_byte_order - elif isinstance(ft, StaticArrayFieldType): - set_ft_effective_byte_order(ft.element_field_type) - elif isinstance(ft, StructureFieldType): - for member in ft.members.values(): - set_ft_effective_byte_order(member.field_type) - - # packet header field type - set_ft_effective_byte_order(self._pkt_header_ft) - - # stream type field types - for stream_type in self._stream_types: - set_ft_effective_byte_order(stream_type._pkt_ctx_ft) - set_ft_effective_byte_order(stream_type._ev_header_ft) - set_ft_effective_byte_order(stream_type._event_common_context_field_type) - - # event type field types - for ev_type in stream_type.event_types: - set_ft_effective_byte_order(ev_type._specific_context_field_type) - set_ft_effective_byte_order(ev_type._payload_field_type) - - @property - def default_byte_order(self) -> ByteOrder: - return self._default_byte_order - @property def uuid(self) -> _OptUuid: return self._uuid @@ -895,9 +883,11 @@ class ConfigurationOptions: class Configuration: - def __init__(self, trace: Trace, options: Optional[ConfigurationOptions] = None): + def __init__(self, trace: Trace, target_byte_order: ByteOrder, + options: Optional[ConfigurationOptions] = None): self._trace = trace self._options = ConfigurationOptions() + self._target_byte_order = target_byte_order if options is not None: self._options = options @@ -917,6 +907,10 @@ class Configuration: def trace(self) -> Trace: return self._trace + @property + def target_byte_order(self): + return self._target_byte_order + @property def options(self) -> ConfigurationOptions: return self._options diff --git a/barectf/config_parse_v2.py b/barectf/config_parse_v2.py index 9118562..bd91042 100644 --- a/barectf/config_parse_v2.py +++ b/barectf/config_parse_v2.py @@ -483,8 +483,9 @@ class _Parser(config_parse_common._Parser): v3_trace_type_node: _MapNode = collections.OrderedDict() v2_trace_node = v2_meta_node['trace'] - # rename `byte-order` property to `$default-byte-order` - _copy_prop_if_exists(v3_trace_type_node, v2_trace_node, 'byte-order', '$default-byte-order') + # Move `byte-order` property to root node's `target-byte-order` + # property. + self._root_node['target-byte-order'] = v2_trace_node['byte-order'] # copy `uuid` property _copy_prop_if_exists(v3_trace_type_node, v2_trace_node, 'uuid') diff --git a/barectf/config_parse_v3.py b/barectf/config_parse_v3.py index 8e0e7ff..11dd216 100644 --- a/barectf/config_parse_v3.py +++ b/barectf/config_parse_v3.py @@ -462,8 +462,10 @@ class _Parser(barectf_config_parse_common._Parser): pkt_content_size_ft, pkt_beginning_time_ft, pkt_end_time_ft, - pkt_discarded_events_counter_ft) - ev_features = barectf_config.StreamTypeEventFeatures(ev_type_id_ft, ev_time_ft) + pkt_discarded_events_counter_ft, + default_byte_order=self._target_byte_order) + ev_features = barectf_config.StreamTypeEventFeatures(ev_type_id_ft, ev_time_ft, + default_byte_order=self._target_byte_order) features = barectf_config.StreamTypeFeatures(pkt_features, ev_features) # create packet context (structure) field type extra members @@ -508,7 +510,7 @@ class _Parser(barectf_config_parse_common._Parser): ev_types.add(self._create_ev_type(ev_name, ev_type_node, ev_header_common_ctx_member_count)) # create stream type - return barectf_config.StreamType(name, ev_types, def_clk_type, features, + return barectf_config.StreamType(name, ev_types, def_clk_type, features, None, pkt_ctx_ft_extra_members, self._try_create_struct_ft(stream_type_node, ev_common_ctx_ft_prop_name)) @@ -604,7 +606,8 @@ class _Parser(barectf_config_parse_common._Parser): except _ConfigurationParseError as exc: _append_error_ctx(exc, '`$features` property') - features = barectf_config.TraceTypeFeatures(magic_ft, uuid_ft, stream_type_id_ft) + features = barectf_config.TraceTypeFeatures(magic_ft, uuid_ft, stream_type_id_ft, + default_byte_order=self._target_byte_order) # create stream types stream_types = set() @@ -613,8 +616,7 @@ class _Parser(barectf_config_parse_common._Parser): stream_types.add(self._create_stream_type(stream_name, stream_type_node)) # create trace type - return barectf_config.TraceType(stream_types, self._default_byte_order, - trace_type_uuid, features) + return barectf_config.TraceType(stream_types, trace_type_uuid, features) except _ConfigurationParseError as exc: _append_error_ctx(exc, 'Trace type') @@ -712,7 +714,7 @@ class _Parser(barectf_config_parse_common._Parser): opts = barectf_config.ConfigurationOptions(cg_opts) # create configuration - self._config = barectf_config.Configuration(trace, opts) + self._config = barectf_config.Configuration(trace, self._target_byte_order, opts) # Expands the field type aliases found in the trace type node. # @@ -1051,7 +1053,7 @@ class _Parser(barectf_config_parse_common._Parser): # # 2. Chooses a specific `class` property value. # - # 3. Chooses a specific `byte-order`/`$default-byte-order` property + # 3. Chooses a specific `byte-order`/`target-byte-order` property # value. # # 4. Chooses a specific `preferred-display-base` property value. @@ -1069,10 +1071,7 @@ class _Parser(barectf_config_parse_common._Parser): trace_node = self.config_node['trace'] trace_type_node = trace_node['type'] - prop_name = '$default-byte-order' - - if prop_name in trace_type_node and type(trace_type_node[prop_name]) is str: - normalize_byte_order_prop(trace_type_node, prop_name) + normalize_byte_order_prop(self.config_node, 'target-byte-order') for parent_node, key in self._trace_type_props(): node = parent_node[key] @@ -1118,9 +1117,9 @@ class _Parser(barectf_config_parse_common._Parser): if node is None: del trace_node[prop_name] - # Substitutes missing/`None` `byte-order` properties with the trace - # type node's default byte order (`$default-byte-order` property), - # if any. + # Substitutes missing/`None` `byte-order` properties with the + # configuration node's target byte order (`target-byte-order` + # property). def _sub_ft_nodes_byte_order(self): ba_ft_class_names = { 'unsigned-integer', @@ -1144,11 +1143,7 @@ class _Parser(barectf_config_parse_common._Parser): byte_order_node = ft_node.get(prop_name) if byte_order_node is None: - if default_byte_order_node is None: - raise _ConfigurationParseError(f'`{key}` property`', - '`byte-order` property is not set or null, but trace type has no `$default-byte-order` property') - - ft_node[prop_name] = default_byte_order_node + ft_node[prop_name] = self._target_byte_order_node members_node = ft_node.get('members') @@ -1166,13 +1161,8 @@ class _Parser(barectf_config_parse_common._Parser): except _ConfigurationParseError as exc: _append_error_ctx(exc, f'Structure field type member `{member_name}`') - default_byte_order_node = self._trace_type_node.get('$default-byte-order') - - self._default_byte_order = None - - if default_byte_order_node is not None: - self._default_byte_order = self._byte_order_from_node(default_byte_order_node) - + self._target_byte_order_node = self.config_node['target-byte-order'] + self._target_byte_order = self._byte_order_from_node(self._target_byte_order_node) features_prop_name = '$features' features_node = self._trace_type_node.get(features_prop_name) @@ -1398,9 +1388,6 @@ class _Parser(barectf_config_parse_common._Parser): # Set `byte-order` properties of bit array field type nodes # missing one. - # - # This process also removes the `$default-byte-order` property - # from the trace type node. self._sub_ft_nodes_byte_order() # Create a barectf configuration object from the configuration diff --git a/barectf/schemas/config/3/config.yaml b/barectf/schemas/config/3/config.yaml index 6041099..306fed4 100644 --- a/barectf/schemas/config/3/config.yaml +++ b/barectf/schemas/config/3/config.yaml @@ -93,8 +93,6 @@ definitions: properties: uuid: $ref: https://barectf.org/schemas/config/common/common.json#/definitions/opt-trace-type-uuid-prop - $default-byte-order: - $ref: https://barectf.org/schemas/config/common/common.json#/definitions/opt-byte-order-prop $features: if: type: object @@ -158,7 +156,6 @@ definitions: minProperties: 1 required: - stream-types - - $default-byte-order additionalProperties: false clock-type: title: Clock type object @@ -298,8 +295,11 @@ properties: additionalProperties: false additionalProperties: false additionalProperties: false + target-byte-order: + $ref: https://barectf.org/schemas/config/common/common.json#/definitions/byte-order-prop trace: $ref: '#/definitions/trace' required: - trace + - target-byte-order additionalProperties: false diff --git a/barectf/templates/c/bitfield.h.j2 b/barectf/templates/c/bitfield.h.j2 index d9b9b23..cdea278 100644 --- a/barectf/templates/c/bitfield.h.j2 +++ b/barectf/templates/c/bitfield.h.j2 @@ -39,9 +39,9 @@ # define _CAST_PTR(_type, _value) ((void *) (_value)) #endif -{% set def_bo = cfg.trace.type.default_byte_order %} -{% set def_bo_str = 'LITTLE_ENDIAN' if def_bo == barectf_config.ByteOrder.LITTLE_ENDIAN else 'BIG_ENDIAN' %} -#define _BYTE_ORDER {{ def_bo_str }} +{% set tgt_bo = cfg.target_byte_order %} +{% set tgt_bo_str = 'LITTLE_ENDIAN' if tgt_bo == barectf_config.ByteOrder.LITTLE_ENDIAN else 'BIG_ENDIAN' %} +#define _BYTE_ORDER {{ tgt_bo_str }} /* We can't shift a int from 32 bit, >> 32 and << 32 on int is undefined */ #define _bt_piecewise_rshift(_vtype, _v, _shift) \ diff --git a/barectf/templates/metadata/metadata.j2 b/barectf/templates/metadata/metadata.j2 index 487e6a7..68331ca 100644 --- a/barectf/templates/metadata/metadata.j2 +++ b/barectf/templates/metadata/metadata.j2 @@ -39,7 +39,7 @@ trace { major = 1; minor = 8; - byte_order = {{ cfg.trace.type.default_byte_order | bo_str }}; + byte_order = {{ cfg.target_byte_order | bo_str }}; {% if cfg.trace.type.uuid %} uuid = "{{ cfg.trace.type.uuid }}"; {% endif %} -- 2.34.1