From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) by sourceware.org (Postfix) with ESMTPS id 197D73858D3C for ; Thu, 7 Dec 2023 21:19:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 197D73858D3C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 197D73858D3C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::634 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701983974; cv=none; b=TBcziFxiZe4VJME5acapdhHi7+Pt4f5vMfl+nvieWLVhnEPnjtKdyMvxZD2AlluVh/HBzMTv+z9JyNVpYjPzrsoKopVwQ0XHkVmfZdFD8mnjNJo7bGT42yagZlZhmS8RIwH+RsSicv3+xb6uUIBa2ad2v8Au0U2xua0xkJpw++k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701983974; c=relaxed/simple; bh=QIEoDZBYX7hgdG7+xGRIK//J0RPaAu408CHYDbokE/A=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=DTK+N/qAU8Mc0XFwGbZzFa4T3Ljl3YLn4EwuSCiaY/m/Lm3frBhW/tXR9H6g486vImJ4mCo/n87cwqoDTpXv0sJI6KQT2+mxfARSz3zSpHLhLkogmQc3qy+sdHSNnoTd2O4sYCfiEHNnCaG7qfd6AwpUruRCmQPydgdjRsiKjIA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x634.google.com with SMTP id d9443c01a7336-1d0897e99e0so10887165ad.3 for ; Thu, 07 Dec 2023 13:19:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1701983972; x=1702588772; darn=gcc.gnu.org; h=mime-version:user-agent:message-id:in-reply-to:date:references :organization:subject:cc:to:from:from:to:cc:subject:date:message-id :reply-to; bh=K57NJ6RC8Cg1uC4lQLbQzrBzehCTi4yBkiuJ3U8Zfa8=; b=kcFygImu0a0gpXdqjUf1J87e++YhIe6I7ZKREdqyNAfpgJRWsMjjYf17yBOzCMo98N 9fNnZgTS4qKOqC7LeYkmuD3U9pHdYqn+iBUjsIPfsQJAUX12RCh+Rwn5KznrB8384a/O 3ODgbtV1FPHZp6PcVxcPjgD7y0srIZhT0sSros2JhEVZeeiYsho8nDQW1vA/AEOkgjyD yJ9kouajRp+rzl5TqoZoWnlJxfz7w09hn14sOMMOzaj8NdoYYM0mqWYHpwQcX2Azrf3N WbIZbK4asgCrh9j9dbqaYxHfTY2M8y2yBUxlAb4FPiGRb8PBsaf7u8eAKwjTrN7S3fkk 3bWA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701983972; x=1702588772; h=mime-version:user-agent:message-id:in-reply-to:date:references :organization:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=K57NJ6RC8Cg1uC4lQLbQzrBzehCTi4yBkiuJ3U8Zfa8=; b=V0DXbkRaMFHU6V3g4ChwJQZeZV1P4j3N+qkczpvoY5ft5fVHtjjhJHtmV0vQI90wJ+ jRT/17kjnTvvqiWn1KAVspgyrFUMLSVuUh/Fw+4VQV8bvdFN0UINhDIucOgx0IeAbrs+ BLBdJFR1gq6NuPk1y8w+r1S90qNbxEPRPzwabnXAR5736YIM7D9wYqlOgYoY8eu4xnva VDRbRUwPxUdkCVbRW/5DW94dKofdNJYPfUIFACmcWAfPEtNMKg1GRN1KhHdu+oDkeyt3 zEgxBc0WiQfSVG79fJdIPiO3GDtWG0+yyuan7iKMeSwn8F/f5/OOAl/xVK7601K1LPK7 x8zQ== X-Gm-Message-State: AOJu0Yw3HcNwdi1oGJq+KiTOc0sfwrIPr/ZiCKRBQyYNIF6Znl6iPHnk 1ytdyRFw43xk0uRElLzjbo5J4Q== X-Google-Smtp-Source: AGHT+IGoUwW6mGV/wsPhMi9zn2zCEKFA+Ibr4ZMJv5AIbPShrjBNuSxZrMhHWZylydAVWRXuZLY1pw== X-Received: by 2002:a17:902:ea02:b0:1cc:bf6b:f3b1 with SMTP id s2-20020a170902ea0200b001ccbf6bf3b1mr3375063plg.37.1701983972041; Thu, 07 Dec 2023 13:19:32 -0800 (PST) Received: from free.home ([2804:14c:4d1:44a5:f4d9:b7a4:4fb8:376f]) by smtp.gmail.com with ESMTPSA id p12-20020a170902e74c00b001d0b32ec81esm266524plf.79.2023.12.07.13.19.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 13:19:31 -0800 (PST) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 3B7LJFJs284576 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 7 Dec 2023 18:19:15 -0300 From: Alexandre Oliva To: Jan Hubicka Cc: Richard Biener , gcc-patches@gcc.gnu.org, Jeremy Bennett , Craig Blackmore , Graham Markall , Martin Jambor , Jim Wilson , Jeff Law , Jakub Jelinek Subject: Re: [PATCH v5] Introduce strub: machine-independent stack scrubbing Organization: Free thinker, does not speak for AdaCore References: Date: Thu, 07 Dec 2023 18:19:15 -0300 In-Reply-To: (Jan Hubicka's message of "Wed, 6 Dec 2023 11:22:13 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-5.9 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,T_SCC_BODY_TEXT_LINE,WEIRD_QUOTING 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 Dec 6, 2023, Jan Hubicka wrote: > I am sorry for sending this late. No need to be sorry. Thank you very much for taking the time to review and comment on it. > I think the ipa changes are generally fine. Phew :-) >> +static inline bool >> +strub_always_inline_p (cgraph_node *node) >> +{ >> + return lookup_attribute ("always_inline", DECL_ATTRIBUTES (node->decl)); >> +} > We may want to ahve this as cgraph_node::always_inline_p since there are > now quite many places we look up this attribute. Can do. Would such a global refactoring still be welcome at this stage, or should it be saved for stage1? I guess it could still go in, so simple it is... >> +/* The strub pass proper adjusts types, signatures, and at-calls calls, and >> + splits internal-strub functions. */ >> + >> +unsigned int >> +pass_ipa_strub::execute (function *) >> +{ >> + cgraph_node *onode; >> + >> + ipa_strub_set_mode_for_new_functions (); >> + >> + /* First, adjust the signature of at-calls functions. We adjust types of >> + at-calls functions first, so that we don't modify types in place unless >> + strub is explicitly requested. */ > I think Martin ma have more specific opinion on this, but since this is > not running as the ipa pass during WPA stage, I think the param > modification infrastructure is not really that much hepful here. Hmm... I wonder if this is indeed what Martin refers to. There are two separate pieces of logic for parm-tweaking, one for "at-calls" strub functions, that get the signature and the type of the function itself modified (akin to adding the implicit "this" parameter to a C++ nonstatic member-function), and is implemented under the comment above, and there's the splitting-out of "internal" strub function bodies into a clone with a modified signature, that is implemented elsewhere. The latter uses cloning and thus (some, but not much) IPA param modification infrastructure, but the former doesn't IIRC. >> + /* ??? Maybe we could adjust it instead. */ >> + if (drop_fnspec) >> + remove_named_attribute_unsharing ("fn spec", >> + &TYPE_ATTRIBUTES (nftype)); > ipa param modification also doesn't know how to update fn spec, this is > something we should look into next stage1... > There is also access attribute which speaks directly about individual > arugments, perhaps you want to drop this one too? Hmm, I can't recall whether I've come across it before (it sounds vaguely familiar, but unless they become "fn spec" (ISTR synthetic "fn spec"s), I think I'd have dealt with them already. I'll dig it a little further. > Are variadic thunks working with scrubbing? Yeah, the wrapper is rewritten to call va_start itself, and the split-out wrapped body is modified to call va_copy instead of va_start. -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive