Re: [PATCH v3] Introduce strub: machine-independent stack scrubbing

[Qing Zhao <qing.zhao@oracle.com>]

Hi, Alexandre,

Thanks a lot for the work. I think that this will be a valuable feature to be added for GCC’s security functionality. 

I have several questions on this patch:


1.  The implementation of register scrubbing,  -fzero-call-used-regs,  is to insert the register zeroing sequence in the routine’s epilogue,
So each routine will be responsible to clean its own call-clobbered registers before returning. 
     This is simple and straightforward, no change to the function’s interface.

     I am wondering why stack scrubbing, proposed in this patch series, cannot do the stack scrubbing in the routine’s epilogue similar as
register scrubbing?

     There are the following benefits from doing the stack scrubbing in the callee’s epilogue:
      A.  The size of the stack need to be cleaned is known by itself, no need to pass this information to other routines,
            Therefore  functions' interface change can be avoided; no need to change the caller’s body, no need for cloning the callee, etc.
      B.   As a result, the runtime overhead of stack scrubbing should be reduced.
      C.  If we do the stack scrubbing in a very late stage and in the routine’s epilogue, similar as register scrubbing, we don’t need 
            to deal with the complicated call-chain staff anymore, right?


   So, what’s the fundamental issues that stack scrubbing cannot be done by the routine itself but its caller?

2.  I have concerns on the runtime performance overhead, do you have any data on this for your current implementation?

3. You mentioned that there are several “modes” for this feature, could you please provide more details on the modes and their description?

thanks.

Qing



> On Jun 16, 2023, at 2:09 AM, Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> 
> This patch adds the strub attribute for function and variable types,
> command-line options, passes and adjustments to implement it,
> documentation, and tests.
> 
> Stack scrubbing is implemented in a machine-independent way: functions
> with strub enabled are modified so that they take an extra stack
> watermark argument, that they update with their stack use, and the
> caller can then zero it out once it regains control, whether by return
> or exception.  There are two ways to go about it: at-calls, that
> modifies the visible interface (signature) of the function, and
> internal, in which the body is moved to a clone, the clone undergoes
> the interface change, and the function becomes a wrapper, preserving
> its original interface, that calls the clone and then clears the stack
> used by it.
> 
> Variables can also be annotated with the strub attribute, so that
> functions that read from them get stack scrubbing enabled implicitly,
> whether at-calls, for functions only usable within a translation unit,
> or internal, for functions whose interfaces must not be modified.
> 
> There is a strict mode, in which functions that have their stack
> scrubbed can only call other functions with stack-scrubbing
> interfaces, or those explicitly marked as callable from strub
> contexts, so that an entire call chain gets scrubbing, at once or
> piecemeal depending on optimization levels.  In the default mode,
> relaxed, this requirement is not enforced by the compiler.
> 
> The implementation adds two IPA passes, one that assigns strub modes
> early on, another that modifies interfaces and adds calls to the
> builtins that jointly implement stack scrubbing.  Another builtin,
> that obtains the stack pointer, is added for use in the implementation
> of the builtins, whether expanded inline or called in libgcc.
> 
> There are new command-line options to change operation modes and to
> force the feature disabled; it is enabled by default, but it has no
> effect and is implicitly disabled if the strub attribute is never
> used.  There are also options meant to use for testing the feature,
> enabling different strubbing modes for all (viable) functions.
> 
> Regstrapped on x86_64-linux-gnu.  Also tested with gcc-13, and with
> various other targets.  Ok to install?
> 
> There have been only minor changes since v2:
> 
> - scrub the stack in the same direction it grows, inline and out-of-line
> 
> - remove need for stack space in __strub_leave
> 
> - add (ultimately not needed) means to avoid using the red zone in
>  __strub_leave
> 
> - introduce and document TARGET_ macros to tune __strub_leave
> 
> - drop a misoptimization in inlined __strub_enter
> 
> - fix handling of cgraph edges without call stmts
> 
> - adjust some testcases (async stack uses; Ada compiler bug fix)
> 
> - drop bits for compatibility with gcc 10
> 
> - preserve the comdat group when resetting a function into a strub
>  wrapper, coping with a symtab_node::reset change in gcc-13
> 
> 
> for  gcc/ChangeLog
> 
> 	* Makefile.in (OBJS): Add ipa-strub.o.
> 	* 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.
> 	* 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.