Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested functions #26

Closed
fxcoudert opened this issue Sep 9, 2020 · 13 comments
Closed

Nested functions #26

fxcoudert opened this issue Sep 9, 2020 · 13 comments

Comments

@fxcoudert
Copy link
Contributor

This may be the cause for some crashes in procedure pointers in Fortran. In any case, here is a C crasher:

$ cat a.c
void foo(void) {
  void bar(void) { ; }
  void (*pptr)(void);
  pptr = bar;
  pptr();
}

int main (void) {
  foo();
}
$ ./bin/gcc a.c -O1 && ./a.out
$ ./bin/gcc a.c -O0 && ./a.out
zsh: segmentation fault  ./a.out
@fxcoudert
Copy link
Contributor Author

This is the cause for the only libgomp failure: libgomp.c/pr52547.c

@iains
Copy link
Owner

iains commented Sep 9, 2020

could you try your case with "-fno-trampolines" ?
I'm rebuilding everything at the moment.

@iains
Copy link
Owner

iains commented Sep 9, 2020

ah, never mind, it seems that this might only be implemented for Ada.

iains pushed a commit that referenced this issue Nov 7, 2020
Enable thumb1_gen_const_int to generate RTL or asm depending on the
context, so that we avoid duplicating code to handle constants in
Thumb-1 with -mpure-code.

Use a template so that the algorithm is effectively shared, and
rely on two classes to handle the actual emission as RTL or asm.

The generated sequence is improved to handle right-shiftable and small
values with less instructions. We now generate:

128:
        movs    r0, r0, #128
264:
        movs    r3, #33
        lsls    r3, #3
510:
        movs    r3, #255
        lsls    r3, #1
512:
        movs    r3, #1
        lsls    r3, #9
764:
        movs    r3, #191
        lsls    r3, #2
65536:
        movs    r3, #1
        lsls    r3, #16
0x123456:
        movs    r3, #18 ;0x12
        lsls    r3, #8
        adds    r3, #52 ;0x34
        lsls    r3, #8
        adds    r3, #86 ;0x56
0x1123456:
        movs    r3, #137 ;0x89
        lsls    r3, #8
        adds    r3, #26 ;0x1a
        lsls    r3, #8
        adds    r3, #43 ;0x2b
        lsls    r3, #1
0x1000010:
        movs    r3, #16
        lsls    r3, #16
        adds    r3, #1
        lsls    r3, #4
0x1000011:
        movs    r3, #1
        lsls    r3, #24
        adds    r3, #17
-8192:
	movs	r3, #1
	lsls	r3, #13
	rsbs	r3, #0

The patch adds a testcase which does not fully exercise
thumb1_gen_const_int, as other existing patterns already catch small
constants.  These parts of thumb1_gen_const_int are used by
arm_thumb1_mi_thunk.

2020-11-02  Christophe Lyon  <christophe.lyon@linaro.org>

	gcc/
	* config/arm/arm.c (thumb1_const_rtl, thumb1_const_print): New
	classes.
	(thumb1_gen_const_int): Rename to ...
	(thumb1_gen_const_int_1): ... New helper function. Add capability
	to emit either RTL or asm, improve generated code.
	(thumb1_gen_const_int_rtl): New function.
	* config/arm/arm-protos.h (thumb1_gen_const_int): Rename to
	thumb1_gen_const_int_rtl.
	* config/arm/thumb1.md: Call thumb1_gen_const_int_rtl instead
	of thumb1_gen_const_int.

	gcc/testsuite/
	* gcc.target/arm/pure-code/no-literal-pool-m0.c: New.
@apinski-cavium
Copy link
Contributor

Most likely the following needs to be added (when initializing the trampoline):
#ifdef HAVE_ENABLE_EXECUTE_STACK
emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__enable_execute_stack"),
LCT_NORMAL, VOIDmode, XEXP (m_tramp, 0), Pmode);
#endif

Like what is done on i386, etc.
Once I get a build going, I will test a patch for that.

@apinski-cavium
Copy link
Contributor

fixtrampolines.diff.txt
This is the start of the patch, there might need some other stuff like libgcc changes done too.

@iains
Copy link
Owner

iains commented May 5, 2021

executable stack is prohibited by the kernel - I've hacked in a temporary solution using the "ada" method, but it uses bit 1 of the address, which is non-ABI-compliant.

@iains
Copy link
Owner

iains commented May 5, 2021

current hacks:
8408ba8
3fdef6d
0246f34

Future plans:

While security policy does not allow executable stack, it does allow for dynamic code generation. This would mean that we can have allocated trampolines (either from a pool or some other way) but it also means that the usage needs to be tracked, since freeing the is no longer "automatic" on scope exit.

So one of my colleagues from embecosm is looking into the following skeleton design (we plan to publish a "GCC12 project" to do this, since it should be useful to other platforms that would prefer to avoid executable stack for security reasons) -- not ready yet tho :

  • the front end(s) need to be ready to emit markers on nested structures that reflect the target’s choice of lowering

  • the middle end (gcc/nested.c) needs to react to those

  • in particular the new piece of work is tracking nested function scopes so that a cleanup may be added (it might be relatively simple to implement, e.g. using a gimple with_cleanup operation)
  • inventing the builtin to control that cleanup
  • I think that there are compiler builtins to deal with trampoline and descriptor cases, but this might need a hybrid (since we have a descriptor for a trampoline that is not on the stack - so the descriptor doesn’t completely replace the trampoline like the Adacode solution … although they do still use the trampoline builtin stuff in some way)
  • the back end(s) need to deal with the allocation / generation / deallocation of the trampolines at the appropriate pass (possibly expand?)

@iains
Copy link
Owner

iains commented Dec 25, 2021

c0f7a8f

includes the proposed (and posted) replacement implementation which uses a heap-allocated trampoline.

@crazypill
Copy link

crazypill commented Dec 28, 2021 via email

@fxcoudert
Copy link
Contributor Author

There are no known failures with nested functions in the new implementation. Closing this.

@Nicholaswogan
Copy link

Nicholaswogan commented Jan 7, 2022

@fxcoudert when do you expect hombrew binaries to be updated with an Apple Silicon gfortran that works with nested functions? Also thanks a million for this fix. You all are very under appreciated!

@fxcoudert
Copy link
Contributor Author

I was aiming at the next GCC 11 minor release. Or waiting for GCC 12. Since the new scheme is incompatible with the previous (partial) implementation, and even though we always labeled support in GCC 11 as experimental, I'm a bit worried about breaking code for current users.

@Nicholaswogan
Copy link

@fxcoudert I hear GCC 12 will come out in April 2022. Will updated Apple Silicon GCC binaries be available then as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants