From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) by sourceware.org (Postfix) with ESMTPS id CD8B23858D35 for ; Thu, 21 Dec 2023 11:11:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CD8B23858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CD8B23858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.48 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703157076; cv=none; b=xcKaXKv91igZ5kElGcpLFu9BxCS+7IOonqbSzol6LlpRSUOChMfXVUMPquQ6Ikwhi4ViricBEfvCNLFQYSBtG12DvBFjFmKqH7qsfA60v/Q+ag8MqoQoZIVtxIyUFNCX88Exye19iI1WjbS0EC9TSNoQdP6zTNLq0W/fuy/Q0Q0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703157076; c=relaxed/simple; bh=exYFFewVNjAXmNYGR9MQSGMYe+qIMhqUZcY5tymLUNQ=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=s8+HzrLkxM1k0zDv90dFc1LVL2Q0QywgfD97WdxL4Rye7yB0Jtq6HKb387X8BhlK2PLJo9be+9G7otB9cH1EcGHfR+QjdOZFycBQQ6pJojrjHJwdUVU7iHry8r+pTpgh00wrVUwZDiFVBPkYy72qQJ05lQhHYX/o8XV0J2TVw3g= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-40c48d7a7a7so4971285e9.3 for ; Thu, 21 Dec 2023 03:11:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703157073; x=1703761873; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ooHfCTfmROnPhKQgFjzeZc/yn+34E3XEqMoPEDa6GMU=; b=VEdOVX9XjZwkKmPyjmc6XL2el42Q29SngI/JhX+4T8/x0mLXCyg5PfXgCXyvUpOGqi S7O3AL++GziYMo82efWmb2Btm+TcBUGkL7UQVu7Y+eLPPpq3//cQ+xB4kNdFjJdEB0nO O1WkdRqqYIn5plcxTRWFldObWl5rnmzDGbwSE7vGTn9LFl6qpCSYtQpgvXprBYGtjM3m 6L16SbdWvczH6mMCr65U8T+qM7lT1ft/a6jTyBt5/f/K6d97fJUzRMYEBWboGhC2dj4h sjwwdaoV+IiDinofVIbimCdLCXFE4fKerQRN6DQSIcLJZyTlp20kZvolQJ5vZj1NZQ13 K3iA== X-Gm-Message-State: AOJu0Yy5nn0Bfm8QGGqG89lP5R6M2jeoJOB+44AQ8kfXdNJ4Jsw1FTTV uDwNK2XEYy5et96LHmBpvTjhrooAPt8Z6g== X-Google-Smtp-Source: AGHT+IHqjNw2hQl6OU23xx7gf0+5UQDzMIIVWhQPiA9K4VjexMKnDE4yfU2Ro0wWpcjucFy+BUsHoA== X-Received: by 2002:a05:600c:4d97:b0:40d:4153:59b with SMTP id v23-20020a05600c4d9700b0040d4153059bmr150580wmp.101.1703157072975; Thu, 21 Dec 2023 03:11:12 -0800 (PST) Received: from ?IPV6:2001:8a0:f923:4f00:cbd5:901:4306:66a? ([2001:8a0:f923:4f00:cbd5:901:4306:66a]) by smtp.gmail.com with ESMTPSA id z5-20020a05600c0a0500b0040c41846919sm2918410wmp.41.2023.12.21.03.11.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 21 Dec 2023 03:11:12 -0800 (PST) Message-ID: Date: Thu, 21 Dec 2023 11:11:10 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] Make cached_reg_t own its data Content-Language: en-US To: Simon Marchi , gdb-patches@sourceware.org References: <20231215181257.1196830-1-pedro@palves.net> <20231215181257.1196830-2-pedro@palves.net> <969f139c-de27-4563-95be-3064b28a53b8@simark.ca> From: Pedro Alves In-Reply-To: <969f139c-de27-4563-95be-3064b28a53b8@simark.ca> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023-12-20 23:48, Simon Marchi wrote: > > > On 2023-12-15 13:12, Pedro Alves wrote: >> struct cached_reg_t own its data buffer, but currently that is managed >> manually. Convert it to use a unique_xmalloc_ptr. >> >> Change-Id: I05a107098b717299e76de76aaba00d7fbaeac77b > > Hi Pedro, > > When building with clang 17: > > CXX python/py-unwind.o > /home/smarchi/src/binutils-gdb/gdb/python/py-unwind.c:126:16: error: flexible array member 'reg' of type 'cached_reg_t[]' with non-trivial destruction > 126 | cached_reg_t reg[]; > | ^ > Bah. Really Clang? Not even a warning instead of an error? I'm going to apply this to unbreak the build ASAP, but maybe we should really get rid of this flexible array member and C++-fy the whole of struct cached_frame_info. >From bfcfa995f9461726d57f0d9a2879ba4352547870 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 21 Dec 2023 10:43:20 +0000 Subject: [PATCH] Fix Clang build issue with flexible array member and non-trivial dtor Commit d5cebea18e7a ("Make cached_reg_t own its data") added a destructor to cached_reg_t. That caused a build problem with Clang, which errors out like so: > CXX python/py-unwind.o > gdb/python/py-unwind.c:126:16: error: flexible array member 'reg' of type 'cached_reg_t[]' with non-trivial destruction > 126 | cached_reg_t reg[]; > | ^ This is is not really a problem for our code, which allocates the whole structure with xmalloc, and then initializes the array elements with in-place new, and then takes care to call the destructor manually. Like, commit d5cebea18e7a did: @@ -928,7 +927,7 @@ pyuw_dealloc_cache (frame_info *this_frame, void *cache) cached_frame_info *cached_frame = (cached_frame_info *) cache; for (int i = 0; i < cached_frame->reg_count; i++) - xfree (cached_frame->reg[i].data); + cached_frame->reg[i].~cached_reg_t (); Maybe we should get rid of the flexible array member and use a bog standard std::vector. I doubt this would cause any visible performance issue. Meanwhile, to unbreak the build, this commit switches from C99-style flexible array member to 0-length array. It behaves the same, and Clang doesn't complain. I got the idea from here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70932#c11 GCC 9, our oldest support version, already supported this: https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Zero-Length.html but the extension is actually much older than that. Note that C99-style flexible array members are not standard C++ either. Change-Id: I37dda18f367e238a41d610619935b2a0f2acacce --- gdb/python/py-unwind.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c index 31b74c67310..8fed55beadc 100644 --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -123,7 +123,15 @@ struct cached_frame_info /* Length of the `reg' array below. */ int reg_count; - cached_reg_t reg[]; + /* Flexible array member. Note: use a zero-sized array rather than + an actual C99-style flexible array member (unsized array), + because the latter would cause an error with Clang: + + error: flexible array member 'reg' of type 'cached_reg_t[]' with non-trivial destruction + + Note we manually call the destructor of each array element in + pyuw_dealloc_cache. */ + cached_reg_t reg[0]; }; extern PyTypeObject pending_frame_object_type base-commit: 3a4ee6286814e850f66d84b6b8b18cd053649d35 -- 2.43.0