diff mbox

[testsuite] Fix multiple definitions of _init

Message ID 1417986266.19509.41.camel@yam-132-YW-E178-FTW
State New
Headers show

Commit Message

Oleg Endo Dec. 7, 2014, 9:04 p.m. UTC 
On Tue, 2014-12-02 at 10:23 -0800, Mike Stump wrote:
> No.  It is reasonable for the test suite to fail when the
> implementation of gcc is wrong (unclean) or newlib startup code is
> wrong (unclean).  Since that is what happened, the fix is to fix the
> cleanliness problem.
> 
> I’ve read through the thread…  I think the sh newlib port has a trivial
> bug and should be fixed.  Either, add an underscore or remove one, just
> like _all_ the other ports.  There is no complexity here, just a typo
> (or someone copied the wrong port).
> 
> When newlib/libc/sys/sh/crt0.S is fixed, and libgcc/config/sh/crt1.S is
> fixed, then these test cases will pass.
> 
> USER_LABEL_PREFIX  is not the issue.  Let me quote from the code:
> 
>         .long   _atexit         .long   _init
> 
> This is wrong.
> 
> Also, _fini needs to be fixed as well.  It is unfortunate that the fix
> involves two packages, but, such is life.

On Tue, 2014-12-02 at 21:55 +0000, Joseph Myers wrote:
> See e.g. config/arm/lib1funcs.S:
> 
> #define SYM(x) CONCAT1 (__USER_LABEL_PREFIX__, x)
> 
> (and the associated macro definition of CONCAT1 that uses, etc.).  If
> you have .S sources that may be used on targets both with and without
> a prefix, you should do something similar.  (The ELF gABI says
> "External C symbols have the same names in C and object files' symbol
> tables.", so ELF targets using leading underscores on C symbol names
> are going against the gABI.)

Thanks for the comments.  I've tried the attached patch on my newlib
config and it seems to fix the problem.  Newlib doesn't seem to be in
need for patching for non-SH64/SH5 in this case.

Kaz, could you please check if the patch doesn't break anything on
sh4-linux?  If so, I'd like to commit this to trunk.

I'm a bit puzzled though.  In libgcc/arm/crti.S there are also _init and
_fini function names.  It seems that an arm-eabi configuration doesn't
add the _ prefix.  But if it did, that'd be the same problem as on SH I
guess?

I still think there should be a configure option to control/override the
target defined default value of USER_LABEL_PREFIX.  But that's another
story.

Cheers,
Oleg

Comments

Kaz Kojima Dec. 8, 2014, 5:51 a.m. UTC | #1 
Oleg Endo <oleg.endo@t-online.de> wrote:
> Kaz, could you please check if the patch doesn't break anything on
> sh4-linux?  If so, I'd like to commit this to trunk.

Build and test ok on sh4-unknown-linux-gnu.

Regards,
	kaz
Oleg Endo Dec. 17, 2014, 2:03 a.m. UTC | #2 
On Mon, 2014-12-08 at 14:51 +0900, Kaz Kojima wrote:
> Oleg Endo <oleg.endo@t-online.de> wrote:
> > Kaz, could you please check if the patch doesn't break anything on
> > sh4-linux?  If so, I'd like to commit this to trunk.
> 
> Build and test ok on sh4-unknown-linux-gnu.
> 

Now also tested here with 
make -k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}"

Committed as r218807.

Cheers,
Oleg
diff mbox

Patch

Index: libgcc/config/sh/crti.S
===================================================================
--- libgcc/config/sh/crti.S	(revision 218464)
+++ libgcc/config/sh/crti.S	(working copy)
@@ -22,6 +22,7 @@ 
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 <http://www.gnu.org/licenses/>.  */
 
+#include "crt.h"
 
 /* The code in sections .init and .fini is supposed to be a single
    regular function.  The function in .init is called directly from
@@ -44,8 +45,8 @@ 
 #else
 	.p2align 1
 #endif
-	.global	 _init
-_init:
+	.global	 GLOBAL(_init)
+GLOBAL(_init):
 #if __SHMEDIA__
 	addi	r15, -16, r15
 	st.q	r15, 8, r14
@@ -89,8 +90,8 @@ 
 #else
 	.p2align 1
 #endif
-	.global  _fini
-_fini:	
+	.global  GLOBAL(_fini)
+GLOBAL(_fini):	
 #if __SHMEDIA__
 	addi	r15, -16, r15
 	st.q	r15, 8, r14
Index: libgcc/config/sh/crt1.S
===================================================================
--- libgcc/config/sh/crt1.S	(revision 218464)
+++ libgcc/config/sh/crt1.S	(working copy)
@@ -22,6 +22,7 @@ 
 see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 <http://www.gnu.org/licenses/>.  */
 
+#include "crt.h"
 
 #ifdef MMU_SUPPORT
 	/* Section used for exception/timer interrupt stack area */
@@ -420,7 +421,7 @@ 
 #endif /* MMU_SUPPORT */
 
 	pt/l	.Lzero_bss_loop, tr0
-	pt/l	_init, tr5
+	pt/l	GLOBAL(_init), tr5
 	pt/l	___setup_argv_and_call_main, tr6
 	pt/l	_exit, tr7
 
@@ -452,7 +453,7 @@ 
 
 	! arrange for exit to call fini
 	pt/l	_atexit, tr1
-	LOAD_ADDR (_fini, r2)
+	LOAD_ADDR (GLOBAL(_fini), r2)
 	blink	tr1, r18
 
 	! call init
@@ -850,9 +851,9 @@ 
 atexit_k:
 	.long	_atexit
 init_k:
-	.long	_init
+	.long	GLOBAL(_init)
 fini_k:
-	.long	_fini
+	.long	GLOBAL(_fini)
 #ifdef VBR_SETUP
 old_vbr_k:
 	.long	old_vbr
@@ -1116,9 +1117,7 @@ 
 #if defined(__SH_FPU_ANY__)
 	.balign 4
 pervading_precision_k:
-#define CONCAT1(A,B) A##B
-#define CONCAT(A,B) CONCAT1(A,B)
-	.long CONCAT(__USER_LABEL_PREFIX__,__fpscr_values)+4
+	.long GLOBAL(__fpscr_values)+4
 #endif
 #else
 	mov.l 2f, r0     ! Load the old vbr setting (if any).
Index: libgcc/config/sh/crt.h
===================================================================
--- libgcc/config/sh/crt.h	(revision 0)
+++ libgcc/config/sh/crt.h	(revision 0)
@@ -0,0 +1,29 @@ 
+/* Copyright (C) 2014 Free Software Foundation, Inc.
+
+This file is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by the
+Free Software Foundation; either version 3, or (at your option) any
+later version.
+
+This file is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef __USER_LABEL_PREFIX__
+#error  __USER_LABEL_PREFIX__ not defined
+#endif
+
+#define CONCAT(a, b) CONCAT2(a, b)
+#define CONCAT2(a, b) a ## b
+
+#define GLOBAL(X) CONCAT(__USER_LABEL_PREFIX__,X)