[PATCH] [MSP430] Make the inclusion of run_*_array functions depend on defined assembler symbols

[PATCH] [MSP430] Make the inclusion of run_*_array functions depend on defined assembler symbols

[Jozef Lawrynowicz]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

Many of the MSP430 crt functions (e.g. to initialize bss) are linked
"dynamically", based on symbols defined in the program.
The GNU assembler defines the symbols corresponding to the crt
functions by examining the section names in the input file.

If GCC has been configured with --enable-initfini-array, then
.init_array and .fini_array will hold pointers to global
constructors/destructors. These sections can also hold functions that
need to be executed for other purposes.

The attached patch puts the __crt0_run_{preinit,init,fini}_array and
__crt0_run_array functions in their own object files, so they will
only be linked into the final executable when needed.

A companion patch for the GNU assembler has been submitted here:
https://sourceware.org/ml/binutils/2019-04/msg00211.html

Successfully regtested the DejaGNU GCC testsuite using the binutils and newlib
changes together with GCC trunk configured with --enable-initfini-array.

If the patch is acceptable, I would appreciate if someone would commit it for
me, as I do not have write access.

Thanks,
Jozef

[-- Attachment #2: 0001-MSP430-Make-the-inclusion-of-run_-_array-fns-depend-.patch --]
[-- Type: text/x-patch, Size: 5973 bytes --]

From 380e0a51a85fa75f2886acc325cfa51e47a2ca7c Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Wed, 17 Apr 2019 13:17:38 +0100
Subject: [PATCH] MSP430: Make the inclusion of run_*_array fns depend on
 defined assembler symbols

Many of the MSP430 crt functions (e.g. to initialize bss) are linked
"dynamically", based on symbols defined in the program.
The GNU assembler defines the symbols corresponding to the crt
functions by examining the section names in the input file.

If GCC has been configured with --enable-initfini-array, then
.init_array and .fini_array will hold pointers to global
constructors/destructors. These sections can also hold functions that
need to be executed for other purposes.

The attached patch puts the __crt0_run_{preinit,init,fini}_array and
__crt0_run_array functions in their own object files, so they will
only be linked when needed.

Successfully regtested the DejaGNU GCC testsuite using the binutils and
newlib changes together with GCC trunk configured with
--enable-initfini-array.
---
 libgloss/msp430/Makefile.in |  4 ++
 libgloss/msp430/crt0.S      | 79 +++++++++++++++++++++++++++----------
 libgloss/msp430/crtn.S      | 13 +++++-
 3 files changed, 74 insertions(+), 22 deletions(-)

diff --git a/libgloss/msp430/Makefile.in b/libgloss/msp430/Makefile.in
index 73079d65b..77c9b8b21 100644
--- a/libgloss/msp430/Makefile.in
+++ b/libgloss/msp430/Makefile.in
@@ -87,6 +87,10 @@ CRT_OBJS = \
 	crt_main.o \
 	crt_main_minrt.o \
 	crt_callexit.o \
+	crt_run_init_array.o \
+	crt_run_preinit_array.o \
+	crt_run_fini_array.o \
+	crt_run_array.o \
 	crt_init.o
 
 #### Host specific Makefile fragment comes in here.
diff --git a/libgloss/msp430/crt0.S b/libgloss/msp430/crt0.S
index 53162e6bd..42464ddbe 100644
--- a/libgloss/msp430/crt0.S
+++ b/libgloss/msp430/crt0.S
@@ -62,6 +62,13 @@ START_CRT_FUNC 0000 start
 END_CRT_FUNC	start
 #endif
 
+;; Some of the CRT functions below will only be present in the final linked
+;; executable if the assembler decides they are needed.  It will only define
+;; the symbol necessary to prevent them being garbage collected by the linker
+;; if the file being assembled has a specific section.
+;; The CRT functions this applies to are:
+;; init_bss, movedata, move_highdata, init_highbss, run_init_array,
+;; run_preinit_array, run_fini_array and run_array.
 
 #if Lbss
 ;; Note - this section is only included in the startup code of the
@@ -215,44 +222,69 @@ END_CRT_FUNC	call_exit
 ;----------------------------------------
 
 #ifndef MINRT
-#if L0
-	.section ".crt_0900main_init", "ax", @progbits
-	.global	_msp430_run_init_array
-	.type	_msp430_run_init_array,@function
-_msp430_run_init_array:
-	mov_	#__init_array_start, R4
-	mov_	#__init_array_end, R5
-	mov_	#PTRsz, R6
-	br_	#_msp430_run_array
 
-	.global	_msp430_run_preinit_array
-	.type	_msp430_run_preinit_array,@function
-_msp430_run_preinit_array:
+#if Lrun_preinit_array
+;; Note - this section is only included in the startup code of the application
+;; if it is needed.  It is responsible for setting up the arguments
+;; required for __crt0_run_array, to run the functions in .preinit_array.
+START_CRT_FUNC 0910 run_preinit_array
+
 	mov_	#__preinit_array_start, R4
 	mov_	#__preinit_array_end, R5
 	mov_	#PTRsz, R6
-	br_	#_msp430_run_array
+	br_	#__crt0_run_array
+
+END_CRT_FUNC	run_preinit_array
+#endif /* Lrun_preinit_array */
+
+#if Lrun_init_array
+;; Note - this section is only included in the startup code of the application
+;; if it is needed.  It is responsible for setting up the arguments
+;; required for __crt0_run_array, to run the functions in .init_array.
+START_CRT_FUNC 0920 run_init_array
+
+	mov_	#__init_array_start, R4
+	mov_	#__init_array_end, R5
+	mov_	#PTRsz, R6
+	br_	#__crt0_run_array
+
+END_CRT_FUNC	run_init_array
+#endif /* Lrun_init_array */
+
+#if Lrun_fini_array
+;; Note - this section is only included in the startup code of the application
+;; if it is needed.  It is responsible for setting up the arguments
+;; required for __crt0_run_array, to run the functions in .fini_array.
+START_CRT_FUNC 0930 run_fini_array
 
-	.global	_msp430_run_fini_array
-	.type	_msp430_run_fini_array,@function
-_msp430_run_fini_array:
 	mov_	#__fini_array_start, R4
 	mov_	#__fini_array_end, R5
 	mov_	#-PTRsz, R6
-	br_	#_msp430_run_array
+	br_	#__crt0_run_array
+
+END_CRT_FUNC	run_fini_array
+#endif /* Lrun_fini_array */
+
+#if Lrun_array
+;; Note - this section is only included in the startup code of the application
+;; if it is needed by one of the above run_*_array functions.
+START_CRT_FUNC 0980 run_array
 
-_msp430_run_array:
 	cmp_	R4, R5
 	jeq	_msp430_run_done
 	mov_	@R4, R7
 	add_	R6, R4
 	call_	R7
-	br_	#_msp430_run_array
+	br_	#__crt0_run_array
+
+END_CRT_FUNC	run_array
 
 _msp430_run_done:
 	ret_
+#endif /* Lrun_array */
 
 ;----------------------------------------
+#if L0
 
 	.section	.init,"ax"
 
@@ -263,7 +295,14 @@ __msp430_init:
 
 	.global __msp430_fini
 __msp430_fini:
-	call_	#_msp430_run_fini_array
+	call_	#__crt0_run_fini_array
 	
+;; If this function is not defined externally, we don't need it to do
+;; anything.
+	.text
+	.weak __crt0_run_fini_array
+__crt0_run_fini_array:
+	ret_
+
 #endif
 #endif /* not MINRT */
diff --git a/libgloss/msp430/crtn.S b/libgloss/msp430/crtn.S
index 939d5ce6f..110fc3090 100644
--- a/libgloss/msp430/crtn.S
+++ b/libgloss/msp430/crtn.S
@@ -15,8 +15,8 @@
 
 #ifndef MINRT
 	.section	.init,"ax"
-	call_	#_msp430_run_preinit_array
-	call_	#_msp430_run_init_array
+	call_	#__crt0_run_preinit_array
+	call_	#__crt0_run_init_array
 	ret_
 	.global	__msp430_init_end
 __msp430_init_end:
@@ -28,5 +28,14 @@ __msp430_init_end:
 __msp430_fini_end:
 
 	.text
+;; If these functions are not defined externally, we don't need them to do
+;; anything.
+	.balign 2
+	.weak __crt0_run_preinit_array
+	.weak __crt0_run_init_array
+__crt0_run_preinit_array:
+__crt0_run_init_array:
+	ret_
+
 
 #endif
-- 
2.17.1

Re: [PATCH] [MSP430] Make the inclusion of run_*_array functions depend on defined assembler symbols

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]

