From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 50F236EC55; Mon, 12 Jul 2021 15:09:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 50F236EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1626091755; bh=gsfcQs7vOTbfRLRerezuUiocakvE7ZIgn1hTFz0lZHs=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=KRp9WpNX5j4DYhO1ktUsYTdkpowYjTj8tQj2wVvGOSRrmrqFpSDAsyCZvcgeGteX3 Kxq6802FLZfu33ChUa3Ig1C22y7HyGYKWWZgCZnpbnsyz7frSk0TAsKhunkq89gLH1 5B3e136d94fqxbeaFcOMPgdGumyyFtu97DLB7PpU= Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5D0C66EC55 for ; Mon, 12 Jul 2021 15:09:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5D0C66EC55 Received: by mail-lf1-f44.google.com with SMTP id x25so30059494lfu.13 for ; Mon, 12 Jul 2021 05:09:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=x3Dy8QchL/iusMODjzMfGwuqlDrSjUub+YMjxmvL+hs=; b=uRFwIEt5Bd8nvAoNV97ri056UvhFV8PrmWKw5ujsvHP2/IXM8nuEnfdy551gkBx5YZ WSbPu97PRpkJ7xruisyxN2kRC25fyvW3xp+zXn4h+3EChFsaF7LH8mm8AhPVOlKCD23N IwAFUcFatlU4B7YrJaTFli6wDc7Hvx/KlUK7geLR3VsnGpSMd+xmGDW0Kz8Otvvm4rLh fsIUzvgsfWciEnuqRyRwfnCm4ulo00+ShMUK5xt3h0PXEZiMWtnhIZp6u33TxFwSOfpk 9gMLBJAl/yhtDzlcgi+TAsmxTxFfS2HK1xwuRbaiWbdoMxg4ePAhupVFnfsK0ezP3ihb 7ScA== X-Gm-Message-State: AOAM530WqcA0fUSdvA3euFim7zy6K82AK6/Gj3Pst4zw3LVtPCrlbbkB 4i1RJqFH8DM7yCqBTBlAFrDUDPC08HD78g== X-Google-Smtp-Source: ABdhPJyPDScd97tYole1LR5gFZnFqzTd9kqmexeFawfzZHnPY9Yn6aUbDPGX27JYkPbaA1ZpqgBsDA== X-Received: by 2002:ac2:5045:: with SMTP id a5mr39747785lfm.203.1626091753492; Mon, 12 Jul 2021 05:09:13 -0700 (PDT) Received: from grain.localdomain ([5.18.199.94]) by smtp.gmail.com with ESMTPSA id f7sm1555969ljn.41.2021.07.12.05.09.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 12 Jul 2021 05:09:12 -0700 (PDT) Received: by grain.localdomain (Postfix, from userid 1000) id 042165A001E; Mon, 12 Jul 2021 15:09:12 +0300 (MSK) Date: Mon, 12 Jul 2021 15:09:11 +0300 To: Egor Elchinov via Tarantool-patches Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.0.7 (2021-05-04) Subject: Re: [Tarantool-patches] [PATCH v2 2/4] fiber: add option and PoC for Lua parent backtrace X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Cyrill Gorcunov via Tarantool-patches Reply-To: Cyrill Gorcunov Cc: Egor Elchinov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Fri, Jul 09, 2021 at 02:03:51PM +0300, Egor Elchinov via Tarantool-patches wrote: > > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c > index 924ff3c82..dd26e2f13 100644 > --- a/src/lib/core/fiber.c > +++ b/src/lib/core/fiber.c > @@ -220,6 +220,10 @@ fiber_mprotect(void *addr, size_t len, int prot) > static __thread bool fiber_top_enabled = false; > #endif /* ENABLE_FIBER_TOP */ > > +#if ENABLE_BACKTRACE > +static __thread bool fiber_parent_bt_enabled = false; > +#endif /* ENABLE_BACKTRACE */ > + So this variable is declared as TLS in a manner of fiber_top functionality. You know I somehow find this fishy because it is unclear why it is done this way. I think we should revisit this moment for @fiber_top_enabled enabled as well but not in this series, so lets leave it as is. Please revisit snprintf calls in the patch and strncpy as well because they _do_not_ append terminating \0 symbol if destination size is not enough. I think we should add \0 explicitly all the time. I mean +static int +fiber_parent_backtrace_cb(int frameno, void *frameret, const char *func, + size_t offset, void *cb_ctx) +{ ... + char buf[512]; + int l = snprintf(buf, sizeof(buf), "#%-2d %p in ", frameno, frameret); + if (func) + snprintf(buf + l, sizeof(buf) - l, "%s+%zu", func, offset); + else + snprintf(buf + l, sizeof(buf) - l, "?"); we could add buf[sizeof(buf)-1] = '\0'; Actually 512 bytes for function name should be more than enough but better be on a safe side. Other than that the patch looks ok to me, great job, thanks!