[libcamera-devel] [PATCH] ipa: raspberrypi: Fix crash under LTO

[Naushir Patuck]

Hi all,


On Fri, 10 Mar 2023 at 13:41, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Dave,
>
> On Fri, Mar 10, 2023 at 11:58:53AM +0000, Dave Jones wrote:
> > On Fri, Mar 10, 2023 at 12:40:33PM +0200, Laurent Pinchart wrote:
> > [snip]
> > >> -static std::map<std::string, CamHelperCreateFunc> camHelpers;
> > >> +namespace {
> > >> +
> > >> +std::map<std::string, CamHelperCreateFunc> &camHelpers()
> > >> +{
> > >> +  static std::unique_ptr<std::map<std::string, CamHelperCreateFunc>>
> obj =
> > >> +          std::make_unique<std::map<std::string,
> CamHelperCreateFunc>>();
> > >> +  return *obj;
> > >
> > > I think you could simply write
> > >
> > >     static std::map<std::string, CamHelperCreateFunc> helpers;
> > >     return helpers;
> >
> > That would likely be fine, but I thought I'd just note that in my
> > original patch [1] I'd actually used "new" with a pointer instead of a
> > static object quite deliberately. As my C++ skills are caked in the dust
> > of grime of numerous years of neglect, I'd consulted the C++ FAQ and
> > noted that using a static pointer opens the possibility of a crash on
> > deinit [2] (I'm not certain it *would* crash, but I figured I'd take the
> > easy/safe way out and leave the OS to deal with the memory) by using the
> > solution posited at [3].
> >
> > [1]: https://github.com/raspberrypi/libcamera/pull/46/files
> >
> > [2]: https://isocpp.org/wiki/faq/ctors#construct-on-first-use-v2
> >
> > [3]: https://isocpp.org/wiki/faq/ctors#static-init-order-on-first-use
> >
> > That *might* be preferable, though I admit it looks ... somewhat ugly.
>
> Unless I'm mistaken, given that you're using a unique pointer, the
> object will get deleted at deinit time. It should thus be functionally
> equivalent to the static variable in v2.
>
> We *could* allocate the objects dynamically without std::unique_ptr to
> avoid deinit issues, but we would then leak memory. That won't cause any
> issue in production, but for development it's nice if we can run
> libcamera applications under valgrind and expect no leaks.
>

The unique_ptr was my addition to avoid leaks in debug.
I think using a static object like in v2 ought to be good for this fix.

Regards,
Naush



>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20230310/8d4e90a0/attachment.htm>