[PATCH] record: Add wrappers for malloc() functions

[Chris Johns]

On 6/9/19 11:09 pm, Sebastian Huber wrote:
> On 02/09/2019 08:00, Chris Johns wrote:
>> On 2/9/19 3:37 pm, Sebastian Huber wrote:
>>> On 01/09/2019 04:29, Chris Johns wrote:
>>>> On 30/8/19 11:07 pm, Sebastian Huber wrote:
>>>>> Introduce new library librtemsrecordwrap.a which contains wrappers for
>>>>> operating system functions which produce entry/exit events.
>>>>
>>>> Why not enhance the trace linker to handle the recorder you have developed? It
>>>> has been able to wrap malloc and friends for a while now, it is just lacking
>>>> support for your recent tracing effort.
>>>>
>>>> Currently the trace linker does not uses the DWARF information to determine the
>>>> function signature as the DWARF support was added to the tool kit after it was
>>>> created and it has not been updated. The idea is to have DWARF provide the
>>>> signature of any suitable function so it can be wrapped.
>>>
>>> With the DWARF information at hand it should be possible to generate generic
>>> record events for function calls with up to 10 input arguments
>>> (RTEMS_RECORD_ARG_[0-9]) and up to 10 return values (RTEMS_RECORD_RETURN_[0-9]).
>>> We need two events for generic function entry/exit:
>>>
>>> RTEMS_RECORD_FUNCTION_ENTRY
>>> RTEMS_RECORD_FUNCTION_EXIT
>>>
>>> The event data for these two is the function address.
>>>
>>>>
>>>> I do not see any upside adding this library or wrapping specific functions
>>>> this way.
>>>
>>> The benefits are:
>>>
>>> * Slightly more compact (the event indicates the function and the data can be
>>> used for the caller address instead, the generic function entry/exit needs two
>>> events for this).
>>
>> Could you please explain this, sorry I am not sure what this means? If the code
>> is what needs to be generated then why not generate?
>>
>>> * It works without extra tools to build the application.
>>
>> You need a bunch of extra tools to record the data and view it. The trace linker
>> is built and installed when the tools are built and installed.
> 
> Ok, all extra RTEMS tools are installed via the RSB, so we can drop this point.
> 
>>
>>> * It can be tested through the RTEMS test suite.
>>
>> We lack better tests for our external tools and adding this would be good to
>> have. If you need a way to test the template adding it to a specific test would
>> work.
>>
>>> * You don't need the ELF file to produce a report.
>>
>> I recommend all users archive the ELF image of an executable with full debug
>> info because you can look into crash dumps. Without it you can only guess.
> 
> Yes, you should archive the ELF file, so we can drop this point.
> 
>>
>>> The only strong argument is the first one. A future support in the trace linker
>>> could use the librtemsrecordwrap.a and only generate generic stubs for other
>>> functions.
>>
>> It complicates the management of wrapping become you have some functions in a
>> special library and others being wrapped.
> 
> We have to make a trade-off between
> 
> * a slightly more complicated host tool, and
> 
> * higher memory demands on the target system.
> 
> From my point of view lower memory demands on the target system are much more
> attractive.

Yes I agree, there is no issue with the code itself, it is about the location of
the code, why we have chosen that path, and possible issues further in time when
we may want to change the implementation.

I would like to examine the use cases...

1. I see the most common use case is connecting to a system to gather interrupt
and task context switching details. The user starts a service however no
handlers are installed until a connection is made to trace.

2. A variation of this is a system with a runtime system configuration option
(eg boot option) to enable the tracing service. By default it is disabled. This
is a system integration function where something during integration is not
working and needs investigation by an engineering team. Rebuilding code and
loading it onto a system may not be easy or possible and staring at the system
offers no insight.

3. Development profiling, validation or debugging of specific pieces of code.
This is a special link to wrap and trace specific code. The wrapping is
unconditional and a development tool for development builds of an application.

4. Instrumentation of specific pieces of code where wrapping is not possible.
There are specific cases where wrapping is problematic and insertion of trace
code is needed to instrument it.

As you move from use case 1 to 4 a more detail knowledge of how trace works is
needed. I see for 1. you make a call to register a service and that enables a
transport, eg bind to a socket and listen. I would expect the various tools in
rtems-tool to work from the command with no configurations, changes or rebuilds.
Use case 2. is similar to 1. but the target trace code is resident but dormant
until needed. The key point is the target trace hooks are dynamic and not
configuration table or build dependent.

Use case 3 is the specific trace of functions including system functions like
malloc. I believe the trace linker is needed here to manage the process for a
user. It defines an interface so we can play the implementation details. The
trace linker can build a C file and perform a link so there is no reason it
cannot add from a template this code. A single tool can then manage the metadata
file generation and if this happens the trace linker can decide to use user
event ids for any wrapped code not just specially defined functions such as
malloc. In fact the id and any allocation become redundant as the allocation is
in the compiled C code and the metadata file and transparent to a user.

I am concerned with us splitting how we trace into 2 separate locations, the
trace linker for some functions and a library for others. I can see cases of a
user thinking they need to add code the wrap library to wrap it.

> If we add DWARF support to the RTEMS Trace Linker, then the configuration
> interface will drastically change since you no longer need the bits an pieces to
> construct the wrapper code. The question is on which API do we base this on,
> directly the libdwarf or instead rld::dwarf::file? To me this looks like a
> complex multi week project.

This seems to have been resolved with the LLVM path.

> Adding pre-defined wrappers for standard functions can be done by a student if
> an example is available. They are also highly desirable due to the more compact
> format on the target. Also the consumer side is simplified (just a case in a
> switch, otherwise you have to go from an address to a function name to the
> output handler), e.g. to translate record items to LTTNG system call events.

I agree they are highly desirable, so much so any function a user wants should
be wrapped this before any other method is selected. If all compact ids are used
a warning could given to a user a way to define slow and fast could be implemented.

> I am not sure if the generator definition via configuration files is the right
> thing to do. I am more in favour of doing this in C++ via subclasses.

Not everyone wants to play with C++ or make changes to released tools. The
current format of the config files can be change but I think giving users the
ability to create and maintain configuration files externally to our tools is
important.

Chris