Re: [PATCH v2 3/3] trace: fix undefined behavior in register

[David Marchand <david.marchand@redhat.com>]

On Thu, Jan 30, 2025 at 8:10 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 30 Jan 2025 15:58:49 +0100
> David Marchand <david.marchand@redhat.com> wrote:
>
> > Registering a tracepoint handler was resulting so far in undefined
> > behavior at runtime.
> >
> > The RTE_TRACE_POINT_REGISTER() macro was casting the tracepoint handler
> > (which expects arguments) to a void (*)(void).
> > At runtime, calling this handler while registering resulted in
> > reading the current stack with no relation to this function prototype.
> >
> > Instead, declare an additional inline _register() handler for each
> > tracepoint and make sure that the emitting macros in
> > rte_trace_point_register.h only work on arguments name and type.
> >
> > The original tracepoint handler prototype is adjusted by adding a
> > __rte_unused for each argument (since emitting macros do nothing
> > with them).
> > This last part introduces an implementation limit of 15 arguments.
> >
> > With this change in place, the workaround in dmadev tracepoints can be
> > removed.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> When I build with -Db_santize=undefined the following warning shows up.
> It seems related.
>
> In function ‘rte_ethdev_trace_get_dcb_info’,
>     inlined from ‘rte_eth_dev_get_dcb_info’ at ../lib/ethdev/rte_ethdev.c:6720:2:
> ../lib/eal/include/rte_trace_point.h:381:9: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
>   381 |         memcpy(mem, &(in), sizeof(in)); \
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../lib/eal/include/rte_trace_point.h:53:9: note: in definition of macro ‘__RTE_TRACE_POINT’
>    53 |         __VA_ARGS__ \
>       |         ^~~~~~~~~~~
> ../lib/ethdev/ethdev_trace.h:1213:1: note: in expansion of macro ‘RTE_TRACE_POINT’
>  1213 | RTE_TRACE_POINT(
>       | ^~~~~~~~~~~~~~~
> ../lib/eal/include/rte_trace_point.h:399:9: note: in expansion of macro ‘__rte_trace_point_emit’
>   399 |         __rte_trace_point_emit(len, uint8_t); \
>       |         ^~~~~~~~~~~~~~~~~~~~~~
> ../lib/ethdev/ethdev_trace.h:1223:9: note: in expansion of macro ‘rte_trace_point_emit_blob’
>  1223 |         rte_trace_point_emit_blob(dcb_info->tc_bws, num_tcs);

This is something I saw with optimisations (like O2 or O3 iirc) too.

Compiling and running with optimisations made GHA vm go out of memory,
so I have been testing only with O0 when it comes to ubsan.

In any case, this series is fixing just one undefined behavior.
Fixing all build and runtime errors seen with ubsan requires more work.


-- 
David Marchand