On Apr 18 01:04, Jozef Lawrynowicz wrote:
> Many of the MSP430 crt functions (e.g. to initialize bss) are linked
> "dynamically", based on symbols defined in the program.
> The GNU assembler defines the symbols corresponding to the crt
> functions by examining the section names in the input file.
> 
> If GCC has been configured with --enable-initfini-array, then
> .init_array and .fini_array will hold pointers to global
> constructors/destructors. These sections can also hold functions that
> need to be executed for other purposes.
> 
> The attached patch puts the __crt0_run_{preinit,init,fini}_array and
> __crt0_run_array functions in their own object files, so they will
> only be linked into the final executable when needed.
> 
> A companion patch for the GNU assembler has been submitted here:
> https://sourceware.org/ml/binutils/2019-04/msg00211.html
> 
> Successfully regtested the DejaGNU GCC testsuite using the binutils and newlib
> changes together with GCC trunk configured with --enable-initfini-array.
> 
> If the patch is acceptable, I would appreciate if someone would commit it for
> me, as I do not have write access.

Pushed.

Btw., you don't have to prepend text to your patch if it's reproducing
the commit message anyway.  Just send the patch with `git send-email'
to the list, patchsets ideally git format-patch'ed with --cover-letter.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] [MSP430] Make the inclusion of run_*_array functions depend on defined assembler symbols

[Jozef Lawrynowicz]

On Thu, 18 Apr 2019 10:33:56 +0200
Corinna Vinschen <vinschen@redhat.com> wrote:

> On Apr 18 01:04, Jozef Lawrynowicz wrote:
> > If the patch is acceptable, I would appreciate if someone would commit it for
> > me, as I do not have write access.  
> 
> Pushed.
> 
> Btw., you don't have to prepend text to your patch if it's reproducing
> the commit message anyway.  Just send the patch with `git send-email'
> to the list, patchsets ideally git format-patch'ed with --cover-letter.
> 

Ok thanks, will do in the future.

Jozef