Message ID | 20210105185820.3796657-1-adhemerval.zanella@linaro.org |
---|---|
Headers |
Return-Path: <libc-alpha-bounces@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 90667393BC34; Tue, 5 Jan 2021 18:58:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 90667393BC34 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1609873111; bh=qBwfT61Ck2CSE40N1pXmE1sAX8vTlxIVFOw7OwRdEec=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=t9/O9mCmli5iqWzvi0ww9mpE8F/uQlw9NaM6EYKLvlE3x+iy8i2o9S45A/H28MnCu VyY8t58VnTauSZjFHisJ8XwAPAhBT9IVfE+WMkoWj4GqA/uN5KNDJxIggk7TJj7wYB WbmKHiJmQV/AZNgw1KgOP07TpegGeBGw8ByaHz6c= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qk1-x730.google.com (mail-qk1-x730.google.com [IPv6:2607:f8b0:4864:20::730]) by sourceware.org (Postfix) with ESMTPS id CDE7C393BC39 for <libc-alpha@sourceware.org>; Tue, 5 Jan 2021 18:58:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CDE7C393BC39 Received: by mail-qk1-x730.google.com with SMTP id 19so287124qkm.8 for <libc-alpha@sourceware.org>; Tue, 05 Jan 2021 10:58:26 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=qBwfT61Ck2CSE40N1pXmE1sAX8vTlxIVFOw7OwRdEec=; b=QjvH7+jqOrv4h2EBAN8YcOgcyL27qT6xsMut8aX8joRNax036+Z3qqdW90eKKk2u3E dWGXKx0GxZgobyOoK2X4T3z+03txMbRNWTDH64Wx45ITVqPISjkgoGDywBBRLIYRLfXC cZAiKysp/y1TylzDFzrZgIsguYUrQNtahZlgWjKzSkFmrJJsqbVJRSOreapkMbIN0Tvu xyTiR7dV8on9MoY4ephA8rl/I+kNF3XplIbqu/VmLMjUbQveuZGqKP45llI9oJ0/gUp1 4jt1xrLo/NdrLOXCeHPu3UNC4Dhspgs3K0/lA92Ob+fS4bfBgb3IbfQlDhloM4xssfeF OIOg== X-Gm-Message-State: AOAM532794syrF2334dBjjITBFuxXlkCfvarCrtlgrIhjD7etjCJg9D8 Vnv32pklG4Ai1snJqCc08KCtviexS7ufCA== X-Google-Smtp-Source: ABdhPJzIc8b/P1MTUMg3RVpjJjf6zrrIjCU2HN4wJ7XvRGTLqcds6ylfgBDkOWQo2DnBuhG84Ty8dQ== X-Received: by 2002:a05:620a:22ee:: with SMTP id p14mr886611qki.343.1609873105894; Tue, 05 Jan 2021 10:58:25 -0800 (PST) Received: from localhost.localdomain ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id t68sm67792qkd.35.2021.01.05.10.58.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Jan 2021 10:58:25 -0800 (PST) To: libc-alpha@sourceware.org, Paul Eggert <eggert@cs.ucla.edu>, bug-gnulib@gnu.org Subject: [PATCH 0/8] Remove alloca usage from glob Date: Tue, 5 Jan 2021 15:58:12 -0300 Message-Id: <20210105185820.3796657-1-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Adhemerval Zanella <adhemerval.zanella@linaro.org> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
Remove alloca usage from glob
|
|
Message
Adhemerval Zanella Netto
Jan. 5, 2021, 6:58 p.m. UTC
This is a respin on an old patchset [1] which is no longer on patchwork. The main change is just a rebase against master since glibc version is now on sync with gnulib. I am sending it also to gnulib to get feedback from the project, specifically for the part not specific for the system support by glibc and extra requirements. The idea of removing the alloca allows a slight better code generation, simplifies the boilerplate code to avoid the unbounded alloca usage, and it plays better with security compiler mitigation tools (such as the ones for stack clash). On x86_64 with gcc 9.2.1 build glob for shared object, the total stack usage went from: glob.c:261:1:next_brace_sub 8 static glob.c:1205:1:prefix_array 96 static glob.c:1185:1:collated_compare 8 static glob.c:250:1:is_dir.isra.0 160 static glob.c:1259:1:glob_in_dir 864 dynamic glob.c:297:1:__glob 1360 dynamic to glob.c:231:1:next_brace_sub 8 static dynarray-skeleton.c:203:1:globnames_array_free 32 static glob.c:1131:1:prefix_array 96 static glob.c:1111:1:collated_compare 8 static dynarray-skeleton.c:275:1:globnames_array_add__ 48 static dynarray-skeleton.c:373:1:char_array_resize.part.0 48 static char_array-skeleton.c:274:1:char_array_replace_str_pos 48 static char_array-skeleton.c:204:1:char_array_crop 32 static char_array-skeleton.c:240:1:char_array_append_str 48 static char_array-skeleton.c:118:1:char_array_set_str_size 32 static char_array-skeleton.c:161:1:char_array_init_str 32 static glob.c:220:1:is_dir.isra.0 160 static glob.c:1200:1:glob_in_dir 1088 static glob.c:267:1:__glob 1824 static (it was obtained with -fstack-usage gcc option). It is a slight increase and due GNU extension where glob might be called recursivelly internally the total stack usage might be still slight larger than current implementation. But I still think it is a worth tradeoff. [1] https://sourceware.org/pipermail/libc-alpha/2018-February/091326.html Adhemerval Zanella (8): malloc: Add specialized dynarray for C strings posix: Use char_array for internal glob dirname posix: Remove alloca usage for GLOB_BRACE on glob posix: Remove alloca usage on glob dirname posix: Use dynarray for globname in glob posix: Remove alloca usage on glob user_name posix: Use char_array for home_dir in glob posix: Remove all alloca usage in glob malloc/Makefile | 4 +- malloc/Versions | 7 + malloc/char_array-impl.c | 57 +++ malloc/char_array-skeleton.c | 288 +++++++++++++ malloc/char_array.h | 53 +++ malloc/dynarray.h | 9 + malloc/dynarray_overflow_failure.c | 31 ++ malloc/malloc-internal.h | 14 + malloc/tst-char_array.c | 112 ++++++ posix/glob.c | 622 +++++++++++------------------ 10 files changed, 817 insertions(+), 380 deletions(-) create mode 100644 malloc/char_array-impl.c create mode 100644 malloc/char_array-skeleton.c create mode 100644 malloc/char_array.h create mode 100644 malloc/dynarray_overflow_failure.c create mode 100644 malloc/tst-char_array.c
Comments
On 1/5/21 10:58 AM, Adhemerval Zanella wrote: > The idea of removing the alloca allows a slight better code generation, > simplifies the boilerplate code to avoid the unbounded alloca usage, > and it plays better with security compiler mitigation tools (such as the > ones for stack clash). Instead of complicating dynarray by adding char_array stuff, I suggest adding any primitives just to glob for now, as it's not clear that they're generally useful. I do see a problem with the proposed patch set, in that it creates several different dynarrays when glob really needs only one or two. The existing code is full of memory-allocation gotchas and would be simplified if it treated the memory it allocates as a first-class part of the problem rather than as some sort of cranky subsidiary that needs to be babied and its diaper changed at random intervals. Attached is a draft set of patches against Gnulib commit 6a00fdb4bb105697aa27ba97ef7ec33287790ad3 which gives a hint about the sort of thing that I mean here. This patch set is not complete (it does only the equivalent of the first four of the patches you proposed) and it's not exactly the form that I want, so I haven't installed it into Gnulib. However, I hope it shows the sort of thing I have in mind. So far, it's needed only one scratch buffer. In this patch set, glob.c continues to use scratch_buffer.h because scratch buffers are good enough for all the changes needed so far. I suppose dynarrays will be helpful for later patches and that we can switch to them as needed, but I wanted to focus on the actual problem first rather than worrying about scratch buffers vs dynarrays.