<div dir="auto"><div>Hi Laurent,<div dir="auto">Thanks for the feedback.</div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, 22 Aug, 2021, 06:37 Laurent Pinchart, <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Vedant,<br>
<br>
Thank you for the patch.<br>
<br>
Looks like you've found an answer to the question you asked on my review<br>
of v1 :-)<br>
<br>
On Sat, Aug 21, 2021 at 08:11:45PM +0530, Vedant Paranjape wrote:<br>
> ASan needs to be loaded first before gstreamer is loaded. This was not<br>
> possible, so verify_asan_link_order was disabled. Better way to tackle<br>
> this issue was disabling forks on the gstreamer side.<br>
> <br>
> verify_asan_link_order=0 disables the check on ASan side which checks if<br>
> ASan was loaded before any other DLLs. Since, gstreamer spawns a child<br>
<br>
s/DLLs/shared objects/ as we're on Linux, no Windows :-)<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Oops 😅</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> helper process while building the registry, we needed to disable this<br>
> check. But with gst_registry_fork_set_enabled() it is possible to<br>
> disable spawning this child helper process, so this ensures that ASan is<br>
> loaded before any other DLL is loaded.<br>
> <br>
> Signed-off-by: Vedant Paranjape <<a href="mailto:vedantparanjape160201@gmail.com" target="_blank" rel="noreferrer">vedantparanjape160201@gmail.com</a>><br>
> ---<br>
>  test/gstreamer/gstreamer_single_stream_test.cpp | 17 ++++++-----------<br>
>  1 file changed, 6 insertions(+), 11 deletions(-)<br>
> <br>
> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp<br>
> index 349dcfa4..80e652eb 100644<br>
> --- a/test/gstreamer/gstreamer_single_stream_test.cpp<br>
> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp<br>
> @@ -41,18 +41,13 @@ protected:<br>
<br>
The comment starts with "GStreamer by spawns a process ...". Now that we<br>
know this can be changed with gst_registry_fork_set_enabled(), I think<br>
this should be changed to "GStreamer by default spawns a process ...".<br>
<br>
>                * GStreamer is most likely not, this will cause the ASan link<br>
>                * order check to fail when gst-plugin-scanner dlopen()s the<br>
>                * plugin as many libraries will have already been loaded by<br>
> -              * then. Work around this issue by disabling the link order<br>
> -              * check. This will only affect child processes, as ASan is<br>
> -              * already loaded for this process by the time this code is<br>
> -              * executed, and should thus hopefully be safe.<br>
> -              *<br>
> -              * This option is not available in gcc older than 8, the only<br>
> -              * option in that case is to skip the test.<br>
> +              * then. Work around this issue by disabling spawning of a<br>
> +              * child helper process when rebuilding the registry. This will<br>
<br>
How about "when scanning the build directory for plugins" instead of<br>
"when rebuilding the registry", as the problems occurs when<br>
gst_registry_scan_path() is called ?<br>
<br>
> +              * only affect child processes, as ASan is already loaded for<br>
> +              * this process by the time this code is executed, and should<br>
> +              * thus hopefully be safe.<br>
<br>
I think the last sentence can be dropped. I wrote it to document that<br>
verify_asan_link_order=0 didn't disable the check for the current<br>
process, which isn't a concern anymore as we don't disable the check<br>
now.<br>
<br>
If you're fine with those changes, I can update the patch when pushing,<br>
there's no need to send a v3.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Yes I am mostly fine with the changes.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>                */<br>
> -#if defined(__SANITIZE_ADDRESS__) && !defined(__clang__) && __GNUC__ < 8<br>
> -             return TestSkip;<br>
> -#endif<br>
> -             setenv("ASAN_OPTIONS", "verify_asan_link_order=0", 1);<br>
> +             gst_registry_fork_set_enabled(false);<br>
>  <br>
>               /* Initialize GStreamer */<br>
>               g_autoptr(GError) errInit = NULL;<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Regards</div><div dir="auto">Vedant Paranjape</div><div dir="auto"></div></div>