[libcamera-devel] [PATCH v2] test: gstreamer: Disable gstreamer registry forks

[Vedant Paranjape]

Hi Laurent,
Thanks for the feedback.


On Sun, 22 Aug, 2021, 06:37 Laurent Pinchart, <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Vedant,
>
> Thank you for the patch.
>
> Looks like you've found an answer to the question you asked on my review
> of v1 :-)
>
> On Sat, Aug 21, 2021 at 08:11:45PM +0530, Vedant Paranjape wrote:
> > ASan needs to be loaded first before gstreamer is loaded. This was not
> > possible, so verify_asan_link_order was disabled. Better way to tackle
> > this issue was disabling forks on the gstreamer side.
> >
> > verify_asan_link_order=0 disables the check on ASan side which checks if
> > ASan was loaded before any other DLLs. Since, gstreamer spawns a child
>
> s/DLLs/shared objects/ as we're on Linux, no Windows :-)
>

Oops 😅

> helper process while building the registry, we needed to disable this
> > check. But with gst_registry_fork_set_enabled() it is possible to
> > disable spawning this child helper process, so this ensures that ASan is
> > loaded before any other DLL is loaded.
> >
> > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > ---
> >  test/gstreamer/gstreamer_single_stream_test.cpp | 17 ++++++-----------
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp
> b/test/gstreamer/gstreamer_single_stream_test.cpp
> > index 349dcfa4..80e652eb 100644
> > --- a/test/gstreamer/gstreamer_single_stream_test.cpp
> > +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
> > @@ -41,18 +41,13 @@ protected:
>
> The comment starts with "GStreamer by spawns a process ...". Now that we
> know this can be changed with gst_registry_fork_set_enabled(), I think
> this should be changed to "GStreamer by default spawns a process ...".
>
> >                * GStreamer is most likely not, this will cause the ASan
> link
> >                * order check to fail when gst-plugin-scanner dlopen()s
> the
> >                * plugin as many libraries will have already been loaded
> by
> > -              * then. Work around this issue by disabling the link order
> > -              * check. This will only affect child processes, as ASan is
> > -              * already loaded for this process by the time this code is
> > -              * executed, and should thus hopefully be safe.
> > -              *
> > -              * This option is not available in gcc older than 8, the
> only
> > -              * option in that case is to skip the test.
> > +              * then. Work around this issue by disabling spawning of a
> > +              * child helper process when rebuilding the registry. This
> will
>
> How about "when scanning the build directory for plugins" instead of
> "when rebuilding the registry", as the problems occurs when
> gst_registry_scan_path() is called ?
>
> > +              * only affect child processes, as ASan is already loaded
> for
> > +              * this process by the time this code is executed, and
> should
> > +              * thus hopefully be safe.
>
> I think the last sentence can be dropped. I wrote it to document that
> verify_asan_link_order=0 didn't disable the check for the current
> process, which isn't a concern anymore as we don't disable the check
> now.
>
> If you're fine with those changes, I can update the patch when pushing,
> there's no need to send a v3.
>

Yes I am mostly fine with the changes.

>                */
> > -#if defined(__SANITIZE_ADDRESS__) && !defined(__clang__) && __GNUC__ < 8
> > -             return TestSkip;
> > -#endif
> > -             setenv("ASAN_OPTIONS", "verify_asan_link_order=0", 1);
> > +             gst_registry_fork_set_enabled(false);
> >
> >               /* Initialize GStreamer */
> >               g_autoptr(GError) errInit = NULL;
>
> --
> Regards,
>
> Laurent Pinchart


>
Regards
Vedant Paranjape
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210822/fe615a8d/attachment.htm>