From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 6A46A3858D39 for ; Wed, 6 Dec 2023 10:22:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6A46A3858D39 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=ucw.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kam.mff.cuni.cz ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 6A46A3858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.113.20.16 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701858137; cv=none; b=YfThc12rBCFHUHpsjWWT9XI7wRImMODwzCPfVVQRMbVITLOop5zGKDO8jHFKzWtaiJLtSZzVtXNh3lpW196T/SncoGLfJx7vkevNgj6joAeORLWQs7Dp4keJqVWcoG6Mb5puRcZGFaJVCbBQZdzQDqQXgVdNXtmfqE93PiGOpyQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701858137; c=relaxed/simple; bh=S5idCEDI7oQsO6As8bO5arWADRWYlQz5MM/VRBqBzrY=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=B+FLeQc6iFNXHWcWZmU1gmBfO30bApVNKIBWYxh2xvVQdoYz/ROHRfk4FZtKb/Bg8iTYQS+S4wh4PXfigtdd6D3I8ElkLZ6D7Z+ZcbaMEqb/S/r99emkkkfN4CmzSp4SE5/jVMVmBKRmokT90rWAHVugPlT9A030qmdLdHDYnWw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 5AC9D28BF29; Wed, 6 Dec 2023 11:22:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1; t=1701858133; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=RzxJePgRMCpb46qWbEju6nnzCbRyv4RqMFhwyrYTkpA=; b=p7RLc81J9w+KKw/+OyKWY55+Y+sGGu5F8cVAyxFJQzEzwE2U+b73J6nbEvawQpHKAXCJ/q C7UFSDFN1BTEMEfkVhsRvCfpH866qwxl25EwUUanK4DpsxNg+gtkyb5BIcdz0dL6cpMcsI 20rGm7SgSQq66JDgpT3jIvgaC3TJV7Q= Date: Wed, 6 Dec 2023 11:22:13 +0100 From: Jan Hubicka To: Alexandre Oliva 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 Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,JMQ_SPF_NEUTRAL,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi, I am sorry for sending this late. I think the ipa changes are generally fine. There are few things which was not clear to me. > for gcc/ChangeLog > > * Makefile.in (OBJS): Add ipa-strub.o. > (GTFILES): Add ipa-strub.cc. > * builtins.def (BUILT_IN_STACK_ADDRESS): New. > (BUILT_IN___STRUB_ENTER): New. > (BUILT_IN___STRUB_UPDATE): New. > (BUILT_IN___STRUB_LEAVE): New. > * builtins.cc: Include ipa-strub.h. > (STACK_STOPS, STACK_UNSIGNED): Define. > (expand_builtin_stack_address): New. > (expand_builtin_strub_enter): New. > (expand_builtin_strub_update): New. > (expand_builtin_strub_leave): New. > (expand_builtin): Call them. > * common.opt (fstrub=*): New options. > * doc/extend.texi (strub): New type attribute. > (__builtin_stack_address): New function. > (Stack Scrubbing): New section. > * doc/invoke.texi (-fstrub=*): New options. > (-fdump-ipa-*): New passes. > * gengtype-lex.l: Ignore multi-line pp-directives. > * ipa-inline.cc: Include ipa-strub.h. > (can_inline_edge_p): Test strub_inlinable_to_p. > * ipa-split.cc: Include ipa-strub.h. > (execute_split_functions): Test strub_splittable_p. > * ipa-strub.cc, ipa-strub.h: New. > * passes.def: Add strub_mode and strub passes. > * tree-cfg.cc (gimple_verify_flow_info): Note on debug stmts. > * tree-pass.h (make_pass_ipa_strub_mode): Declare. > (make_pass_ipa_strub): Declare. > (make_pass_ipa_function_and_variable_visibility): Fix > formatting. > * tree-ssa-ccp.cc (optimize_stack_restore): Keep restores > before strub leave. > * multiple_target.cc (pass_target_clone::gate): Test seen_error. > * attribs.cc: Include ipa-strub.h. > (decl_attributes): Support applying attributes to function > type, rather than pointer type, at handler's request. > (comp_type_attributes): Combine strub_comptypes and target > comp_type results. > * doc/tm.texi.in (TARGET_STRUB_USE_DYNAMIC_ARRAY): New. > (TARGET_STRUB_MAY_USE_MEMSET): New. > * doc/tm.texi: Rebuilt. > * cgraph.h (symtab_node::reset): Add preserve_comdat_group > param, with a default. > * cgraphunit.cc (symtab_node::reset): Use it. > > for gcc/c-family/ChangeLog > > * c-attribs.cc: Include ipa-strub.h. > (handle_strub_attribute): New. > (c_common_attribute_table): Add strub. > > for gcc/ada/ChangeLog > > * gcc-interface/trans.cc: Include ipa-strub.h. > (gigi): Make internal decls for targets of compiler-generated > calls strub-callable too. > (build_raise_check): Likewise. > * gcc-interface/utils.cc: Include ipa-strub.h. > (handle_strub_attribute): New. > (gnat_internal_attribute_table): Add strub. > > for gcc/testsuite/ChangeLog > > * c-c++-common/strub-O0.c: New. > * c-c++-common/strub-O1.c: New. > * c-c++-common/strub-O2.c: New. > * c-c++-common/strub-O2fni.c: New. > * c-c++-common/strub-O3.c: New. > * c-c++-common/strub-O3fni.c: New. > * c-c++-common/strub-Og.c: New. > * c-c++-common/strub-Os.c: New. > * c-c++-common/strub-all1.c: New. > * c-c++-common/strub-all2.c: New. > * c-c++-common/strub-apply1.c: New. > * c-c++-common/strub-apply2.c: New. > * c-c++-common/strub-apply3.c: New. > * c-c++-common/strub-apply4.c: New. > * c-c++-common/strub-at-calls1.c: New. > * c-c++-common/strub-at-calls2.c: New. > * c-c++-common/strub-defer-O1.c: New. > * c-c++-common/strub-defer-O2.c: New. > * c-c++-common/strub-defer-O3.c: New. > * c-c++-common/strub-defer-Os.c: New. > * c-c++-common/strub-internal1.c: New. > * c-c++-common/strub-internal2.c: New. > * c-c++-common/strub-parms1.c: New. > * c-c++-common/strub-parms2.c: New. > * c-c++-common/strub-parms3.c: New. > * c-c++-common/strub-relaxed1.c: New. > * c-c++-common/strub-relaxed2.c: New. > * c-c++-common/strub-short-O0-exc.c: New. > * c-c++-common/strub-short-O0.c: New. > * c-c++-common/strub-short-O1.c: New. > * c-c++-common/strub-short-O2.c: New. > * c-c++-common/strub-short-O3.c: New. > * c-c++-common/strub-short-Os.c: New. > * c-c++-common/strub-strict1.c: New. > * c-c++-common/strub-strict2.c: New. > * c-c++-common/strub-tail-O1.c: New. > * c-c++-common/strub-tail-O2.c: New. > * c-c++-common/torture/strub-callable1.c: New. > * c-c++-common/torture/strub-callable2.c: New. > * c-c++-common/torture/strub-const1.c: New. > * c-c++-common/torture/strub-const2.c: New. > * c-c++-common/torture/strub-const3.c: New. > * c-c++-common/torture/strub-const4.c: New. > * c-c++-common/torture/strub-data1.c: New. > * c-c++-common/torture/strub-data2.c: New. > * c-c++-common/torture/strub-data3.c: New. > * c-c++-common/torture/strub-data4.c: New. > * c-c++-common/torture/strub-data5.c: New. > * c-c++-common/torture/strub-indcall1.c: New. > * c-c++-common/torture/strub-indcall2.c: New. > * c-c++-common/torture/strub-indcall3.c: New. > * c-c++-common/torture/strub-inlinable1.c: New. > * c-c++-common/torture/strub-inlinable2.c: New. > * c-c++-common/torture/strub-ptrfn1.c: New. > * c-c++-common/torture/strub-ptrfn2.c: New. > * c-c++-common/torture/strub-ptrfn3.c: New. > * c-c++-common/torture/strub-ptrfn4.c: New. > * c-c++-common/torture/strub-pure1.c: New. > * c-c++-common/torture/strub-pure2.c: New. > * c-c++-common/torture/strub-pure3.c: New. > * c-c++-common/torture/strub-pure4.c: New. > * c-c++-common/torture/strub-run1.c: New. > * c-c++-common/torture/strub-run2.c: New. > * c-c++-common/torture/strub-run3.c: New. > * c-c++-common/torture/strub-run4.c: New. > * c-c++-common/torture/strub-run4c.c: New. > * c-c++-common/torture/strub-run4d.c: New. > * c-c++-common/torture/strub-run4i.c: New. > * g++.dg/strub-run1.C: New. > * g++.dg/torture/strub-init1.C: New. > * g++.dg/torture/strub-init2.C: New. > * g++.dg/torture/strub-init3.C: New. > * gnat.dg/strub_attr.adb, gnat.dg/strub_attr.ads: New. > * gnat.dg/strub_ind.adb, gnat.dg/strub_ind.ads: New. > > for libgcc/ChangeLog > > * Makefile.in (LIB2ADD): Add strub.c. > * libgcc2.h (__strub_enter, __strub_update, __strub_leave): > Declare. > * strub.c: New. > * libgcc-std.ver.in (__strub_enter): Add to GCC_14.0.0. > (__strub_update, __strub_leave): Likewise. > +/* Return TRUE iff NODE carries the always_inline attribute. */ > + > +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. > +/* 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. It may save bit of legwork on adjusting call sites and callees but it does not look that bad. > + /* ??? 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? > + /* Remove the function's body but keep arguments to be reused > + for thunk. */ > + onode->release_body (true); > + onode->reset (/* unlike create_wrapper: preserve_comdat_group = */true); > + > + DECL_UNINLINABLE (decl) = false; > + DECL_RESULT (decl) = decl_result; > + DECL_INITIAL (decl) = NULL; > + allocate_struct_function (decl, false); > + set_cfun (NULL); > + > + /* Turn alias into thunk and expand it into GIMPLE representation. */ > + onode->definition = true; Are variadic thunks working with scrubbing? Honza