Simon Marchi [Thu, 23 May 2019 15:59:39 +0000 (11:59 -0400)]
bt2: Adapt test_trace_collection_message_iterator.py and make it pass
Update test_trace_collection_message_iterator.py and
trace_collection_message_iterator.py to work with the current Babeltrace
API.
It is no longer possible to pass a "message type" filter to a message
iterator. This means that the TraceCollectionMessageIterator objects in
the test now return all messages, where we previously filtered to only
have those of type EventMessage. The new count_msg_by_type function
helps to get around this problem.
In trace_collection_message_iterator.py, there are just a few changes to
account for the API changes. The src.ctf.fs component now takes a
"paths" parameter instead of "path". It is no longer possible to get a
component from a port, instead we use the component passed as a
parameter in the callbacks.
The change in the way we handle the timestamps in _create_trimmer is due
to the fact that if we pass a single integer, it is interpreted as
seconds. Since we have a number of nanoseconds, it is misinterpreted.
Instead, pass a string in the seconds.nanoseconds form.
Change-Id: Iddd1666008d06e49843932dc74ee51ac772d001b Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com> Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1328 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
which they can then access using the user_data property of
_UserComponentPort:
print(self_port.user_data) # {'foo': 23}
We confine the user data under the user_data property to avoid
clashes with future methods and properties that we might add to
_UserComponentPort or its subclasses.
A new "in" typemap is created for the "add port" functions to pass the
PyObject* representing the user data Python object to the creation
function. Without the typemap, SWIG complains that the passed value (a
PyObject *) is not of the right type (it expects a void *):
TypeError: in method 'self_component_source_add_output_port', argument 3 of type 'void *'
If the port is created successfully, it now owns a reference to this
Python object. We must reflect that in the Python object's refcount,
this is done through an "argout" typemap. In this typemap, we need to
check the return value of the function we called.
When fetching the user data of a port, we use an "out" typemap to
increment the refcount of the returned value. This is because a Python
method that returns an object must return a new reference that is
transferred to the caller.
Change-Id: I0b83454a81e71bd7c2fe9449c7fc65c09f18fcf4 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1359 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Mon, 3 Jun 2019 20:48:45 +0000 (16:48 -0400)]
bt2: Adapt test_message_iterator.py and make it pass
Update test_message_iterator.py to work with the current Babeltrace API.
Most changes are related to the fact that it is not possible anymore to
create most objects in isolation, everything is derived from a trace
class. Message iterators are created in the _graph_is_configured
callback rather than _port_connected. Because of this, we now need to
call graph.run() in the tests to get _graph_is_configured called.
I noticed that the component method of _SelfPortInputMessageIterator was
a bit problematic, as we don't know the type of the component at that
point. Returning the right type of component (the right Python class,
wrapping the right type of Swig pointer) would require a bit of dirty
work. I am not sure this is even useful, so I've just removed it and
marked the tests as skipped / commented them out (they can be removed if
we rule this is indeed unnecessary).
An important change is the addition the self_output_port parameter in
the constructor of _UserMessageIterator (which the user is expected to
override). This parameter is how the iterator is supposed to know what
it's supposed to iterate on (if the component has multiple ports).
There are no other changes in message_iterator.py, since everything had
to be done already for test_message.py to pass.
Change-Id: I574d812ed81d9628a8e4a2a2dd5c593c4d730c95 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1326 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Mon, 3 Jun 2019 22:28:10 +0000 (18:28 -0400)]
bt2: Adapt test_message.py and make it pass
Update test_message.py to match the current Babeltrace API. Update
message.py to also match the current API and make the test pass.
message_iterator.py is also updated, as it is required by the test, but
just the essential changes are done.
The test changes substantially, since it is no longer possible to create
a message by itself, they are created from a component message iterator,
which requires quite a bit of setup. Also, it would be invalid to emit
an "event message" without having first emitted a "stream beginning
message", "packet beginning message", and so on. So testing the message
types in isolation would required quite a lot of boilerplate. This is
why all messages types are now tested in the same test. We have a
source that emits all message types at least once. The sink on the
other side verifies that what it receives from the source is what we
expect.
Everything related to equality, copy and deep copy is removed.
Change-Id: Ibe6146049cc1065df82dde821a42f10d4e5bba27 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1324 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
Simon Marchi [Tue, 21 May 2019 21:01:59 +0000 (17:01 -0400)]
bt2: Adapt test_field.py and make it pass
Update test_field.py and field.py to match the current API of
Babeltrace.
In the test, most changes are related to the fact that field classes and
fields are created ultimately from a trace class. A bunch of helpers
are added at the top, which allow to easily create a field of a certain
type without repeating the boilerplate.
Anything related to copy and deepcopy is removed. However, the test
_TestNumericField._test_binop_lhs_value_same relies on copy support, so
I am not too sure what to do with it. I have currently marked it as
skipped, but we should either adapt it or remove it.
A concept that disappeared (or rather, is not exposed by the public API)
is whether a field is set or not, and the ability to reset a field's
value. All tests and code related to that are removed.
Change-Id: I27f1ee6a3a2152232556d9d9c301de8411189a2c Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1323 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 29 May 2019 16:08:29 +0000 (12:08 -0400)]
bt2: Adapt test_field_class.py and make it pass
Update the test_field_class.py test to match the current API of
Babeltrace. Update field_class.py and some others to make that test
pass.
The main change in test_field_class.py is that field classes are now
created from an existing trace class (tc.create_foo_field_class())
rather than by constructor (FooFieldClass()). Everything related to
copy, deep copy and equality is removed. Everything related to
structure alignment, byte order and encoding is removed, as those were
ctf concepts that were removed from Babeltrace's trace-ir API.
All specific field class tests try to follow the same pattern. The
test_create_default test method verifies the properties of a specific
field class create with passing as few parameters as possible. Then,
for each possible creation parameter, we verify a case that works and
some problematic cases such as invalid value or invalid type. More
tests are done for specific field classes that support additional
features, such as container field classes.
The support for modifying field class properties after they have been
created is removed. All characteristics of a field class must passed
during construction (this doesn't apply to fields of composite field
classes).
A new concept of "field path" was introduced in the library, which is
useful in the context of dynamically-sized arrays and variants. Support
is therefore added to obtain the "field path" to a variant's selector
field or dynamic array length field.
Change-Id: Ia998119fcf7c61ef5904fbe72baa36a2838d5780 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1319 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Mon, 3 Jun 2019 20:45:56 +0000 (16:45 -0400)]
bt2: Adapt test_graph.py and make it pass
This patch updates test_graph.py and graph.py to work with the current
BT API.
An important change is that now, the port added and ports connected
listeners can report their failures up the stack. Therefore, if a
Python listener raises an exception, we now report it as an error. Some
tests are added for this.
were removed, since the library no longer returns an error in these
situations (they are just verified by precondition check). However, a
test was added to check that when the graph gets cancelled during
execution, a bt2.GraphCanceled gets raised.
Some changes in message.py, message_iterator.py and port.py are
necessary to support the test. They are not complete, but they are
representative of what will come after.
Any reference to ports being removed or disconnected were removed, since
it's not longer possible to remove or disconnect a port.
Change-Id: Ie8f11553f34208bb58242d9108efc361acba3c18 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1317 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
Simon Marchi [Thu, 16 May 2019 16:10:18 +0000 (12:10 -0400)]
lib: Make graph listeners return an error status
Currently, if a graph listener fails for some reason (for example, an
uncaught exception in Python), no error is reported to the initial
caller, so it has no way to handle the failure properly. If a listener
fails, we can't really trust that the execution will proceed as
intended, so it is likely that the caller wants to exit with an error
too.
All the listener typedefs are updated to return a new type,
bt_graph_listener_status. If any listener fails, the graph is put in
"faulty" state and we return an error up the stack.
If a listener fails, the action is not rolled back. This means that if
a port added listener fails, the port that was added is not removed. If
a ports connected listener fails, the connection is not undone.
A few call sites needed to be updated. An interesting one is in
cli/babeltrace.c. Where we previously aborted, we can now return an
error and exit cleanly.
The Python bindings is another user of the listener API. I have
modified the functions in native_bt_graph.i to keep the code building,
but the errors are currently not propagated from the Python callback.
This will be done in a subsequent patch.
Change-Id: I115773c405162f7b1c617cf4a8302b980315e14d Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1316 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
- Add an assert to verify that there is at least a sink component in
the graph when it's configured.
- Remove the equivalent in the Python bindings, including the test in
test_graph.py, even though test_graph.py is not functional at the
moment.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I8d6ce79a2c77527908f500017bb346b2c2d94c63
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1315
CI-Build: Simon Marchi <simon.marchi@efficios.com> Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Tue, 14 May 2019 19:38:58 +0000 (15:38 -0400)]
bt2: Adapt test_packet.py and make it pass
This patch updates test_packet.py and packet.py to work with the current
API.
In both files, everything about equality, copy, deep copy and header
fields is removed. The setter for context_field is also removed.
One side-effect change (required for the test) is in the
Field.field_class property. Previously, when creating a Field object
wrapping a bt_field pointer, we would create the corresponding
FieldClass object and store it in Field._field_class. Instead, we can
compute it as needed in the Field.field_class property, I don't really
see any problem with that.
Change-Id: I509b3bd272fb323ed6df7de47d21c31b8aedc72f Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1301 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Mon, 3 Jun 2019 22:41:45 +0000 (18:41 -0400)]
bt2: Adapt test_clock_class.py and make it pass
This patch adapts test_clock_class to the current BT API and changes
what's needed to make it pass.
One change in test_clock_class is that clock classes need to be created
from self components now, so it requires a bit more boilerplate.
Everthing related to equality, copy and deepcopy is removed from
ClockClass. However, it remains possible to test a _ClockSnapshot for
equality against an integer. I thoough it would be useful to support
other relational operations (<, <=, >, >=) for clock snapshots, so I
added support for them using functools.total_ordering.
The constructors for both _ClockSnapshot and ClockClass are removed, as
the user never directly creates those objects anymore. Clock snapshots
are obtained from messages, while clock classes are created using the
_create_clock_class method of a component.
As in previous patches, the public setters are removed, as we only
support setting properies when creating an object.
Change-Id: I7228b32530f98811cb512243469ae7d0d61a9da1 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1299 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
Simon Marchi [Mon, 13 May 2019 17:43:38 +0000 (13:43 -0400)]
bt2: Adapt test_event.py and make it pass
This patch changes test_event.py to work with the current BT API and
adapts event.py accordingly to make it pass. Since this is a central
piece of the API (it has many related concepts), a few other files are
modified as well, just enough to support the test. For example, events
are no longer created directly, instead we need to instantiate an
EventMessage. This requires to get the foundation for messages
working. Even though the changes in message_iterator (the "create"
methods to create various messages) are not complete, it gives an idea
of what is to come, so it would be good to get some comments on it right
now.
In event.py, things related to clock snapshots, header fields, equality,
copy and deep copy are removed.
In test_event.py, we now need a bigger setup, since events are created
from event messages, which are created from output iterators, which are
created from components.
In object.py, UniqueObject now needs to record its _owner_get_ref
callback, so that if we get a second UniqueObject from an existing
UniqueObject (such as a sub-field from a struct field), the callback can
be passed to the new object.
Change-Id: I72309826a61245b0fe4fdd9a638ddee3689c5921 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1298 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
Simon Marchi [Mon, 13 May 2019 18:01:22 +0000 (14:01 -0400)]
Fix: lib: usage of output port message iterator
Fix some issues relate to output port message iterator usage.
The first encountered issue when trying to create one such iterator is:
05-13 13:51:20.845 14995 14995 F MSG-ITER bt_self_component_port_input_message_iterator_create@iterator.c:444 Graph is not configured: addr=0x612000000640, is-canceled=0, can-consume=0, config-state=BT_GRAPH_CONFIGURATION_STATE_CONFIGURING, comp-count=2, conn-count=1, en-pool-size=0, en-pool-cap=0, pbn-pool-size=0, pbn-pool-cap=0, pen-pool-size=0, pen-pool-cap=0
The problem is that the colander is trying to create an input iterator on its
input port in the "port connected" callback, which is before the graph is
configured. This is not a valid thing to do anymore, so change it to create
the input iterator during the "graph is configured" step.
The next issue we face is then:
05-13 14:00:01.903 21344 21344 W COLANDER colander_consume@component-class-sink-colander.c:142 Trying to consume without an upstream message iterator: comp-addr=0x60c0000028c0, c
omp-name="colander-36ac3409-b1a8-4d60-ab1f-4fdf341a8fb1", comp-class-type=BT_COMPONENT_CLASS_TYPE_SINK, comp-class-name="colander", comp-class-partial-descr="", comp-class-is-frozen=1, comp-input-port-count=1, comp-output-port-count=0
This is because the "graph is configured" callback is never invoked,
the reason being that bt_port_output_message_iterator_create just sets
the graph's config state field directly, rather than calling
bt_graph_configure.
Change-Id: Idbf873912ea5d116f0b011639177e57aa78e6759 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1297 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Mon, 13 May 2019 17:10:40 +0000 (13:10 -0400)]
bt2: Adapt test_event_class.py and make it pass
This patch updates event_class.py to be more in line with the current
API and updates test_event_class.py accordingly.
Everything related to equality, copy and deepcopy is removed. An event
class is always created from an existing stream class, so it is not
longer possible to create an event class out of thin air. We only
support passing parameters to the event class when creating it, not
assigning them afterwards.
Change-Id: I2f1ad9f98f25e3e2dfdea511a1410529d014745b Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1296 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Fri, 31 May 2019 14:31:35 +0000 (10:31 -0400)]
bt2: Adapt test_stream.py and make it pass
This patch updates test_stream.py and stream.py. It remove everything
related to equality, copy and deepcopy, as in the previous patches.
Since we don't want to share code between the CTF writer and the
trace-ir objects, the _StreamBase class is removed, and ctf_writer.py is
updated just so import still works.
Change-Id: I72e80694e0c8b401a86ce23d94b6e064afb08ac2 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com> Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1292 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 29 May 2019 14:28:55 +0000 (10:28 -0400)]
bt2: Adapt test_stream_class.py and make it pass
This patch updates test_stream_class.py quite a bit. In particular, it
removes everything related to equality, copy and deep copy. Since we
don't support (at least, for the moment) assigning stream class
properties after it has been created, most "assign" tests have become
"create" tests.
Stream class objects in Python are created using
TraceClass.create_stream_class. This method has been introduced
previously to support TraceClass tests, but this patch now adds the
missing options. Some field class creation methods are added to
TraceClass, just enough to support test_stream_class.
Changes in stream_class.py are mostly to adjust to past API changes.
The assigns_automatic_event_class_id property is new, everything related
to "header" field classes is removed. Everything related to equality,
copy and deep copy is also removed. The __call__ function is removed,
as it's not how create streams anymore (we use Trace.create_stream).
A new concept in the lib is that packet beginning/end messages may or
may not have a default clock snapshot, even if the stream has a default
clock class. An option to TraceClass.create_stream_class is added for
this.
Changes to other files are there to support the tests, and not meant to
be complete.
Change-Id: Idd182d3f512f4f5772816fee0c04e2b39fd163f2 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1291 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Thu, 9 May 2019 19:28:15 +0000 (15:28 -0400)]
bt2: Adapt test_trace.py and make it pass
test_trace.py is heavily modified, because most of the responsibilities
of Trace have been moved to TraceClass. A number of concepts, such as
packet headers, also disappeared.
Similarly, trace.py loses a lot of weight. Note that copy and equality
operations are removed, since we ruled that they were complex but not
particularly useful.
A trace implements abc.Mapping, and maps stream ids to stream objects.
A trace is created by "calling" a trace class, hence the addition of the
__call__ method to TraceClass.
The test_trace test creates some streams and requires to control the ids
of those streams, so the create_stream_class method and StreamClass type
are enhanced to support that.
Change-Id: If23089abfbcbe5f0064069f93bd91126f1549b62 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1290 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 8 May 2019 20:55:37 +0000 (16:55 -0400)]
bt2: Add bindings for trace classes
This patch adds a Python wrapper for the bt_trace_class concept.
Trace classes in Python are created using the _create_trace_class on a
_UserComponent.
A TraceClass is seen as a mapping of stream class ids to stream classes.
This means that with trace class `tc`, it's possible to access its
stream classes by id using `tc[id]`. It's also possible to iterate on
`tc` to iterate on stream class ids.
A brand new test is also added, along with some test utils that should
be useful in future tests as well.
It is not feature-complete yet, for example there is no way to
instantiate a trace using this trace class, this will come in a
subsequent patch (that will handle test_trace.py). It is possible to
create stream classes in a trace class, but not with every possible
option, just enough to support the trace class tests.
Change-Id: Ib9f29d7ebc21b6ade19e9ffbad3f4b85e790321d Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1283 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Wed, 8 May 2019 21:07:22 +0000 (17:07 -0400)]
Fix: lib: prevent infinite recursion when destroying trace classes and traces
When implementing support for trace and trace class destruction
listeners in Python, we hit a problem where we enter infinite recursion
(until the process crashes).
When the refcount of a trace class reaches 0, the destruction listeners
for that trace class are called. The Python listener creates a
TraceClass object, which acquires a new reference to the trace class
being destroyed. When the listener is done, it drops that reference.
This causes the refcount to reach 0 again, and the lib to call all the
destruction listeners again.
The same happens for traces.
To prevent it, this patch makes the lib increment the refcount by one
before calling all destruction listeners.
Change-Id: Ib7e7bd0428c2a505fb25ad0aa80150b518da4c4e Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1282 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Fri, 31 May 2019 15:24:29 +0000 (11:24 -0400)]
bt2: Adapt test_connection.py and make it pass
The test_connection test is adapted to account for:
- Connections can't be in an ended state anymore.
- We don't need to define equality between connections.
- Private connections don't exist anymore.
The bindings are also simplified by the fact that private connections
don't exist anymore, and connections can't end. Otherwise, changes are
straightforward to adapt to the existing API.
Change-Id: I52cb89512df1e8cb0cd88670c6d63e68287556c3 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1280 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
Simon Marchi [Wed, 8 May 2019 18:00:05 +0000 (14:00 -0400)]
bt2: Adapt test_port.py and make it pass
This patch does the essential to make test_port.py pass.
Everything related to equality between ports was removed from
test_port.py, as we decided that it was not particularly useful.
Everything related to disconnection of ports and removal of ports from
components was also removed, since these concepts don't exist in
Babeltrace anymore.
I have also removed the possibility of getting a component from a port.
This was the only place where an explicit downcast from bt_component to
the specific component type would have been necessary. Instead, we have
concluded that every time you have access to the port, you already have
access to its component, already with the right type.
The possibility to create an output message iterator from an output port
is also removed, that operation will be done from a graph instead.
The other notable changes in the bindings are:
- Graph: Change add_sink_component to add_component, and handle the
various component types.
- Port: Simple adaptations to the current API, using the now famous pattern
of _as_*_ptr static methods.
- Component: Adjust how ports are listed and iterated on to account for
changes in the API.
Change-Id: Iee42bb4f8508e08d7f2b2cca451cae05d52a0ed0 Signed-off-by: Simon Marchi <simon.marchi@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1279 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
Philippe Proulx [Fri, 31 May 2019 19:08:37 +0000 (15:08 -0400)]
lib: make packet beginning/end default CS optional
This patch makes it possible to create packet beginning and end messages
using bt_message_packet_beginning_create() and
bt_message_packet_end_create(), that is, without default clock
snapshots, even if the corresponding stream class has a default clock
snapshot. This was considered like a precondition break before this
patch.
Rationale
=========
The main use case for this is to support sources which do not have a
concept of beginning and end times for their packets, but which have
times for their events. Currently, such sources need to make up default
clock snapshot values for those packet messages.
This can be the case of `src.ctf.fs` which, when a given stream class's
packet context does not contain the `timestamp_begin` and
`timestamp_end` members, uses the current default clock's value during
the stream decoding process, except for the beginning time of the first
packet which is set to 0. This is problematic because it means the
stream is active from clock value 0 to the first event, which is
probably not true. A solution would be to use the first event's time in
this scenario, but:
* Several packets can be empty at the beginning, making the task
difficult because we need to queue pseudo-messages until we have the
first event's time and then create the real messages with this value.
* All packets can be empty, so no specific time value is available for
the whole stream.
Although they are weird scenarios, they can still happen. Using 0 for
the first packet's beginning time is just wrong in those cases.
Other sources which do not need the concept of a packet can use this
new feature too: `src.text.dmesg` is one of them.
Library changes
===============
As with other features of stream-related objects, the default clock
snapshot property is not optional _per_ packet beginning/end message:
for the streams of a given stream class, either all packet beginning
and/or end messages have a default clock snapshot, or they don't.
This is controlled by a new stream class flag of which the API is:
By default, a stream class has no default clock class, so its packets
are also known to have no beginning/end default clock snapshots. When
you call
bt_stream_class_set_packets_have_default_beginning_clock_snapshot() or
bt_stream_class_set_packets_have_default_end_clock_snapshot(), you must
have called bt_stream_class_set_default_clock_class() first. This means
you can check if a packet beginning message, for example, has a default
clock snapshot with
bt_stream_class_set_packets_have_default_beginning_clock_snapshot()
without also checking
bt_stream_class_borrow_default_clock_class_const().
It is required that you have called
bt_stream_class_set_packets_have_default_beginning_clock_snapshot() in
order to use
bt_message_packet_beginning_create_with_default_clock_snapshot() for the
corresponding stream class. Same thing for
bt_stream_class_set_packets_have_default_end_clock_snapshot() and
bt_message_packet_end_create_with_default_clock_snapshot().
This is all validated in developer mode.
Plugin changes
==============
Component classes are changed as such:
`src.ctf.fs`:
`src.ctf.lttng-live`:
In order to separate concerns into different patches, the procedure
is not changed in this component class: all packet beginning/end
messages have a default clock snapshot when the corresponding stream
class has a default clock class, even if the stream class's packet
context does not contain the `timestamp_begin` and `timestamp_end`
members.
Another patch will fix this issue.
`src.text.dmesg`:
Like `src.ctf.fs`, the procedure is not changed: all packet
beginning/end messages have a default clock snapshot when the
corresponding stream class has a default clock class.
The simplification change will be done in another patch.
`flt.utils.trimmer`:
Stream classes without a default clock class are already not
supported. With this patch, packet beginning/end messages without a
default clock snapshot are not supported.
The support will be added by another patch.
`flt.utils.muxer`:
When a packet beginning/end message does not have a default clock
snapshot, the iterator's last time is used to sort the messages.
This is similar to other messages without a default clock snapshot.
`flt.lttng-utils.debug-info`:
Stream class properties and packet beginning/end messages are copied
considering the new feature.
`sink.ctf.fs`:
Packet beginning/end without a default clock snapshot, but with a
stream class which has a default clock class, are not supported.
Support will be added by another patch.
All the tests still pass because, even though some filter/sink
components do not fully support the new feature, the existing sources do
not use it yet.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I7183bfe8f954235c7f54195b101f781b176ab733
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1361 Tested-by: jenkins
Philippe Proulx [Fri, 31 May 2019 12:33:14 +0000 (08:33 -0400)]
lib: remove "unknown clock snapshot" concept
The team decided that, because this "unknown clock snapshot" feature has
no current use case and that CTF 1.8 cannot use it, we can wait until we
really need it to implement it.
The feature was only partially implemented: you could know that a given
clock snapshot is unknown, but you could not make one unknown. This
asymmetry was so that current consumers (filters and sinks) could
support unknown clock snapshots without taking the development time to
make the producers (sources and filters) create them. This is another
proof that the feature is not needed now.
In the library, all bt_*_borrow_*_clock_snapshot() functions, except for
bt_message_stream_activity_beginning_borrow_default_clock_snapshot_const()
and
bt_message_stream_activity_end_borrow_default_clock_snapshot_const(),
now return the borrowed clock snapshot object instead of returning a
clock snapshot status (known or unknown). Python bindings and plugins
are adapted accordingly.
The `BT_MESSAGE_STREAM_ACTIVITY_CLOCK_SNAPSHOT_STATE_UNKNOWN` state
still exists for stream activity messages because it can be useful to
set that the first stream activity beginning or the last stream activity
end is unknown. `src.ctf.fs` is one user of this state.
Also, the bt_stream_class_default_clock_is_always_known() function is
removed because the default clock is always known. This function
unconditionally returned `BT_TRUE` anyway. Python bindings and plugins
are adapted accordingly.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: I2665f713917b14efdecb181bef16c199ddd9eb81
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1358 Tested-by: jenkins Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 31 May 2019 11:56:44 +0000 (07:56 -0400)]
bt2: always use staticmethod() with native function class attributes
This is a recurrent pattern in the Python bindings:
class Something(_SomeBase):
_set_value = native_bt.something_set_value
Then template methods in `_SomeBase` can use this class attribute by
name within the template.
This function needs to be a static method, because template methods do:
self._set_value(...)
and native_bt.something_set_value() must not receive `self` as its first
argument. For some reason, when you set a class attribute to a native
function (defined within an extension module), it is automatically a
static method, so you don't need to specify it manually. It is not the
case for any other Python function.
With SWIG 3, the generated wrapper code in `native_bt.py` looks like:
There's a defined Python function, but then the same name is reassigned
to the native function itself (`_native_bt.value_unsigned_integer_set`).
I didn't investigate why SWIG does that; it could be an optimization,
but then I don't get why the Python wrapper is created in the first
place. This means that, with:
class UnsignedIntegerValue(_IntegerValue):
...
_set_value = native_bt.value_unsigned_integer_set
...
UnsignedIntegerValue._set_value() is a static method. However, with
SWIG 4, the generated wrapper looks like this:
Now, UnsignedIntegerValue._set_value() is not a static method anymore,
so calling it with `self._set_value()` gives the expected:
TypeError: value_unsigned_integer_set() takes 2 positional arguments
but 3 were given
Instead of trying to make SWIG 4 generate this native function
assignment again, I prefer to be more explicit/clean and use
staticmethod() to force the function into a static method.
From https://docs.python.org/3/library/functions.html#staticmethod,
the documentation also suggests this:
> Like all decorators, it is also possible to call staticmethod as a
> regular function and do something with its result. This is needed in
> some cases where you need a reference to a function from a class body
> and you want to avoid the automatic transformation to instance method.
> For these cases, use this idiom:
>
> class C:
> builtin_open = staticmethod(open)
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ic51397e34132adc78fddc3105878080a8bf74ff9
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1357 Tested-by: jenkins Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
Philippe Proulx [Fri, 31 May 2019 11:07:04 +0000 (07:07 -0400)]
bindings/python/bt2/setup.py.in: put native module in `bt2` package
In SWIG 3, the native module (`_native_bt.*.so`, which they call the
"C/C++ module") can be found in two locations by the Python module
(`native_bt.py`). From
http://www.swig.org/Doc3.0/SWIGDocumentation.html#Python_package_search:
> The pure Python module needs to load the C/C++ module in order to link
> to the wrapped C/C++ methods. To do this it must make some assumptions
> about what package the C/C++ module may be located in. The approach
> the pure Python module uses to find the C/C++ module is as follows:
>
> 1. The pure Python module, foo.py, tries to load the C/C++ module,
> _foo, from the same package foo.py is located in. The package name
> is determined from the __name__ attribute given to foo.py by the
> Python loader that imported foo.py. If foo.py is not in a package
> then _foo is loaded as a global module.
>
> 2. If the above import of _foo results in an ImportError being thrown,
> then foo.py makes a final attempt to load _foo as a global module.
In other words, `native_bt.py` tries:
from . import _native_bt
If this fails:
import _native_bt
Currently, we're using the second location (global module).
In SWIG 4.0.0, released 27 april 2019, things changed. From
http://www.swig.org/Doc4.0/SWIGDocumentation.html#Python_package_search:
> The pure Python module needs to load the C/C++ module in order to call
> the wrapped C/C++ methods. To do this it must make some assumptions
> about the location of the C/C++ module. There are two configurations
> that are supported by default.
>
> 1. Both modules in the same package
>
> 2. Both modules are global
and:
> Compatibility Note: Versions of SWIG prior to SWIG-4.0.0 supported
> split modules without the above customization. However, this had to be
> removed as the default import code often led to confusion due to
> obfuscation of genuine Python ImportError problems. Using one of the
> two default configurations is the recommended approach now.
In other words, `native_bt.py` does not try
import _native_bt
because `native_bt.py` is part of a package; it only tries:
from . import _native_bt
The result is that, when the bindings are generated by SWIG 4,
`native_bt.py` cannot find `_native_bt`, which makes the bindings not
work at all.
The common method for SWIG 3 and 4 is importing from the same package.
For this to work, `setup.py` needs to install the built extension into
the `bt2` package. This patch changes `bindings/python/bt2/setup.py.in`
to do so.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Icf3622953280466e8d458caee79a28ed4a01fa2b
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1356 Reviewed-by: Simon Marchi <simon.marchi@efficios.com> Tested-by: jenkins
Fix: test_bitfield: extend coverage: 0-len signed write/read
By convention, a 0-len bitfield write is a no-op, and a 0-len read
sets the value of the output to 0.
So we can "encode" the value 0 over a length of 0 bit. Cover this
in the test-cases for signed types. It is already covered for the
unsigned test-cases.
Fix: flt.lttng-utils.debug-info: build id comparison
Issue
=====
Commit af54bd1801 fixed a endianness bug by coincidence.
The real bug is the following line:
name_sz = (uint32_t) *buf;
The `buf` variable is a `uint8_t *` that points at a 32bits word storing
the name size field.
On that line we dereference `buf` and cast the result to a uint32_t.
Dereferencing `buf` gives a `uint8_t` containing the first byte of the
32bits word which is then cast to 32bits and stored in `name_sz`.
However, the intent was to read the full 32bits word in `name_sz`.
It was working properly on little-endian architectures because the first
byte of the 32bit word was probably always enough to contain the size of
the name so casting the `uint8_t` to a `uint32_t` did not lead to
truncating.
But on big-endian architectures, that same first byte of 32bits word
containing the name size is probably always zero which lead to a failure
to recognize the note as a build id note.
Wrong Solution
==============
Commit af54bd1801 assumed that the `elf_getdata()` returned the note
section in the file byte order and used `gelf_xlatetom()` to swap the by
to the machine byte order. This assumption is false, the `elf_getdata()`
man page says:
The returned data descriptor will be setup to contain translated
data[...]
So no need to translate from the file byte order to the machine byte
order after the extracting the data using `elf_getdata()`.
The mis-guided use of `gelf_xlatetom()` made the testcase pass on
PowerPC because it swapped the bytes of the 32bits word making the cast
described above work. But running that same test on a PowerPC on a
PowerPC binary would not swap the byte order (as machine and file byte
order are the same) and would trigger the casting bug described above.
Right Solution
==============
The right solution is to cast the `buf` pointer to a `uint32_t *` before
dereferencing it. Such as:
name_sz = (uint32_t) *(uint32_t *)buf;
But, during this investigation, we discovered that the libelf library
offers the `gelf_getnote()` function that does all the work to extract
the note fields. Using this function makes it clearer and safer.
This commit uses this approach as well as reverting all the changes
added by commit af54bd1801.
Drawbacks
=========
None.
Reported-by: Michael Jeanson <mjeanson@efficios.com> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I952aad69d225c202b3aa9c41724b6645550a2d68
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1340 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Fix: lib-logging: possible buffer not null terminated
Issue
=====
While fixing Coverity warnings, commit 879e14e8 introduced other
warnings. Indeed, the destination string after the `strncpy()` call
would be left not null terminated if the size of the source buffer is
equal or larger than the `TMP_PREFIX_LEN`. Also, the `strncat()` does
not consider the characters added by the previous `strncpy()` call when
supplying the size of the destination buffer. This could result in
buffer overflows.
Solution
========
Use `snprintf()` to concatenate the two strings and set the last
character to `\0`. It's worth it to make the code easier to understand
even though the `snprintf()` is probably more work because of the format
string.
Drawbacks
=========
None.
Notes
=====
List of coverity defect reports:
CID 1401510 (#5 of 5): Buffer not null terminated
(BUFFER_SIZE_WARNING) 19. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401511 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING) 4. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401513 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 19. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401517 (#5 of 5): Buffer not null terminated
(BUFFER_SIZE_WARNING) 9. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
ID 1401519 (#4 of 4): Buffer not null terminated
(BUFFER_SIZE_WARNING) 15. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401523 (#3 of 3): Buffer not null terminated
(BUFFER_SIZE_WARNING) 16. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401525 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 10. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401526 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING) 11. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401528 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 15. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401532 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 5. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401533 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 6. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401534 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 12. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401535 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 13. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401537 (#2 of 2): Buffer not null terminated
(BUFFER_SIZE_WARNING) 23. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401539 (#10 of 10): Buffer not null terminated
(BUFFER_SIZE_WARNING) 10. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401540 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 28. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
CID 1401541 (#1 of 1): Buffer not null terminated
(BUFFER_SIZE_WARNING) 9. buffer_size_warning: Calling strncpy
with a maximum size argument of 64 bytes on destination array
tmp_prefix of size 64 bytes might leave the destination string
unterminated.
Reported-by: Coverity - Buffer not null terminated Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Ie10d4d6c8b1a0caff2fe70bbb1046673bbb1a999
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1333 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Fix: flt.lttng-utils.debug-info: cannot find addr past the first CU
Issue
=====
The `bin_info_lookup_source_location()` function doesn't look past the
first Compile Unit(CU) in the Dwarf information to find the filename and
line number of an address. This function iterates over all CUs of the
binary (or shared object) and tries to translate the address with each
of them. It ends up calling two other functions:
`bin_info_lookup_cu_src_loc_no_inl()`and `bin_info_lookup_cu_src_loc_inl()`
in a loop to cover all the CUs.
The `bin_info_lookup_cu_src_loc_no_inl()` function returns an error
(returning -1) if it can't find the line of the give address making the
calling function exit the loop, and thus abort the resolving.
The information that the resolving was unsuccessful is transmit back to
the caller by omitting to set the `src_loc` output parameter. The return
value of this function is used to notify the caller of an error. Not
resolving the source line should not make the function return -1.
This has the effect that if an address is contained in a CU after the
first one this function will not be able to resolve it.
Solution
========
Return 0 if the line is not found so that the search continues with
other CUs.
Drawbacks
=========
None.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I4d85bc75d32a9a25a7044f3686c6d5940311fa68
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1337 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Fix: flt.lttng-utils.debug-info: note name memcmp() overflow
Issue
=====
If the note section that we are currently parsing has a name longer
than the "GNU" string, the `memcmp()` call will read garbage after the
"GNU" string.
I witnessed this when the component was parsing a note section named
"stapsdt".
Solution
========
Make the section name length comparison explicit.
Drawbacks
=========
None.
Reported-by: Address Sanitizer - Global buffer overflow Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I515f8c883ddbc1884045e86aecef700ee2111959
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1322 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Cleanup: src.ctf.lttng-live: rename _is_canceled function
Rename the `lttng_live_is_canceled()` function to
`lttng_live_graph_is_canceled()` to make it clear it's the graph that
may be canceled and not the lttng_live component.
No behaviour change intended.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I918c5262e4b833eb8ed302eab42df19ceac0cbbc
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1309 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Fix: flt.utils.trimmer: no error when upstream can not seek
Issue
=====
According to commit 7de0e49, every upstream message iterators of a
`flt.utils.trimmer` must support seeking. If it's not the case, the
graph should fail gracefully.
I encountered a situation where this invariant is not held. I can
reproduce this issue by commenting out the `_SEEK_BEGINNING_METHOD()`
and `_CAN_SEEK_BEGINNING_METHOD()` of the `flt.utils.muxer` and ran the
following graph:
babeltrace run \
--component=src:src.ctf.fs \
--params='paths=["/home/frdeso/lttng-traces/auto-20181210-133612"]' \
--component=mux:filter.utils.muxer \
--component=trim:filter.utils.trimmer \
--params='begin="13:36:12.871289834",end="13:36:12.871303475"' \
--component=sink:sink.text.pretty \
\
--connect=src.*:mux.* \
--connect=mux.*:trim.* \
--connect=trim.*:sink.*
Note: The `begin` and `end` parameters are event timestamps from the trace.
When the trimmer component sees that seeking is not supported, it simply
skip the seeking and continue. This triggers a `BT_ASSERT()` later on.
Simon Marchi [Thu, 23 May 2019 15:30:29 +0000 (11:30 -0400)]
Add .gitignore entry for .theia
This directory is used for project-specific settings for the Theia IDE.
Since I use that, I would like if the directory was ignored. Even
though it doesn't apply to everybody, it shouldn't bother anybody to
have this entry. Also, I wouldn't mind if we added similar entries for
other IDEs, if it helped other people.
Issue
=====
Multiple possible buffer overflows on string operations using the
`SET_TMP_PREFIX()` macro that uses `strcpy()` on parameter char array.
Solution
========
Use a #define to set the length of the destination array and use it as
the size parameter of the `strncpy()` and `strncat()` calls.
Drawbacks
=========
None.
Notes
=====
Coverity reported defects:
CID 1401179 (#5 of 5): Copy into fixed size buffer (STRING_OVERFLOW) 9.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401181 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 5.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length
CID 1401186 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 15.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401192 (#2 of 2): Copy into fixed size buffer (STRING_OVERFLOW) 4.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401197 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 6.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401198 (#4 of 4): Copy into fixed size buffer (STRING_OVERFLOW) 15.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401203 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 28.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401212 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 12.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401215 (#4 of 4): Copy into fixed size buffer (STRING_OVERFLOW) 16.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401221 (#2 of 2): Copy into fixed size buffer (STRING_OVERFLOW) 19.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401227 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 13.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401230 (#3 of 3): Copy into fixed size buffer (STRING_OVERFLOW) 10.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401232 (#3 of 3): Copy into fixed size buffer (STRING_OVERFLOW) 23.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401234 (#5 of 5): Copy into fixed size buffer (STRING_OVERFLOW) 19.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401254 (#10 of 10): Copy into fixed size buffer (STRING_OVERFLOW) 10.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401257 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW) 9.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
CID 1401261 (#2 of 2): Copy into fixed size buffer (STRING_OVERFLOW) 11.
fixed_size_dest: You might overrun the 64-character fixed-size string
tmp_prefix by copying prefix without checking the length.
Cleanup: flt.utils.muxer: avoid returning a pointer needlessly
Reshuffle `muxer_msg_iter_add_upstream_msg_iter()` function to return an
int rather than a pointer.
Returning a pointer in this case has no value since the newly allocated
structure is stored in a GQueue directly. Returning an int to express
success or error seems more appropriate and less error-prone.
scan-build reports a potential memory leak of the
muxer_upstream_msg_iter pointer allocated in the
`muxer_msg_iter_add_upstream_msg_iter()` function if the `g_queue_new()`
fails.
So, free the muxer_upstream_msg_iter on the error path.
Fix: bitfield: shift undefined/implementation defined behaviors
bitfield.h uses the left shift operator with a left operand which
may be negative. The C99 standard states that shifting a negative
value is undefined.
When building with -Wshift-negative-value, we get this gcc warning:
In file included from /home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:44:0,
from /home/smarchi/src/babeltrace/ctfser/ctfser.c:42:
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h: In function ‘bt_ctfser_write_unsigned_int’:
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:116:24: error: left shift of negative value [-Werror=shift-negative-value]
mask = ~((~(type) 0) << (__start % ts)); \
^
/home/smarchi/src/babeltrace/include/babeltrace/bitfield-internal.h:222:2: note: in expansion of macro ‘_bt_bitfield_write_le’
_bt_bitfield_write_le(ptr, type, _start, _length, _v)
^~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/babeltrace/include/babeltrace/ctfser-internal.h:418:3: note: in expansion of macro ‘bt_bitfield_write_le’
bt_bitfield_write_le(mmap_align_addr(ctfser->base_mma) +
^~~~~~~~~~~~~~~~~~~~
This boils down to the fact that the expression ~((uint8_t)0) has type
"signed int", which is used as an operand of the left shift. This is due
to the integer promotion rules of C99 (6.3.3.1):
If an int can represent all values of the original type, the value is
converted to an int; otherwise, it is converted to an unsigned int.
These are called the integer promotions. All other types are unchanged
by the integer promotions.
We also need to cast the result explicitly into the left hand
side type to deal with:
warning: large integer implicitly truncated to unsigned type [-Woverflow]
The C99 standard states that a right shift has implementation-defined
behavior when shifting a signed negative value. Add a preprocessor check
that the compiler provides the expected behavior, else provide an
alternative implementation which guarantees the intended behavior.
A preprocessor check is also added to ensure that the compiler
representation for signed values is two's complement, which is expected
by this header.
Document that this header strictly respects the C99 standard, with
the exception of its use of __typeof__.
Adapt use of _bt_piecewise_lshift in plugins/text/pretty/print.c
to the new API.
Fix: flt.utils.trimmer: bt_message_put_ref() on freed message
Issue
=====
In this function, we convert a timestamp in second to a clock value in
order to create a stream activity beginning message. We create a new
message and proceed to call `clock_raw_value_from_ns_from_origin()` to
convert the timestamp to a clock value.
If this call fails, we put the reference on this message using
`bt_message_put_ref()`. This frees the message as the refcount was at 1
because it was just created.
The problem here is that we don't reset the `msg` pointer and return it
to the call via the output parameter. This pointer points to freed data.
The caller then figures that the function failed by looking at the
return value and proceed to call `bt_message_put_ref()` again resulting
and assertion failure.
This can be easily triggered by passing a `begin` parameter of value 0
to a trimmer component.
Solution
========
Use a local variable to keep the pointer to the newly created message
and move that reference to the output parameter only on success.
Fix: flt.utils.muxer: Undefined or garbage value returned
Reported-by: scan-build - Undefined or garbage value returned to caller Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I46c6e431ae4dcb68367dea84971779af017ecded
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1310 Tested-by: jenkins Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Fix: src.ctf.fs: possible use-after-free in the error path
Reported-by: scan-build - Use of memory after it is freed Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I5c19a45b0ddd11eed3872295efeb7b5201967419
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1303 Tested-by: jenkins Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
This removal should have been done in an earlier commit modifying this
loop:
Author: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Tue Apr 30 19:01:17 2019 -0400
src.ctf.fs: compute stream range using entire file info group
Given that index entries are sorted within their file info structure and
that file info structures are sorted by time within the file info group,
we can simply compute the range of the stream by using the absolute
first index entry and the absolute last index entry of the entire file
info group.
Add `BT_ASSERT()` to enforce invariants in developer mode.
No need to check if `trace->metadata_path` is null at the point because
it has been used by `fopen()` already and `abort()` is called if
`fopen()` fails.
CID 1401238 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking trace->metadata_path suggests that it
may be null, but it has already been dereferenced on all paths leading
to the check.
Fix: sink.ctf.fs: possible uses after free because of unchecked return values
Uses after free are possible because we ignore the return value of the
`bt_ctfser_write_*()` function calls. To avoid this, check the return
values and log an error message.
CID 1401217 (#4 of 4): Use after free (USE_AFTER_FREE)
11. deref_arg: Calling bt_ctfser_write_byte_aligned_unsigned_int
dereferences freed pointer stream->ctfser.base_mma
CID 1401193 (#2 of 2): Unchecked return value (CHECKED_RETURN)
8. check_return: Calling bt_ctfser_write_byte_aligned_unsigned_int
without checking return value (as is done elsewhere 8 out of 10
times).
Reported-by: Coverity (1401217) Use after free Reported-by: Coverity (1401193) Unchecked return value Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I7581c0e549d7bf916f42afe306976baa9822897a
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1286 Tested-by: jenkins Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
CID 1401190 (#1 of 1): Useless call (USELESS_CALL)
side_effect_free: Calling bt_common_color_bold() is only useful for
its return value, which is ignored.
side_effect_free: Calling bt_common_color_fg_red() is only useful for
its return value, which is ignored.
Add -Werror=implicit-function-declaration to AM_CFLAGS
Implicit declaration of function is most likely always a bad thing.
Implicit declaration of function is particularly problematic with
Babeltrace 2 because of its plugin system. Currently, when building a
plugin shared object, the compiler issues a warning when it encounters
an implicit declaration but will create the .so just fine but with the
implicit function marked as undefined, as expected by the standard prior
to C99.
When trying to load a Babeltrace plugin with an undefined symbol, glib's
`g_module_open()` fails and Babeltrace then considers that this .so is
not a valid Babeltrace plugin.
A failure to load a plugin is not a error because we want Babeltrace to
be able to list/use every plugins in a given directory by looking at
every .so file. So we can't simply return an error in those cases.
Since Babeltrace marks plugins with undefined symbols as invalid they
can't be used in a graph.
This problem is caused when a typo in a function name finds its way into
the code and a warning message is ignored. It can also happen when the
code uses a function of more recent library than what is currently
installed on the system.
So, to ensure this never happens, we promote the
`-Wimplicit-function-declaration` warning to the rank of compilation
error.
This is diagnostic has been supported by GCC since v4.8 and Clang since
v4 at least so we believe it's safe to enable it by default.
flt.lttng-utils.debug-info: fd-cache: log to `debug` severity on stat() error
Issue
=====
A `debug-info` component tries to open multiple files in its search to
find the debugging information necessary to resolve the addresses
contained in the trace.
Early in the `bt_fd_cache_get_handle()` function, a `stat()`
is done on the path to get the inode number and device number. This
`stat()` returns an error if the file is absent. Currently, in those
cases, an message is logged at the `error` severity level (BT_LOGE_*)
resulting in the printing of an error message. Since multiple files are
tired until the right one is found (if any), the user can see multiple
error messages even if the right file may be found later.
This is undesirable because it mislead the user into thinking that an
error occurred when in fact it's completely normal.
Solution
========
Log the failure of this `stat()` call to the `debug` severity level.
Fix: sink.ctf.fs: writing 64bit real number as 32bit
Both 32bit and 64bit Trace IR real numbers are written as 32bit CTF
floats with the `bt_ctfser_write_float32()`. This is most probably a
copy-paste error.
To fix this, use the `bt_ctfser_write_float64()` in the else branch instead.
Coverity report:
CID 1401207 (#1 of 1): Identical code for different branches (IDENTICAL_BRANCHES)
identical_branches: The same code is executed regardless of whether
fc->base.size == 32U is true, because the 'then' and 'else' branches
are identical. Should one of the branches be modified, or the entire
'if' statement replaced?
Reported-by: Coverity (1401207) Identical code for different branches Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I462fd47e968e39f5878431d5ac6cbcbbf1d2ffcf
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1268 Tested-by: jenkins Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Fix: flt.lttng-utils.debug-info: `ip` field is 32bit on 32bit cpus
Issue
=====
The debug-info component enforces that the `ip` field used to resolve
addresses is a 64bit integer, but the LTTng UST `ip` context field is
32bit on a 32bit cpu. A debug-info component thus fail to recognize that
the trace has all the necessary information to resolve the addresses.
Here is the metadata representing the context on a 32bit system:
event.context := struct {
integer { size = 32; align = 8; signed = 1; encoding = none; base = 10; } _vpid;
integer { size = 32; align = 8; signed = 0; encoding = none; base = 16; } _ip;
};
Here is the metadata representing the context on a 64bit system:
event.context := struct {
integer { size = 32; align = 8; signed = 1; encoding = none; base = 10; } _vpid;
integer { size = 64; align = 8; signed = 0; encoding = none; base = 16; } _ip;
};
Solution
========
Remove the integer size check in the
`is_event_common_ctx_dbg_info_compatible()` function.
CID 1401244 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking
graph->listeners.source_output_port_added suggests that it may be null,
but it has already been dereferenced on all paths leading to the check.
CID 1401260 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking
graph->listeners.filter_output_port_added suggests that it may be null,
but it has already been dereferenced on all paths leading to the check.k
CID 1401243 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking
graph->listeners.filter_input_port_added suggests that it may be null,
but it has already been dereferenced on all paths leading to the check.
CID 1401185 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking graph->listeners.sink_input_port_added
suggests that it may be null, but it has already been dereferenced on
all paths leading to the check.
CID 1401262 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking
graph->listeners.source_filter_ports_connected suggests that it may be
null, but it has already been dereferenced on all paths leading to the
check.
CID 1401200 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking
graph->listeners.filter_filter_ports_connected suggests that it may be
null, but it has already been dereferenced on all paths leading to the
check.
CID 1401252 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking
graph->listeners.source_sink_ports_connected suggests that it may be
null, but it has already been dereferenced on all paths leading to the
check.
CID 1401264 (#1 of 1): Dereference before null check (REVERSE_INULL)
check_after_deref: Null-checking
graph->listeners.filter_sink_ports_connected suggests that it may be
null, but it has already been dereferenced on all paths leading to the
check.
Reported-by: Coverity (various ids) Dereference before null check Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I88c8172e006e4bae2c7b5e29b501e1a36f19ae5c
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1262 Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
The gelf_getshdr() function return a pointer to the second parameter on
success and NULL on error. This means that on we overwrite the
`curr_section_hdr` variable and leak memory.
To fix it, we simply change the variable from a heap allocated variable
to a stack allocated variable this has the effect of making code simpler.
Found using scan-build:
line 334, column 7
Potential leak of memory pointed to by 'curr_section_hdr'
Fix: sink.text.pretty: Unsigned compared against 0
CID 1401251 (#1 of 1): Unsigned compared against 0 (NO_EFFECT)
unsigned_compare: This less-than-zero comparison of an unsigned value is
never true. nr_fields < 0UL.
Reported-by: Coverity (1401251) Unsigned compared against 0 Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: I8aa8a03a6b03ca1640570d70a51729d001af3b66
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1263 Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Philippe Proulx [Sun, 5 May 2019 11:53:50 +0000 (07:53 -0400)]
cli/babeltrace.c: update known log level environment variable names
Eventually we could find another way to synchronize the environment
variable names we use throughout the project with the CLI. We want to
decouple the plugins from the CLI (plugins do not know CLI, but CLI
knows plugins) so I don't want the plugins to support an environment
variable defined by the CLI for example.
Philippe Proulx [Fri, 3 May 2019 19:14:18 +0000 (15:14 -0400)]
lib: add unsigned and signed integer value API
This patch replaces the integer value API with the more specific
unsigned and signed integer value API. This opens the whole unsigned
64-bit set for component initialization parameters, query parameters,
queyr results, and, eventually, custom metadata attributes. This is also
more in line with the field API, which has both the unsigned and signed
versions.
Library
=======
The API is simply split in two, where an unsigned integer value contains
a `uint64_t` raw value and a signed integer value is the equivalent of
the previous integer value (`int64_t` raw value).
The types are `BT_VALUE_TYPE_UNSIGNED_INTEGER` and
`BT_VALUE_TYPE_SIGNED_INTEGER`.
There's no interaction between unsigned and signed integer values, in
that, for example, you cannot compare an unsigned integer value with a
signed integer value with bt_value_compare().
Behind the scenes, I kept a single `struct bt_value_integer` for
simplicity. It contains a union of `uint64_t` and `int64_t`. Most
functions call a generic unsigned version, casting to `uint64_t` when
necessary. For example, there's a common bt_value_integer_compare()
which compares the `uint64_t` values.
CLI
===
Before this patch, the `--params` option's format makes any integer
constant a signed integer value object. With this patch, you can create
an unsigned integer value by prepending the constant with `+`. For
example:
hello=+293, meow=+0x388ab88fd, uint-zero=+0
I wanted to use the `U` suffix, like in C/C++, but the GLib lexical
scanner won't allow a constant to be followed by an identifier without a
whitespace.
The documentation about the format of `--params` is updated for the
CLI's `--help` option and in the man pages.
Plugins
=======
It's up to each plugin to accept, for a given parameter, an unsigned
integer value, a signed integer value, or both. A plugin can be very
strict and it's not a bad thing: there are situations where an unsigned
integer is conceptually required.
The individual changes are:
`src.ctf.fs`:
In the `trace-info` query result, create unsigned integer values for
the `id` (stream ID) and `class-id` (stream class ID) entries. A CTF
metadata ID is never negative.
`src.ctf.lttng-live`:
In the `sessions` query result, the `timer-us`, `stream-count`, and
`client-count` entries are still signed integer values. They could
probably be unsigned, but I want to confirm this with developers
more involved with this component class before doing the change. For
the moment, the query operation is unchanged.
`sink.utils.counter`:
The `step` initialization parameter is now expected to be an
unsigned integer value. This is never negative.
I also made the initialization refuse parameters when they don't
have the expected type. There are new `logging.c` and `logging.h`
files for this component class because I used logging to communicate
said errors.
Library tests
=============
The integer value tests now test the signed value API.
I added equivalent unsigned value object tests.
Python bindings
===============
`_IntegerValue` is now the base class of `UnsignedIntegerValue` and
`SignedIntegerValue`. `_IntegerValue` contains the whole public
interface, using template methods with specialized parts defined in
subclasses.
`_IntegerValue` is imported in the `bt2` package for this use case:
if isinstance(val, bt2._IntegerValue):
do_something_with(val)
because `int(val)` is always valid.
The bt2.create_value() function still creates a `SignedIntegerValue`
object with an `int` value. This is the safest, as there's no way to
know that this value will be changed to a negative value in the future,
before the value object is frozen. For example, this would not be
possible with a conversion to an `UnsignedIntegerValue`:
val = bt2.create_value(47)
val.value = -23
whereas it is possible with a `SignedIntegerValue`. You can still
explicitly create an unsigned integer value:
Python bindings tests
=====================
`test_value.py` is updated to also test the `UnsignedIntegerValue`
class. _inject_numeric_testing_methods() now accepts an optional
`has_neg` parameter which, if false, does not inject tests which involve
negative integer values.
There is a common `_TestIntegerValue` class which
`SignedIntegerValueTestCase` and `UnsignedIntegerValueTestCase` inherit.
The tests are almost the same, but `SignedIntegerValueTestCase` adds a
few tests related to negative values. Both specific test cases set the
`_CLS` class attribute to the value object class to test, either
`UnsignedIntegerValue` or `SignedIntegerValue`.
Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ic96ef9e1e16883cb5c59844c6ba5a060936efdb0
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1257 Tested-by: jenkins
Simon Marchi [Fri, 3 May 2019 21:25:14 +0000 (17:25 -0400)]
bt2: Make test_query_executor pass
This patch fixes and enables test_query_executor.py.
The only important change is in the query method: it is necessary to
check for `is_canceled` prior to calling the API function, since it
asserts that the executor is not cancelled.
I have removed the __eq__ method and tests, since it's quite unlikely to
be useful for the QueryExecutor class.
Change-Id: If88c2552627358b06f7d6068126034a20ebf88e8 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1252 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Fri, 3 May 2019 19:58:04 +0000 (15:58 -0400)]
bt2: Fix Makefile dependency tracking when building out of tree
The problem, from the point of view of the developer:
1. Build out of tree, python bindings enabled.
2. Modify a bindings source file (e.g. trace_class.py,
native_bt_trace.i).
3. Type "make" in the builddir.
4. The bindings are not rebuilt from the new source files.
5. Sad.
When we build out of tree, it's done in two steps (assume $builddir is
$top_builddir/bindings/python):
1. Copy all the static files (those checked in the repo, not generated
at runtime) from $srcdir/bt2 to $builddir/bt2. Those files are placed
next to other files that are generated (__init__.py, setup.py).
Together, they form the complete source of the bt2 package.
2. Run setup.py, which runs SWIG and outputs the built package in
$builddir/build_lib.
The problem described above happens because of how the dependencies are
stated for step #1:
When nothing exists yet in the build directory, bt2/native_bt_trace.i is
resolved to the version in the srcdir (because of make's VPATH, I
believe), so it works fine. But once the copied files exist, and you
try to change the source files and "make" again, the dependency
bt2/native_bt_trace.i will be resolved to the version in the build dir.
Since that version is not newer than copy-static-deps.stamp, nothing is
done.
The fix it, we can force the dependency to be the one in the source
directory, by prepending $(srcdir) to each file.
Change-Id: Ic09d430e53b59e5afa9ebb19d98ba219dff16537 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1250 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Michael Jeanson [Fri, 3 May 2019 19:06:45 +0000 (15:06 -0400)]
fix: g_hash_table_insert prior to glib 2.40 returns void
Even the post glib 2.40 version will only return false if the value is
already present in the hashtable but will not fail to add it. The old
value will be handled according to the configuration of the hashtable
which should not be an error.
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Change-Id: I5512c921ce220bf8a956c61f594d6a27e6ceb41e
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1247 Tested-by: jenkins Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Philippe Proulx [Fri, 3 May 2019 23:51:11 +0000 (19:51 -0400)]
tests/plugins/ctf/Makefile.am: do not set `TESTS` in this file
We want the project's `make check` to continue even if specific tests
fail, so we can't define `TESTS` in a subdirectory's `Makefile.am`,
because if the test fails, `make` fails, and subsequent tests are not
executed.
There are ways to circumvent this, for example using `make`'s
`--always-make` option, but it was decided previously that we want
`make check` to run all tests as is, without any option.
Therefore we set `TESTS` to all tests in `tests/Makefile.am`.
To execute tests specific to a subdirectory, we have the following
targets:
Fix: flt.lttng-utils.debug-info: build id note section non-native byte order
Issue
=====
The build id comparison code does not take into account that the target
binary file could be compiled for another architecture and thus could be
of a different byte order. This makes the build_id testcase fail on
PowerPC. The artefacts used for this testcase are compiled on x86 and
are thus in little endian as opposed to PowerPC's big endianness.
Solution
========
Parse the Elf file to extract the endianness of the binary and use that
information to change the byte order of the note section if necessary.
Drawbacks
=========
None.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Id5d761832f9463b38ea0452da36053e885929aa9
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1246 Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com> Reviewed-by: Michael Jeanson <mjeanson@efficios.com>
Fix: common: va_list type is implementation dependant
Issue
=====
BT_ASSERT(*args) happens to work on some library implementations because
the va_list type happens to be a pointer but this is not true with all
libraries. For example, compiling on arm64 results in the following
error:
In file included from common.c:34:0:
common.c: In function ‘bt_common_custom_vsnprintf’:
../include/babeltrace/assert-internal.h:42:7: error: wrong type
argument to unary exclamation mark
if (!(_cond)) { \
^
common.c:1524:2: note: in expansion of macro ‘BT_ASSERT’
BT_ASSERT(*args);
^~~~~~~~~
Solution
========
Remove that BT_ASSERT() call as its intent is unclear anyway.
Drawback
========
None.
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Iada5189ef4580e48caefed12835bfab7c23eaf73
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1253
CI-Build: Jérémie Galarneau <jeremie.galarneau@efficios.com> Reviewed-by: Michael Jeanson <mjeanson@efficios.com> Reviewed-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
translate-trace-ir-to-ctf-ir.c:935:16: warning: ‘var_fc’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
named_fc =
fs_sink_ctf_field_class_variant_borrow_option_by_index(
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
var_fc, i);
I don't think this "condition" is reachable, but it is, in principle
possible. Initializing var_fc to NULL silences this warning.
Simon Marchi [Thu, 2 May 2019 16:21:20 +0000 (12:21 -0400)]
src.ctf.fs: make trace-info query accept clock-class-offset-{s,ns} parameters
The src.ctf.fs components accept clock-class-offset-s and
clock-class-offset-ns parameters, allowing to add an offset to the
clock.
The trace-info query currently doesn't accept the same parameters. In
general, I think we'll want the components and trace-info to accept
pretty much the same set of parameters, so that the information reported
by trace-info accurately represents how an eventual component reading
the same traces will work.
This patch splits the parameter handling of the component in a
standalone function re-used by the query. The advantage of sharing a
function like this is that if we add a parameter to the component, we'll
be forced to think about what happens to the trace-info query.
One behavior change is that trace_info_query now returns
BT_QUERY_STATUS_INVALID_PARAMS if the passed parameters are invalid.
A test is added to test the new functionality.
Change-Id: I3d917b8713fce4ce56a79a6e7c7bdc3e32d9ec0c Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1244 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Thu, 2 May 2019 20:53:30 +0000 (16:53 -0400)]
ctf: Use g_time_val_from_iso8601 instead of g_date_time_new_from_iso8601
g_date_time_new_from_iso8601 was introduced in glib 2.56, which is quite
recent. To support build with older glibs, change the code to use
g_time_val_from_iso8601 instead, introduced in glib 2.12.
The code in question only uses this function to validate that the passed
string is ISO8601-compliant, it doesn't use the resulting time value.
The problem of g_time_val_from_iso8601 is that its result is not year
2038-safe on 32-bit systems. We can expect that by then, that function
will have been removed from glib, forcing us to update our code.
Change-Id: I2f4fdfce9020ecefcfb5d653a3885d343e49bd76 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1245 Reviewed-by: Michael Jeanson <mjeanson@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
... so they should be removed from EXTRA_DIST as well. Currently, it
causes a "make dist" failure:
make[4]: *** No rule to make target 'test_trace.py', needed by 'distdir'. Stop.
The tests/lib/trace-ir directory is currently not very useful, as it
contains no tests at all, so we could remove it in theory. But I
presume we'll add something in there in not too long, so I think we can
leave it there.
Change-Id: Iec44efd7c52e84b35a5a55206055b5d63635fbe1 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1241 Tested-by: jenkins Reviewed-by: Jonathan Rajotte Julien <jonathan.rajotte-julien@efficios.com> Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Simon Marchi [Thu, 2 May 2019 18:36:57 +0000 (14:36 -0400)]
include: add missing files to dist tarball
When making a tarball (make dist) and trying to build from it, it fails
because of missing header files. This patch adds all the necessary
headers to the dist tarball to make "./configure && make" from the
tarball succeed.
Change-Id: I89051cb060e911eac154ef1726034e905b49b085 Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1243 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Reviewed-by: Jonathan Rajotte Julien <jonathan.rajotte-julien@efficios.com>
Simon Marchi [Wed, 1 May 2019 22:00:50 +0000 (18:00 -0400)]
bt2: Add wrapper for bt_plugin_get_version
There is a subtle bug with our use of the "char **OUT" typemap for the
bt_plugin_get_version function.
THe SWIG wrapper does this:
char *temp_value5;
arg5 = &temp_value5;
The initial content of temp_value5 is unknown, assume that it points to
unreadable memory. If the call to bt_plugin_get_version returns
PROPERTY_AVAILABILITY_NOT_AVAILABLE, the value of temp_value5 is
unchanged (but really, it could be anything, again we should assume it
points to unreadable memory). However, the epilogue of the typemap will
still try to transform the C string to a Python str object. Doing so
will cause a crash when trying to read the C string.
The fix is to make our own wrapper to bt_plugin_get_version, which
guarantees that if it returns PROPERTY_AVAILABILITY_NOT_AVAILABLE, it
will force the value of the output parameter to be NULL (which is
handled by the epilogue and transformed to None).
Setting the temporary variable to an invalid but non-zero pointer value
((void *) 1) makes this bug consistently reproducible and not rely on
random memory contents. I suggest we keep it there, in order to quickly
find other instances of this bug in the future.
Change-Id: Idd12ca4c4d4d732b841c1e12420e46e6a5a25874
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1221 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Fix: bt2: enum-conversion warning in native_bt_component_class.i
clang warns about the following enumeration conversion.
bt2/native_bt_wrap.c:3896:12: warning: implicit conversion from
enumeration type 'enum bt_message_iterator_status' to different
enumeration type 'enum bt_self_message_iterator_status'
[-Wenum-conversion]
status = BT_MESSAGE_ITERATOR_STATUS_END;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bt2/native_bt_wrap.c:3898:12: warning: implicit conversion from
enumeration type 'enum bt_message_iterator_status' to different
enumeration type 'enum bt_self_message_iterator_status'
[-Wenum-conversion]
status = BT_MESSAGE_ITERATOR_STATUS_AGAIN;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bt2/native_bt_wrap.c:3901:12: warning: implicit conversion from
enumeration type 'enum bt_message_iterator_status' to different
enumeration type 'enum bt_self_message_iterator_status'
[-Wenum-conversion]
status = BT_MESSAGE_ITERATOR_STATUS_ERROR;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bt2/native_bt_wrap.c:4513:43: warning: implicit conversion from
enumeration type 'enum bt_message_iterator_status' to different
enumeration type 'bt_self_message_iterator_status' (aka 'enum
bt_self_message_iterator_status') [-Wenum-conversion]
bt_self_message_iterator_status status = BT_MESSAGE_ITERATOR_STATUS_OK;
~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bt2/native_bt_wrap.c:4678:43: warning: implicit conversion from
enumeration type 'enum bt_message_iterator_status' to different
enumeration type 'bt_self_message_iterator_status' (aka 'enum
bt_self_message_iterator_status') [-Wenum-conversion]
bt_self_message_iterator_status status = BT_MESSAGE_ITERATOR_STATUS_OK;
~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Change-Id: Icb3bc0141cdd9f35a5a42420bb48167c12e70df0
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1183 Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com> Tested-by: jenkins
projects / babeltrace.git / log
