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
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)
