[{"id":19887,"web_url":"https://patchwork.libcamera.org/comment/19887/","msgid":"","date":"2021-09-27T14:04:51","subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit :\n> libunwind is an alternative to glibc's backtrace() to extract a\n> backtrace. Use it when available to extend backtrace support to more\n> platforms.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/base/backtrace.h | 3 ++\n> src/libcamera/base/backtrace.cpp | 61 ++++++++++++++++++++++++++++--\n> src/libcamera/base/meson.build | 6 +++\n> 3 files changed, 67 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h\n> index aefc76defa83..58ccc14c8f81 100644\n> --- a/include/libcamera/base/backtrace.h\n> +++ b/include/libcamera/base/backtrace.h\n> @@ -26,6 +26,9 @@ public:\n> private:\n> \tLIBCAMERA_DISABLE_COPY(Backtrace)\n> \n> +\tbool backtraceTrace();\n> +\tbool unwindTrace();\n> +\n> \tstd::vector backtrace_;\n> };\n> \n> diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp\n> index 011f2e428d5d..40fa60d0f9bf 100644\n> --- a/src/libcamera/base/backtrace.cpp\n> +++ b/src/libcamera/base/backtrace.cpp\n> @@ -18,6 +18,15 @@\n> #include \n> #endif\n> \n> +#if HAVE_UNWIND\n> +/*\n> + * Disable support for remote unwinding to enable a more optimized\n> + * implementation.\n> + */\n> +#define UNW_LOCAL_ONLY\n> +#include \n> +#endif\n> +\n> #include \n> \n> #include \n> @@ -146,6 +155,15 @@ std::string DwflParser::stackEntry(const void *ip)\n> * It can later be converted to a string with toString().\n> */\n> Backtrace::Backtrace()\n> +{\n> +\t/* Try libunwind first and fall back to backtrace() if it fails. */\n> +\tif (unwindTrace())\n> +\t\treturn;\n> +\n> +\tbacktraceTrace();\n> +}\n> +\n> +bool Backtrace::backtraceTrace()\n> {\n> #if HAVE_BACKTRACE\n> \tbacktrace_.resize(32);\n> @@ -153,10 +171,44 @@ Backtrace::Backtrace()\n> \tint num_entries = backtrace(backtrace_.data(), backtrace_.size());\n> \tif (num_entries < 0) {\n> \t\tbacktrace_.clear();\n> -\t\treturn;\n> +\t\treturn false;\n> \t}\n> \n> \tbacktrace_.resize(num_entries);\n> +\n> +\treturn true;\n> +#else\n> +\treturn false;\n> +#endif\n> +}\n> +\n> +bool Backtrace::unwindTrace()\n> +{\n> +#if HAVE_UNWIND\n> +\tunw_context_t uc;\n> +\tint ret = unw_getcontext(&uc);\n> +\tif (ret)\n> +\t\treturn false;\n> +\n> +\tunw_cursor_t cursor;\n> +\tret = unw_init_local(&cursor, &uc);\n> +\tif (ret)\n> +\t\treturn false;\n> +\n> +\tdo {\n> +\t\tunw_word_t ip;\n> +\t\tret = unw_get_reg(&cursor, UNW_REG_IP, &ip);\n> +\t\tif (ret) {\n> +\t\t\tbacktrace_.push_back(nullptr);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tbacktrace_.push_back(reinterpret_cast(ip));\n> +\t} while (unw_step(&cursor) > 0);\n> +\n> +\treturn true;\n> +#else\n> +\treturn false;\n> #endif\n> }\n> \n> @@ -181,8 +233,11 @@ Backtrace::Backtrace()\n> */\n> std::string Backtrace::toString(unsigned int skipLevels) const\n> {\n> -\t/* Skip the first entry, corresponding to the Backtrace construction. */\n> -\tskipLevels += 1;\n> +\t/*\n> +\t * Skip the first two entries, corresponding to the Backtrace\n> +\t * construction.\n> +\t */\n> +\tskipLevels += 2;\n\nIs this value specific to unwind ? Or perhaps you simply had it wrong in the\noriginal ? Perhaps you could fix the original or comment ?\n\n> \n> \tif (backtrace_.size() <= skipLevels)\n> \t\treturn std::string();\n> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> index 1fa894cf1896..8d14a65648fc 100644\n> --- a/src/libcamera/base/meson.build\n> +++ b/src/libcamera/base/meson.build\n> @@ -20,6 +20,7 @@ libcamera_base_sources = files([\n> ])\n> \n> libdw = cc.find_library('libdw', required : false)\n> +libunwind = cc.find_library('libunwind', required : false)\n> \n> if cc.has_header_symbol('execinfo.h', 'backtrace')\n> config_h.set('HAVE_BACKTRACE', 1)\n> @@ -29,10 +30,15 @@ if libdw.found()\n> config_h.set('HAVE_DW', 1)\n> endif\n> \n> +if libunwind.found()\n> + config_h.set('HAVE_UNWIND', 1)\n> +endif\n> +\n> libcamera_base_deps = [\n> dependency('threads'),\n> libatomic,\n> libdw,\n> + libunwind,\n> ]\n> \n> # Internal components must use the libcamera_base_private dependency to enable","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D1F41BDC71\n\tfor ;\n\tMon, 27 Sep 2021 14:04:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 57CB86012C;\n\tMon, 27 Sep 2021 16:04:58 +0200 (CEST)","from mail-qt1-x82b.google.com (mail-qt1-x82b.google.com\n\t[IPv6:2607:f8b0:4864:20::82b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE6F36012C\n\tfor ;\n\tMon, 27 Sep 2021 16:04:56 +0200 (CEST)","by mail-qt1-x82b.google.com with SMTP id f15so3828480qtv.9\n\tfor ;\n\tMon, 27 Sep 2021 07:04:56 -0700 (PDT)","from nicolas-tpx395.localdomain (173-246-12-168.qc.cable.ebox.net.\n\t[173.246.12.168]) by smtp.gmail.com with ESMTPSA id\n\td5sm10680342qtr.61.2021.09.27.07.04.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 27 Sep 2021 07:04:52 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20210112.gappssmtp.com\n\theader.i=@ndufresne-ca.20210112.gappssmtp.com\n\theader.b=\"aoQa/RyV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20210112.gappssmtp.com; s=20210112;\n\th=message-id:subject:from:to:date:in-reply-to:references:user-agent\n\t:mime-version:content-transfer-encoding;\n\tbh=vaupNXFO8XWjqiZHers/uwrzK2myxMg0OpVSPxGk1Ns=;\n\tb=aoQa/RyVGgw94Eou5McX/j+v/lbEeXdmVVIwdKQRdg14rlmX8y30W+LYfIeScj1G1o\n\t7flbsP8wcDdz36uZd404RmzSV68cmMYOvC3plXGtyA2IXk8xPf1/7m7m8hBslRRv6sCe\n\tspwU9RjqMgHPo6oPfYsy/dkIw9k11FtvwAPGT42/icbQa6aL8mYnZVe9qktlizmv8mnu\n\tW7k0Cj1pxj+ih3RDyuzG6WSQb2FAqqGPQfg0RLH+XbdYR1UXppuUGPQZNdUfKpiKDBz1\n\tokJRhnIlHcBnDSXC9iS8f/5OkMyOggwuDbx9J7RFnKciwWWyaB3wqcl7TmaoW+9WG7li\n\tQ5Sg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:subject:from:to:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=vaupNXFO8XWjqiZHers/uwrzK2myxMg0OpVSPxGk1Ns=;\n\tb=lK781wSmNmPZwdIBd8KMWvHp7pn97VfFVoxzURYLMI6PawyJvUPdKbuMxeuCHMTWgz\n\t+T3ZqWd1AS4qoqkH/h5ZeX6NBWuuLAUvOd2Qw1VjHEHLc2EcELS5BLWpYsLVbgs5MMha\n\tD4kjBQaSJwQwDHCVECnuKX1yT+nF6dpb3NNkrVVuyv5GOJm8HtRgRfv2gBucowN4duz9\n\tFHgs83N448Sq3hKJw/eZa7rfRiFFK+wAVchDQLW2xxw5gtf4ZgVUOoj9RxXCK86J7fYb\n\tkVDYKC7nxOiUezkhpgGEA2LHlCDu0sRDqF0484VJpibEWdnoVFq1pSAdGIPuepLO1x09\n\t+KPA==","X-Gm-Message-State":"AOAM531zmQ62mhSP1JbWBRVRZRAZSuA8EE+EqYKTnmmjmG6dto+gcXgQ\n\tL22dT/UL0a0bSh4az1HF7B86yQ==","X-Google-Smtp-Source":"ABdhPJyXYosrhQcj9q17nri2hNkVfsExrzb73T1KkpTZCcCSipW4yiK+HmALyLNBuMTsdysWT8DXEw==","X-Received":"by 2002:ac8:5a86:: with SMTP id c6mr2028288qtc.3.1632751492641; \n\tMon, 27 Sep 2021 07:04:52 -0700 (PDT)","Message-ID":"","From":"Nicolas Dufresne ","To":"Laurent Pinchart , \n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 27 Sep 2021 10:04:51 -0400","In-Reply-To":"<20210924102323.26787-4-laurent.pinchart@ideasonboard.com>","References":"<20210924102323.26787-1-laurent.pinchart@ideasonboard.com>\n\t<20210924102323.26787-4-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19911,"web_url":"https://patchwork.libcamera.org/comment/19911/","msgid":"","date":"2021-09-28T02:35:11","subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Mon, Sep 27, 2021 at 10:04:51AM -0400, Nicolas Dufresne wrote:\n> Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit :\n> > libunwind is an alternative to glibc's backtrace() to extract a\n> > backtrace. Use it when available to extend backtrace support to more\n> > platforms.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/base/backtrace.h | 3 ++\n> > src/libcamera/base/backtrace.cpp | 61 ++++++++++++++++++++++++++++--\n> > src/libcamera/base/meson.build | 6 +++\n> > 3 files changed, 67 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h\n> > index aefc76defa83..58ccc14c8f81 100644\n> > --- a/include/libcamera/base/backtrace.h\n> > +++ b/include/libcamera/base/backtrace.h\n> > @@ -26,6 +26,9 @@ public:\n> > private:\n> > \tLIBCAMERA_DISABLE_COPY(Backtrace)\n> > \n> > +\tbool backtraceTrace();\n> > +\tbool unwindTrace();\n> > +\n> > \tstd::vector backtrace_;\n> > };\n> > \n> > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp\n> > index 011f2e428d5d..40fa60d0f9bf 100644\n> > --- a/src/libcamera/base/backtrace.cpp\n> > +++ b/src/libcamera/base/backtrace.cpp\n> > @@ -18,6 +18,15 @@\n> > #include \n> > #endif\n> > \n> > +#if HAVE_UNWIND\n> > +/*\n> > + * Disable support for remote unwinding to enable a more optimized\n> > + * implementation.\n> > + */\n> > +#define UNW_LOCAL_ONLY\n> > +#include \n> > +#endif\n> > +\n> > #include \n> > \n> > #include \n> > @@ -146,6 +155,15 @@ std::string DwflParser::stackEntry(const void *ip)\n> > * It can later be converted to a string with toString().\n> > */\n> > Backtrace::Backtrace()\n> > +{\n> > +\t/* Try libunwind first and fall back to backtrace() if it fails. */\n> > +\tif (unwindTrace())\n> > +\t\treturn;\n> > +\n> > +\tbacktraceTrace();\n> > +}\n> > +\n> > +bool Backtrace::backtraceTrace()\n> > {\n> > #if HAVE_BACKTRACE\n> > \tbacktrace_.resize(32);\n> > @@ -153,10 +171,44 @@ Backtrace::Backtrace()\n> > \tint num_entries = backtrace(backtrace_.data(), backtrace_.size());\n> > \tif (num_entries < 0) {\n> > \t\tbacktrace_.clear();\n> > -\t\treturn;\n> > +\t\treturn false;\n> > \t}\n> > \n> > \tbacktrace_.resize(num_entries);\n> > +\n> > +\treturn true;\n> > +#else\n> > +\treturn false;\n> > +#endif\n> > +}\n> > +\n> > +bool Backtrace::unwindTrace()\n> > +{\n> > +#if HAVE_UNWIND\n> > +\tunw_context_t uc;\n> > +\tint ret = unw_getcontext(&uc);\n> > +\tif (ret)\n> > +\t\treturn false;\n> > +\n> > +\tunw_cursor_t cursor;\n> > +\tret = unw_init_local(&cursor, &uc);\n> > +\tif (ret)\n> > +\t\treturn false;\n> > +\n> > +\tdo {\n> > +\t\tunw_word_t ip;\n> > +\t\tret = unw_get_reg(&cursor, UNW_REG_IP, &ip);\n> > +\t\tif (ret) {\n> > +\t\t\tbacktrace_.push_back(nullptr);\n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > +\t\tbacktrace_.push_back(reinterpret_cast(ip));\n> > +\t} while (unw_step(&cursor) > 0);\n> > +\n> > +\treturn true;\n> > +#else\n> > +\treturn false;\n> > #endif\n> > }\n> > \n> > @@ -181,8 +233,11 @@ Backtrace::Backtrace()\n> > */\n> > std::string Backtrace::toString(unsigned int skipLevels) const\n> > {\n> > -\t/* Skip the first entry, corresponding to the Backtrace construction. */\n> > -\tskipLevels += 1;\n> > +\t/*\n> > +\t * Skip the first two entries, corresponding to the Backtrace\n> > +\t * construction.\n> > +\t */\n> > +\tskipLevels += 2;\n> \n> Is this value specific to unwind ? Or perhaps you simply had it wrong in the\n> original ? Perhaps you could fix the original or comment ?\n\nIt was 1 before because backtrace() was called directly in\nBacktrace::Backtrace(), while now there are two helper functions\n(backtraceTrace() and unwindTrace()) called by the constructor.\n\n> > \n> > \tif (backtrace_.size() <= skipLevels)\n> > \t\treturn std::string();\n> > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> > index 1fa894cf1896..8d14a65648fc 100644\n> > --- a/src/libcamera/base/meson.build\n> > +++ b/src/libcamera/base/meson.build\n> > @@ -20,6 +20,7 @@ libcamera_base_sources = files([\n> > ])\n> > \n> > libdw = cc.find_library('libdw', required : false)\n> > +libunwind = cc.find_library('libunwind', required : false)\n> > \n> > if cc.has_header_symbol('execinfo.h', 'backtrace')\n> > config_h.set('HAVE_BACKTRACE', 1)\n> > @@ -29,10 +30,15 @@ if libdw.found()\n> > config_h.set('HAVE_DW', 1)\n> > endif\n> > \n> > +if libunwind.found()\n> > + config_h.set('HAVE_UNWIND', 1)\n> > +endif\n> > +\n> > libcamera_base_deps = [\n> > dependency('threads'),\n> > libatomic,\n> > libdw,\n> > + libunwind,\n> > ]\n> > \n> > # Internal components must use the libcamera_base_private dependency to enable","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 43741C3243\n\tfor ;\n\tTue, 28 Sep 2021 02:35:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1062069189;\n\tTue, 28 Sep 2021 04:35:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E640A684C7\n\tfor ;\n\tTue, 28 Sep 2021 04:35:18 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7C39A3F1;\n\tTue, 28 Sep 2021 04:35:18 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BDtNJ9Vc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632796518;\n\tbh=cVqrd5/OwBRvfS1KXGPUzULooLd6h1y0Pf9bzkACG/w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BDtNJ9VckPFAfodTriMSHnTfbx1K/pWE3Apwh3XtVh9YBzLBG9Q8K+auEjmmNH7i3\n\tltopECOdF2SSERdOUB48d2uEXvZKUo8jrQxMY/RIedOX5VSXmZdpIYyhYMI/Vw+0p+\n\tQX5f1zwDGqzAsMduutyjLt2dH/2WVVpJbL+lgRCU=","Date":"Tue, 28 Sep 2021 05:35:11 +0300","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Message-ID":"","References":"<20210924102323.26787-1-laurent.pinchart@ideasonboard.com>\n\t<20210924102323.26787-4-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19931,"web_url":"https://patchwork.libcamera.org/comment/19931/","msgid":"<975196fed35cadd4699b4e2d49c775d141ae4b2c.camel@ndufresne.ca>","date":"2021-09-28T14:27:18","subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le mardi 28 septembre 2021 à 05:35 +0300, Laurent Pinchart a écrit :\n> Hi Nicolas,\n> \n> On Mon, Sep 27, 2021 at 10:04:51AM -0400, Nicolas Dufresne wrote:\n> > Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit :\n> > > libunwind is an alternative to glibc's backtrace() to extract a\n> > > backtrace. Use it when available to extend backtrace support to more\n> > > platforms.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/base/backtrace.h | 3 ++\n> > > src/libcamera/base/backtrace.cpp | 61 ++++++++++++++++++++++++++++--\n> > > src/libcamera/base/meson.build | 6 +++\n> > > 3 files changed, 67 insertions(+), 3 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h\n> > > index aefc76defa83..58ccc14c8f81 100644\n> > > --- a/include/libcamera/base/backtrace.h\n> > > +++ b/include/libcamera/base/backtrace.h\n> > > @@ -26,6 +26,9 @@ public:\n> > > private:\n> > > \tLIBCAMERA_DISABLE_COPY(Backtrace)\n> > > \n> > > +\tbool backtraceTrace();\n> > > +\tbool unwindTrace();\n> > > +\n> > > \tstd::vector backtrace_;\n> > > };\n> > > \n> > > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp\n> > > index 011f2e428d5d..40fa60d0f9bf 100644\n> > > --- a/src/libcamera/base/backtrace.cpp\n> > > +++ b/src/libcamera/base/backtrace.cpp\n> > > @@ -18,6 +18,15 @@\n> > > #include \n> > > #endif\n> > > \n> > > +#if HAVE_UNWIND\n> > > +/*\n> > > + * Disable support for remote unwinding to enable a more optimized\n> > > + * implementation.\n> > > + */\n> > > +#define UNW_LOCAL_ONLY\n> > > +#include \n> > > +#endif\n> > > +\n> > > #include \n> > > \n> > > #include \n> > > @@ -146,6 +155,15 @@ std::string DwflParser::stackEntry(const void *ip)\n> > > * It can later be converted to a string with toString().\n> > > */\n> > > Backtrace::Backtrace()\n> > > +{\n> > > +\t/* Try libunwind first and fall back to backtrace() if it fails. */\n> > > +\tif (unwindTrace())\n> > > +\t\treturn;\n> > > +\n> > > +\tbacktraceTrace();\n> > > +}\n> > > +\n> > > +bool Backtrace::backtraceTrace()\n> > > {\n> > > #if HAVE_BACKTRACE\n> > > \tbacktrace_.resize(32);\n> > > @@ -153,10 +171,44 @@ Backtrace::Backtrace()\n> > > \tint num_entries = backtrace(backtrace_.data(), backtrace_.size());\n> > > \tif (num_entries < 0) {\n> > > \t\tbacktrace_.clear();\n> > > -\t\treturn;\n> > > +\t\treturn false;\n> > > \t}\n> > > \n> > > \tbacktrace_.resize(num_entries);\n> > > +\n> > > +\treturn true;\n> > > +#else\n> > > +\treturn false;\n> > > +#endif\n> > > +}\n> > > +\n> > > +bool Backtrace::unwindTrace()\n> > > +{\n> > > +#if HAVE_UNWIND\n> > > +\tunw_context_t uc;\n> > > +\tint ret = unw_getcontext(&uc);\n> > > +\tif (ret)\n> > > +\t\treturn false;\n> > > +\n> > > +\tunw_cursor_t cursor;\n> > > +\tret = unw_init_local(&cursor, &uc);\n> > > +\tif (ret)\n> > > +\t\treturn false;\n> > > +\n> > > +\tdo {\n> > > +\t\tunw_word_t ip;\n> > > +\t\tret = unw_get_reg(&cursor, UNW_REG_IP, &ip);\n> > > +\t\tif (ret) {\n> > > +\t\t\tbacktrace_.push_back(nullptr);\n> > > +\t\t\tcontinue;\n> > > +\t\t}\n> > > +\n> > > +\t\tbacktrace_.push_back(reinterpret_cast(ip));\n> > > +\t} while (unw_step(&cursor) > 0);\n> > > +\n> > > +\treturn true;\n> > > +#else\n> > > +\treturn false;\n> > > #endif\n> > > }\n> > > \n> > > @@ -181,8 +233,11 @@ Backtrace::Backtrace()\n> > > */\n> > > std::string Backtrace::toString(unsigned int skipLevels) const\n> > > {\n> > > -\t/* Skip the first entry, corresponding to the Backtrace construction. */\n> > > -\tskipLevels += 1;\n> > > +\t/*\n> > > +\t * Skip the first two entries, corresponding to the Backtrace\n> > > +\t * construction.\n> > > +\t */\n> > > +\tskipLevels += 2;\n> > \n> > Is this value specific to unwind ? Or perhaps you simply had it wrong in the\n> > original ? Perhaps you could fix the original or comment ?\n> \n> It was 1 before because backtrace() was called directly in\n> Backtrace::Backtrace(), while now there are two helper functions\n> (backtraceTrace() and unwindTrace()) called by the constructor.\n\nThank you, I had totally missed the point of why we had to skip. Now said, its\nsounds all very obvious. Now I have a new concern, what if the compiler auto-\ninline some of this ? Do we need to add something to prevent this, or is it all\nsafe ?\n\n> \n> > > \n> > > \tif (backtrace_.size() <= skipLevels)\n> > > \t\treturn std::string();\n> > > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> > > index 1fa894cf1896..8d14a65648fc 100644\n> > > --- a/src/libcamera/base/meson.build\n> > > +++ b/src/libcamera/base/meson.build\n> > > @@ -20,6 +20,7 @@ libcamera_base_sources = files([\n> > > ])\n> > > \n> > > libdw = cc.find_library('libdw', required : false)\n> > > +libunwind = cc.find_library('libunwind', required : false)\n> > > \n> > > if cc.has_header_symbol('execinfo.h', 'backtrace')\n> > > config_h.set('HAVE_BACKTRACE', 1)\n> > > @@ -29,10 +30,15 @@ if libdw.found()\n> > > config_h.set('HAVE_DW', 1)\n> > > endif\n> > > \n> > > +if libunwind.found()\n> > > + config_h.set('HAVE_UNWIND', 1)\n> > > +endif\n> > > +\n> > > libcamera_base_deps = [\n> > > dependency('threads'),\n> > > libatomic,\n> > > libdw,\n> > > + libunwind,\n> > > ]\n> > > \n> > > # Internal components must use the libcamera_base_private dependency to enable\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E1B03C3243\n\tfor ;\n\tTue, 28 Sep 2021 14:27:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 462846918C;\n\tTue, 28 Sep 2021 16:27:23 +0200 (CEST)","from mail-qt1-x836.google.com (mail-qt1-x836.google.com\n\t[IPv6:2607:f8b0:4864:20::836])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5964269185\n\tfor ;\n\tTue, 28 Sep 2021 16:27:21 +0200 (CEST)","by mail-qt1-x836.google.com with SMTP id r1so19977257qta.12\n\tfor ;\n\tTue, 28 Sep 2021 07:27:21 -0700 (PDT)","from nicolas-tpx395.localdomain (173-246-12-168.qc.cable.ebox.net.\n\t[173.246.12.168]) by smtp.gmail.com with ESMTPSA id\n\t205sm15401921qkf.19.2021.09.28.07.27.19\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 28 Sep 2021 07:27:19 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20210112.gappssmtp.com\n\theader.i=@ndufresne-ca.20210112.gappssmtp.com\n\theader.b=\"fYNGM2hp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20210112.gappssmtp.com; s=20210112;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:user-agent:mime-version:content-transfer-encoding;\n\tbh=9gc/cBa3MtmkgFkPw1CQN8OGky4WgYOY88TC/Fwkdmo=;\n\tb=fYNGM2hpXHpQiWYdJrZ7niZ0+DesiZxOMgv9Q/yvMvFucTsIKuD51tl3nRaWDhhSzQ\n\tIuKyWxMC+zWw38MfNbGG4Q605PepZqZcPdqGGm3ZFgG2I1HtEjfLcAv+15VFI7dWUQwM\n\tjccQIu+4jsEwbC67exPrCFpbOHvJK9Y2SZwECoijRgQT27Iy7iGDQ1u0W5GdmOILnU4O\n\tsNMd+FpL15bZIHxVTcUrajs55/lBqEfiqX7OxjHzld6LOF+PDud0FNposr7kItA2pMUr\n\tHc3XX0Cl7lUhVIWkbdf+VS8MMoTUfTe6VZIYHMxwHH1bIOUVC85OWVtqefNjQ2ZFvUTA\n\t76yw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=9gc/cBa3MtmkgFkPw1CQN8OGky4WgYOY88TC/Fwkdmo=;\n\tb=6YGyyPyTCRghVLnGcpW7yY3LGKao7u8S3icXOdcn4U8Hwwxva5MmqvRcOa4l5vU/Dy\n\t5BlOQnt0OH4oXwjH+rum1S/3wFIevzU0k37oXtuz15ejZnm2+9FbN3wUQbn8tLoZAcjB\n\tNNh6XSMY9OZcQt31p7d5INGnVauLG4UL2u9DpJ1ZO8dLSAB4WYG2LtGVT/nMH46sdOPm\n\tdJ0Z8T1TLs88C6UNUpHv/WECbe9pF4ZbcFC0m8zUzEcZZKzKjEh5V8hqZ/t/7Mw+vJ/W\n\tH/yzSVAliREDC2ZtjXbiWsAbTpB2m9KJZ8ictitpcWJtoBTShgtA5VF3uTmsyimUxw7f\n\tIO0Q==","X-Gm-Message-State":"AOAM5305iGwHmbxNDdJ1dgv7EfblPfT3yrZ/kGpBMC3ps5R27MV+UzVt\n\t3tQCAbEFR2Bppqk8orV7FkUPWw==","X-Google-Smtp-Source":"ABdhPJzWZu1XixyNZ02YZOKDjR08OrkYtDZo3ZJPiyOxdlsbxI6A79UIq+7i6Rzh3lT0jZdup00YNA==","X-Received":"by 2002:ac8:4802:: with SMTP id g2mr5976857qtq.217.1632839240238;\n\tTue, 28 Sep 2021 07:27:20 -0700 (PDT)","Message-ID":"<975196fed35cadd4699b4e2d49c775d141ae4b2c.camel@ndufresne.ca>","From":"Nicolas Dufresne ","To":"Laurent Pinchart ","Date":"Tue, 28 Sep 2021 10:27:18 -0400","In-Reply-To":"","References":"<20210924102323.26787-1-laurent.pinchart@ideasonboard.com>\n\t<20210924102323.26787-4-laurent.pinchart@ideasonboard.com>\n\t\n\t","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.40.4 (3.40.4-1.fc34) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19951,"web_url":"https://patchwork.libcamera.org/comment/19951/","msgid":"","date":"2021-09-28T22:10:14","subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Tue, Sep 28, 2021 at 10:27:18AM -0400, Nicolas Dufresne wrote:\n> Le mardi 28 septembre 2021 à 05:35 +0300, Laurent Pinchart a écrit :\n> > On Mon, Sep 27, 2021 at 10:04:51AM -0400, Nicolas Dufresne wrote:\n> > > Le vendredi 24 septembre 2021 à 13:23 +0300, Laurent Pinchart a écrit :\n> > > > libunwind is an alternative to glibc's backtrace() to extract a\n> > > > backtrace. Use it when available to extend backtrace support to more\n> > > > platforms.\n> > > > \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > include/libcamera/base/backtrace.h | 3 ++\n> > > > src/libcamera/base/backtrace.cpp | 61 ++++++++++++++++++++++++++++--\n> > > > src/libcamera/base/meson.build | 6 +++\n> > > > 3 files changed, 67 insertions(+), 3 deletions(-)\n> > > > \n> > > > diff --git a/include/libcamera/base/backtrace.h b/include/libcamera/base/backtrace.h\n> > > > index aefc76defa83..58ccc14c8f81 100644\n> > > > --- a/include/libcamera/base/backtrace.h\n> > > > +++ b/include/libcamera/base/backtrace.h\n> > > > @@ -26,6 +26,9 @@ public:\n> > > > private:\n> > > > \tLIBCAMERA_DISABLE_COPY(Backtrace)\n> > > > \n> > > > +\tbool backtraceTrace();\n> > > > +\tbool unwindTrace();\n> > > > +\n> > > > \tstd::vector backtrace_;\n> > > > };\n> > > > \n> > > > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp\n> > > > index 011f2e428d5d..40fa60d0f9bf 100644\n> > > > --- a/src/libcamera/base/backtrace.cpp\n> > > > +++ b/src/libcamera/base/backtrace.cpp\n> > > > @@ -18,6 +18,15 @@\n> > > > #include \n> > > > #endif\n> > > > \n> > > > +#if HAVE_UNWIND\n> > > > +/*\n> > > > + * Disable support for remote unwinding to enable a more optimized\n> > > > + * implementation.\n> > > > + */\n> > > > +#define UNW_LOCAL_ONLY\n> > > > +#include \n> > > > +#endif\n> > > > +\n> > > > #include \n> > > > \n> > > > #include \n> > > > @@ -146,6 +155,15 @@ std::string DwflParser::stackEntry(const void *ip)\n> > > > * It can later be converted to a string with toString().\n> > > > */\n> > > > Backtrace::Backtrace()\n> > > > +{\n> > > > +\t/* Try libunwind first and fall back to backtrace() if it fails. */\n> > > > +\tif (unwindTrace())\n> > > > +\t\treturn;\n> > > > +\n> > > > +\tbacktraceTrace();\n> > > > +}\n> > > > +\n> > > > +bool Backtrace::backtraceTrace()\n> > > > {\n> > > > #if HAVE_BACKTRACE\n> > > > \tbacktrace_.resize(32);\n> > > > @@ -153,10 +171,44 @@ Backtrace::Backtrace()\n> > > > \tint num_entries = backtrace(backtrace_.data(), backtrace_.size());\n> > > > \tif (num_entries < 0) {\n> > > > \t\tbacktrace_.clear();\n> > > > -\t\treturn;\n> > > > +\t\treturn false;\n> > > > \t}\n> > > > \n> > > > \tbacktrace_.resize(num_entries);\n> > > > +\n> > > > +\treturn true;\n> > > > +#else\n> > > > +\treturn false;\n> > > > +#endif\n> > > > +}\n> > > > +\n> > > > +bool Backtrace::unwindTrace()\n> > > > +{\n> > > > +#if HAVE_UNWIND\n> > > > +\tunw_context_t uc;\n> > > > +\tint ret = unw_getcontext(&uc);\n> > > > +\tif (ret)\n> > > > +\t\treturn false;\n> > > > +\n> > > > +\tunw_cursor_t cursor;\n> > > > +\tret = unw_init_local(&cursor, &uc);\n> > > > +\tif (ret)\n> > > > +\t\treturn false;\n> > > > +\n> > > > +\tdo {\n> > > > +\t\tunw_word_t ip;\n> > > > +\t\tret = unw_get_reg(&cursor, UNW_REG_IP, &ip);\n> > > > +\t\tif (ret) {\n> > > > +\t\t\tbacktrace_.push_back(nullptr);\n> > > > +\t\t\tcontinue;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tbacktrace_.push_back(reinterpret_cast(ip));\n> > > > +\t} while (unw_step(&cursor) > 0);\n> > > > +\n> > > > +\treturn true;\n> > > > +#else\n> > > > +\treturn false;\n> > > > #endif\n> > > > }\n> > > > \n> > > > @@ -181,8 +233,11 @@ Backtrace::Backtrace()\n> > > > */\n> > > > std::string Backtrace::toString(unsigned int skipLevels) const\n> > > > {\n> > > > -\t/* Skip the first entry, corresponding to the Backtrace construction. */\n> > > > -\tskipLevels += 1;\n> > > > +\t/*\n> > > > +\t * Skip the first two entries, corresponding to the Backtrace\n> > > > +\t * construction.\n> > > > +\t */\n> > > > +\tskipLevels += 2;\n> > > \n> > > Is this value specific to unwind ? Or perhaps you simply had it wrong in the\n> > > original ? Perhaps you could fix the original or comment ?\n> > \n> > It was 1 before because backtrace() was called directly in\n> > Backtrace::Backtrace(), while now there are two helper functions\n> > (backtraceTrace() and unwindTrace()) called by the constructor.\n> \n> Thank you, I had totally missed the point of why we had to skip. Now said, its\n> sounds all very obvious. Now I have a new concern, what if the compiler auto-\n> inline some of this ? Do we need to add something to prevent this, or is it all\n> safe ?\n\nYou ask very good questions :-) I've thought about it too, but didn't\nreach an obvious conclusion. We could use the __noinline__ attribute to\nensure that the functions don't get inlined:\n\nnoinline\n\n This function attribute prevents a function from being considered\n for inlining. If the function does not have side effects, there are\n optimizations other than inlining that cause function calls to be\n optimized away, although the function call is live. To keep such\n calls from being optimized away, put\n\n asm (\"\");\n\n (see Extended Asm) in the called function, to serve as a special\n side effect.\n\nWe'll lose possible optimizations, but laybe it's not a big deal here ?\n\nThe other option is to force inlining of those functions, but the\n\"inline\" keyword is a hint only. We could try the __always_inline__\nattribute, I'm not sure if it works for member functions though, and it\nmay not be as clearcut as its name implies:\n\nalways_inline\n\n Generally, functions are not inlined unless optimization is\n specified. For functions declared inline, this attribute inlines the\n function independent of any restrictions that otherwise apply to\n inlining. Failure to inline such a function is diagnosed as an\n error. Note that if such a function is called indirectly the\n compiler may or may not inline it depending on optimization level\n and a failure to inline an indirect call may or may not be\n diagnosed.\n\nI'm tempted to go for noinline.\n\n> > > > \n> > > > \tif (backtrace_.size() <= skipLevels)\n> > > > \t\treturn std::string();\n> > > > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> > > > index 1fa894cf1896..8d14a65648fc 100644\n> > > > --- a/src/libcamera/base/meson.build\n> > > > +++ b/src/libcamera/base/meson.build\n> > > > @@ -20,6 +20,7 @@ libcamera_base_sources = files([\n> > > > ])\n> > > > \n> > > > libdw = cc.find_library('libdw', required : false)\n> > > > +libunwind = cc.find_library('libunwind', required : false)\n> > > > \n> > > > if cc.has_header_symbol('execinfo.h', 'backtrace')\n> > > > config_h.set('HAVE_BACKTRACE', 1)\n> > > > @@ -29,10 +30,15 @@ if libdw.found()\n> > > > config_h.set('HAVE_DW', 1)\n> > > > endif\n> > > > \n> > > > +if libunwind.found()\n> > > > + config_h.set('HAVE_UNWIND', 1)\n> > > > +endif\n> > > > +\n> > > > libcamera_base_deps = [\n> > > > dependency('threads'),\n> > > > libatomic,\n> > > > libdw,\n> > > > + libunwind,\n> > > > ]\n> > > > \n> > > > # Internal components must use the libcamera_base_private dependency to enable","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4E358C3243\n\tfor ;\n\tTue, 28 Sep 2021 22:10:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD70F6918B;\n\tWed, 29 Sep 2021 00:10:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DDAF66012C\n\tfor ;\n\tWed, 29 Sep 2021 00:10:15 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5AEF8D87;\n\tWed, 29 Sep 2021 00:10:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jqnk0+8U\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632867015;\n\tbh=4pZ7Om8gCaU5FGPMPFVt3DQoNYhzZjn5vtwGKnLvXcE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jqnk0+8UVVgtBrOtm2bjcW8FKzSboJx5S3A39CeZPZL81O60M2oigfXTflUYGVf6F\n\tsewJVwlXtE5N6K8u3xxukGdkrLFc6Nv2XKLV7sO1upJ6RJJaJCYc0fBfbYVffimeei\n\tP/kwSlIv90NeF9TpgwX11GpvSxVdf3JXXJy+7zeM=","Date":"Wed, 29 Sep 2021 01:10:14 +0300","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Message-ID":"","References":"<20210924102323.26787-1-laurent.pinchart@ideasonboard.com>\n\t<20210924102323.26787-4-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t<975196fed35cadd4699b4e2d49c775d141ae4b2c.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<975196fed35cadd4699b4e2d49c775d141ae4b2c.camel@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH v1 3/4] libcamera: base: backtrace:\n\tUse libunwind when available","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]