Message ID | cover.1677533215.git.aburgess@redhat.com |
---|---|
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AE889385843A for <patchwork@sourceware.org>; Mon, 27 Feb 2023 21:29:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AE889385843A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677533399; bh=UPbTysIDk0NqcYS35qQim4asj79ZLuBG0TbTsuqL9aE=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=G9mTSYluqBc6s4XEqjvqRWtuCj4b337yymcxpdDeA4IkPuFvC5ENkoSAVvMTTCWub wxF/ednYSxInrG/Z4ETrIxjO2MmWSEzdCkr17Q+mVea8RAbVMjAiIjPg/SwgeNZC+k lk5+q/PiNgp3+mxtYxjESWIxUQSZLLiOlPt7Iy7o= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id BBAD13858D32 for <gdb-patches@sourceware.org>; Mon, 27 Feb 2023 21:29:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BBAD13858D32 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-113-eF3580oOMHWsySjyMrJrHg-1; Mon, 27 Feb 2023 16:29:31 -0500 X-MC-Unique: eF3580oOMHWsySjyMrJrHg-1 Received: by mail-wm1-f71.google.com with SMTP id c7-20020a7bc847000000b003e00be23a70so5928518wml.2 for <gdb-patches@sourceware.org>; Mon, 27 Feb 2023 13:29:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677533369; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=UPbTysIDk0NqcYS35qQim4asj79ZLuBG0TbTsuqL9aE=; b=uv0AhT5yybDqpADVK3zbTaBQdrzI4E81GDaghIhTidiC9tMeNeFTwDXxQIch9x9nou JTRY9Nh6bzkP/0PLgyKIskZMSL6QK2w/QB5beLJoy42LIpJJGYimmssX9IHDeas+2vIG WMPNWcIdxLKMccj3vIsGtQLSjsIp2Oof806WRVd954sHIKPydfPvCXaPjZjBjKPDcONU Ln0StWjYOvR82hUUfyyetZVPtGL0vtlwneYwssWTos9jA7Q3HT0gWrb8j0e5cLmE2zk2 v4sgr92GqvFu5VclsVpz2TIOERBvTXM3YfXfgVo8F9bJfRxPq7Z8NJ+PMeZcwAbBTPrw uZlQ== X-Gm-Message-State: AO0yUKWWX7Ku/Se1TesbMCIbK4kTns5DaAgejJNuTGCGsGS2Adz6Bnf/ boQsqJYH7MZhm5QOLdQkYW6IBM9CtLmRNiriD1+oVbaFZ+vsQpPs0639gQa11epNzYVWStYW0HN fUZNa3z3S1KETPdxqEKd3NivblfHy9L1MCP4o87Tz1LpXNrlaFgtLOSfHo2xrde5OZtfsrWKvZn 8EfYg= X-Received: by 2002:a05:600c:310b:b0:3eb:323e:de79 with SMTP id g11-20020a05600c310b00b003eb323ede79mr420837wmo.6.1677533369718; Mon, 27 Feb 2023 13:29:29 -0800 (PST) X-Google-Smtp-Source: AK7set/HoxImrV11rvmRK+CGndPJiXYodukQWfFqGW/RjV60WfbP7B4IKs5x+4rd1TDPP5XkL+68Fg== X-Received: by 2002:a05:600c:310b:b0:3eb:323e:de79 with SMTP id g11-20020a05600c310b00b003eb323ede79mr420825wmo.6.1677533369411; Mon, 27 Feb 2023 13:29:29 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id p15-20020a05600c1d8f00b003e20970175dsm14312489wms.32.2023.02.27.13.29.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Feb 2023 13:29:29 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess <aburgess@redhat.com> Subject: [PATCH 00/13] Remove a bunch of alloca uses Date: Mon, 27 Feb 2023 21:29:13 +0000 Message-Id: <cover.1677533215.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Andrew Burgess <aburgess@redhat.com> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
Remove a bunch of alloca uses
|
|
Message
Andrew Burgess
Feb. 27, 2023, 9:29 p.m. UTC
Continuing effort to replace alloca calls with C++ data structures, here's a bunch of places where gdb::byte_vector can be used. This isn't all of them, there's still plenty to work through, but I thought I'd see how this lot is received. Thanks, Andrew --- Andrew Burgess (13): gdb: remove uses of alloca from arch-utils.c gdb: remove use of alloca from auxv.c gdb: remove use of alloca from c-lang.c gdb: remove use of alloca from corefile.c gdb: remove uses of alloca from dwarf2/expr.c gdb: remove a use of alloca from elfread.c gdb: remove use of alloca from findvar.c gdb: remove use of alloca from linux-nat-trad.c gdb: remove use of alloca from mem-break.c gdb: remove some uses of alloca from printcmd.c gdb: remove some uses of alloca from remote.c gdb: remove uses of alloca from valprint.c gdb: remove a use of alloca from symfile.c gdb/arch-utils.c | 13 ++++++------- gdb/auxv.c | 10 +++++----- gdb/c-lang.c | 8 +++----- gdb/corefile.c | 18 +++++++++--------- gdb/dwarf2/expr.c | 25 ++++++++++++++----------- gdb/elfread.c | 7 ++++--- gdb/findvar.c | 6 +++--- gdb/linux-nat-trad.c | 8 ++++---- gdb/mem-break.c | 7 +++---- gdb/printcmd.c | 44 +++++++++++++++++++++----------------------- gdb/remote.c | 40 ++++++++++++++++++---------------------- gdb/symfile.c | 11 +++++------ gdb/valprint.c | 17 ++++++++--------- 13 files changed, 103 insertions(+), 111 deletions(-) base-commit: 85c7cb3c4b70cc484ecf3d72a116503876a28f0a
Comments
On 2/27/23 16:29, Andrew Burgess via Gdb-patches wrote: > Continuing effort to replace alloca calls with C++ data structures, > here's a bunch of places where gdb::byte_vector can be used. > > This isn't all of them, there's still plenty to work through, but I > thought I'd see how this lot is received. Every time I spot an alloca in the wild, I'm also tempted to switch it to vector or something like that. But then I think, it's an extra dynamic allocation for no real gain, so it would just make things worse. When we know the maximum size a buffer can have, instead of using alloca, it would better to statically allocate that size on the stack, and then user an array_view to define the effectively used portion of it. For the cases where we don't (I'm thinking of buffer the size of registers, often small, but there isn't a statically known maximum length), perhaps an std::vector-like structure with small size optimization (like std::string has) would be nice. So you could define a buffer that would use stack memory up to let's say 128 bytes, and heap memory above that. Some references: https://stoyannk.wordpress.com/2017/11/18/small-vector-optimization/ https://llvm.org/doxygen/classllvm_1_1SmallVector.html https://github.com/facebook/folly/blob/main/folly/docs/small_vector.md https://www.boost.org/doc/libs/1_60_0/doc/html/boost/container/small_vector.html With something like this, I think we could change pretty much all allocas without feeling bad about it. If the license allows, I'd be all for just importing an existing implementation in our code base, to avoid reinventing the wheel. Simon
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> Every time I spot an alloca in the wild, I'm also tempted to switch it
Simon> to vector or something like that. But then I think, it's an extra
Simon> dynamic allocation for no real gain, so it would just make things worse.
I'd imagine these allocations probably don't matter in most cases, but
on the other hand, alloca is a hazard, as it's easy to accidentally blow
up the stack.
Simon> When we know the maximum size a buffer can have, instead of using
Simon> alloca, it would better to statically allocate that size on the stack,
Simon> and then user an array_view to define the effectively used portion of
Simon> it.
True, and for registers in particular we could just have a #define
somewhere probably. At least, assuming they have some maximum size
across all supported arches.
Simon> If the license allows, I'd be all for just importing an existing
Simon> implementation in our code base, to avoid reinventing the wheel.
This would also be fine with me but I tend to think in most cases it's
probably unnecessary. Like, I wouldn't blink if new code came in using
a local vector<> but I might mention a new alloca in a review.
Tom