From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122152 invoked by alias); 30 Nov 2018 11:58:46 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 122135 invoked by uid 89); 30 Nov 2018 11:58:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS autolearn=unavailable version=3.3.2 spammy=evil, H*f:sk:87h8fza, real-life, H*f:sk:43e6c9e X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 30 Nov 2018 11:58:44 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5F1C630012E0; Fri, 30 Nov 2018 11:58:42 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5F98B5D736; Fri, 30 Nov 2018 11:58:41 +0000 (UTC) Subject: Re: RFA/RFC: Add stack recursion limit to libiberty's demangler To: Nick Clifton , Tom Tromey References: <87sgzkszbh.fsf@redhat.com> <8bf7952a-5cf1-27b9-e385-e1e12536bf3f@redhat.com> <87h8fza6fh.fsf@tromey.com> <43e6c9e6-8249-bf56-aed8-90d0f771c567@redhat.com> Cc: binutils@sourceware.org, "gcc-patches@gcc.gnu.org" From: Pedro Alves Message-ID: <6e8af218-8f5f-4d75-29a9-dd97a79f46c1@redhat.com> Date: Fri, 30 Nov 2018 11:58:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <43e6c9e6-8249-bf56-aed8-90d0f771c567@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-11/txt/msg02560.txt.bz2 On 11/30/2018 08:26 AM, Nick Clifton wrote: > Hi Pedro, Hi Tom, > >> Pedro> E.g., in GDB, loading big binaries, demangling is very high >> Pedro> in profiles, and so we've kicked around the desire to parallelize >> Pedro> it > > I did consider this, but I encountered two problems: > > 1. I wanted users of the demangling functions to be able to change > the recursion limit. So for example in environments with a very > limited amount of stack space the limit could be reduced. This > is one of the purposes of the cplus_demangle_set_recursion_limit() > function. > > Since a new d_info structure is created every time a string is demangled > the only way to implement cplus_demangle_set_recursion_limit would > be to have it set the value that is used to initialize the field in > the structure, which is basically back to where we are now. That's a lesser evil. With the proposed cplus_demangle_set_recursion_limit interface, you essentially end up with an interface that makes all threads in the process end up with the same limit. There's precedent for global options in the current_demangling_style global, set by the cplus_demangle_set_style function. That's one I recalled, there may be others. I'd prefer interfaces that pass down the options as arguments, or a pointer to an options struct, but that's not the main issue. The main issue is the current recursion level counter. That's the static variable that I had pointed at: d_function_type (struct d_info *di) { [...] + static unsigned long recursion_level = 0; [...] + recursion_level ++; [...] + recursion_level --; return ret; } Even if we go with a recursion limit per process, this recursion level count must be moved to d_info for thread-safety without synchronization. > > 2. I wanted to be able to access the recursion limit from code > in cplus-dem.c, which does not use the d_info structure. That should just mean a bit of plumbing is in order to pass down either a d_info structure or something like it? > This was > the other purpose of the cplus_demangler_set_recursion_limit() > function - it allows the limit to be read without changing it. > > As an alternative I did consider adding an extra parameter to the > cplus_demangle(), cplus_demangle_opname() and related functions. But > this would make them non-backwards compatible and I did not want to > do that. I'd say that ideally, we'd aim at the interface we want, and then go from there. If changing interfaces is a problem, we can always add a new entry point and keep the old as backward compatibility. But is it (a problem)? Do we require ABI compatibility here? But again, the main issue I'm seeing is the recursion level counter, not the global limit. > I would imagine that changing the recursion limit would be a very > rare event, possibly only ever done once in an executable's runtime, > so I doubt that there will ever be any real-life thread safety > problems associated with it. But I do understand that this is just > an assumption, not a guarantee. See above. Thanks, Pedro Alves