Bug in gcc.c for_each_path() (leads to invalid crt0.o selection)?

Bug in gcc.c for_each_path() (leads to invalid crt0.o selection)?

[Sebastian Huber]

Hello,

I noticed that the ARM libgomp is built without TLS support for RTEMS, 
since the thread-local storage detection fails, due to missing symbols 
in the crt0.o. I added the missing symbols to newlib/libc/sys/rtems/crt0.c:

https://sourceware.org/ml/newlib/2016/msg01138.html

However, this still didn't work. The reason is that during GCC build the 
random crt0.o of the prefix is used.

The configure test command line is:

/build/git-build/b-gcc-git-arm-rtems4.12/./gcc/xgcc 
-B/build/git-build/b-gcc-git-arm-rtems4.12/./gcc/ -nostdinc 
-B/build/git-build/b-gcc-git-arm-rtems4.12/arm-rtems4.12/thumb/newlib/ 
-isystem 
/build/git-build/b-gcc-git-arm-rtems4.12/arm-rtems4.12/thumb/newlib/targ-include 
-isystem /home/EB/sebastian_h/archive/gcc-git/newlib/libc/include 
-B/opt/rtems-4.12/arm-rtems4.12/bin/ 
-B/opt/rtems-4.12/arm-rtems4.12/lib/ -isystem 
/opt/rtems-4.12/arm-rtems4.12/include -isystem 
/opt/rtems-4.12/arm-rtems4.12/sys-include  -mthumb -o conftest -g -O2 
conftest.c

GCC searches then in:

Breakpoint 10, file_at_path (path=0x72a960 
"/build/git-build/b-gcc-git-arm-rtems4.12/./gcc/arm-rtems4.12/7.0.0/thumb/", 
data=0x7fffffffc8c0) at /home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$70 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Breakpoint 10, file_at_path (path=0x72a960 
"/build/git-build/b-gcc-git-arm-rtems4.12/./gcc/thumb/", 
data=0x7fffffffc8c0) at /home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$71 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Breakpoint 10, file_at_path (path=0x72a960 
"/build/git-build/b-gcc-git-arm-rtems4.12/arm-rtems4.12/thumb/newlib/arm-rtems4.12/7.0.0/thumb/", 
data=0x7fffffffc8c0) at /home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$72 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Breakpoint 10, file_at_path (path=0x72a960 
"/build/git-build/b-gcc-git-arm-rtems4.12/arm-rtems4.12/thumb/newlib/thumb/", 
data=0x7fffffffc8c0) at /home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$73 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Breakpoint 10, file_at_path (path=0x72a960 
"/opt/rtems-4.12/arm-rtems4.12/bin/arm-rtems4.12/7.0.0/thumb/", 
data=0x7fffffffc8c0) at /home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$74 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Breakpoint 10, file_at_path (path=0x72a960 
"/opt/rtems-4.12/arm-rtems4.12/bin/thumb/", data=0x7fffffffc8c0) at 
/home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$75 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Breakpoint 10, file_at_path (path=0x72a960 
"/opt/rtems-4.12/arm-rtems4.12/lib/arm-rtems4.12/7.0.0/thumb/", 
data=0x7fffffffc8c0) at /home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$76 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Breakpoint 10, file_at_path (path=0x72a960 
"/opt/rtems-4.12/arm-rtems4.12/lib/thumb/", data=0x7fffffffc8c0) at 
/home/EB/sebastian_h/archive/gcc-git/gcc/gcc.c:2733
2733    {
$77 = {name = 0x728d00 "crt0.o", suffix = 0x4d2257 "", name_len = 6, 
suffix_len = 0, mode = 4}

Since all previous attempts to find the file failed, it picks up 
"/opt/rtems-4.12/arm-rtems4.12/lib/thumb/crt0.o".

We have in the build tree:

find -name crt0.o
[...]
./arm-rtems4.12/thumb/newlib/crt0.o
./arm-rtems4.12/thumb/newlib/libc/crt0.o
./arm-rtems4.12/thumb/newlib/libc/sys/crt0.o
./arm-rtems4.12/thumb/newlib/libc/sys/rtems/crt0.o
[...]

In gcc.c we have:

[...]
/* Call CALLBACK for each path in PATHS, breaking out early if CALLBACK
    returns non-NULL.
    If DO_MULTI is true iterate over the paths twice, first with multilib
    suffix then without, otherwise iterate over the paths once without
    adding a multilib suffix.  When DO_MULTI is true, some attempt is made
    to avoid visiting the same path twice, but we could do better.  For
    instance, /usr/lib/../lib is considered different from /usr/lib.
    At least EXTRA_SPACE chars past the end of the path passed to
    CALLBACK are available for use by the callback.
    CALLBACK_INFO allows extra parameters to be passed to CALLBACK.

    Returns the value returned by CALLBACK.  */

static void *
for_each_path (const struct path_prefix *paths,
	       bool do_multi,
	       size_t extra_space,
	       void *(*callback) (char *, void *),
	       void *callback_info)
[...]

We have

-B/build/git-build/b-gcc-git-arm-rtems4.12/arm-rtems4.12/thumb/newlib/
-B/opt/rtems-4.12/arm-rtems4.12/lib/

Its not really clear from the documentation how the search order and 
command line order is related.  The documentation doesn't mention 
multilib and multiarch options.

https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html

If we assume that the command line order determines the search order, 
then its not clear why for_each_path() first iterates for all paths with 
the multilib postfix and then without. Shouldn't it iterate over all 
paths and check with/without multilib postfix in each step?

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Bug in gcc.c for_each_path() (leads to invalid crt0.o selection)?

[Joseph Myers]

On Mon, 5 Dec 2016, Sebastian Huber wrote:

> If we assume that the command line order determines the search order, then its
> not clear why for_each_path() first iterates for all paths with the multilib
> postfix and then without. Shouldn't it iterate over all paths and check
> with/without multilib postfix in each step?

No.  The directory without the multilib postfix is generally the directory 
for another multilib, i.e. always wrong to search, and in case the library 
appears earlier in the non-multilib directories than the multilib ones, 
mixing the search paths (and there may already be some unwanted mixing 
when sysroots are involved) would result in the wrong library being 
chosen.

Unfortunately, getting rid of the search of non-multilib directories 
completely is hard.  It was removed in r96915 
<https://gcc.gnu.org/ml/gcc-patches/2004-11/msg01895.html>.  Then this was 
reverted in <https://gcc.gnu.org/ml/gcc-patches/2005-12/msg00818.html>.  
Furthermore, it turns out there is a case where the existing multilib 
configuration machinery cannot express things correctly without searching 
non-multilib directories.  Specifically, consider the combination of 
MULTILIB_OSDIRNAMES with sysroot suffixes.  This is used by 
config/mips/{t-mti-linux,mti-linux.h}, for example.

MULTILIB_OSDIRNAMES provides directory names used in two ways: relative to 
$target/lib/ in the GCC installation, and relative to lib/ and usr/lib/ in 
a sysroot.  For the latter, we want names such as plain ../lib64, but 
these cannot be used outside the sysroot because different multilibs would 
be mapped to the same directory.  The solution to this issue relies on 
searches without multilib suffixes: you define STARTFILE_PREFIX_SPEC 
(which used to be used more widely before MULTILIB_OSDIRNAMES was added in 
the first place) so sysroot libraries can be found by the combination of 
(sysroot, sysroot suffix, startfile prefix, *no* multilib suffix).

So while this case of MULTILIB_OSDIRNAMES with multiple sysroots makes it 
hard to eliminate the search of non-multilib directories, they should be 
searched as late as possible to reduce the risk of problems from the wrong 
libraries being found.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Bug in gcc.c for_each_path() (leads to invalid crt0.o selection)?

[Sebastian Huber]

Hallo Joseph,

On 05/12/16 23:02, Joseph Myers wrote:
> On Mon, 5 Dec 2016, Sebastian Huber wrote:
>
>> If we assume that the command line order determines the search order, then its
>> not clear why for_each_path() first iterates for all paths with the multilib
>> postfix and then without. Shouldn't it iterate over all paths and check
>> with/without multilib postfix in each step?
> No.  The directory without the multilib postfix is generally the directory
> for another multilib, i.e. always wrong to search, and in case the library
> appears earlier in the non-multilib directories than the multilib ones,
> mixing the search paths (and there may already be some unwanted mixing
> when sysroots are involved) would result in the wrong library being
> chosen.
>
> Unfortunately, getting rid of the search of non-multilib directories
> completely is hard.  It was removed in r96915
> <https://gcc.gnu.org/ml/gcc-patches/2004-11/msg01895.html>.  Then this was
> reverted in <https://gcc.gnu.org/ml/gcc-patches/2005-12/msg00818.html>.
> Furthermore, it turns out there is a case where the existing multilib
> configuration machinery cannot express things correctly without searching
> non-multilib directories.  Specifically, consider the combination of
> MULTILIB_OSDIRNAMES with sysroot suffixes.  This is used by
> config/mips/{t-mti-linux,mti-linux.h}, for example.
>
> MULTILIB_OSDIRNAMES provides directory names used in two ways: relative to
> $target/lib/ in the GCC installation, and relative to lib/ and usr/lib/ in
> a sysroot.  For the latter, we want names such as plain ../lib64, but
> these cannot be used outside the sysroot because different multilibs would
> be mapped to the same directory.  The solution to this issue relies on
> searches without multilib suffixes: you define STARTFILE_PREFIX_SPEC
> (which used to be used more widely before MULTILIB_OSDIRNAMES was added in
> the first place) so sysroot libraries can be found by the combination of
> (sysroot, sysroot suffix, startfile prefix, *no* multilib suffix).
>
> So while this case of MULTILIB_OSDIRNAMES with multiple sysroots makes it
> hard to eliminate the search of non-multilib directories, they should be
> searched as late as possible to reduce the risk of problems from the wrong
> libraries being found.

thanks for the detailed explanation. I guess, then the root cause for my 
problem is that the Newlib provided crt0.o files in the build tree are 
not in the same relative location of the installation tree. In the build 
tree, they reside in a "newlib" subdirectory. I changed the Newlib build 
flags set by GCC to use -L instead of -B and this seems to work (comment 
is wrong now). Does it make sense to submit such a patch or has it maybe 
some nasty side-effects?

diff --git a/configure b/configure
index b6389e4..3c84acb 100755
--- a/configure
+++ b/configure
@@ -7497,7 +7497,7 @@ case " $target_configdirs " in
        # If we're building newlib, use its generic headers last, but search
        # for any libc-related directories first (so make it the last -B
        # switch).
-      FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' 
-B$$r/$(TARGET_SUBDIR)/newlib/ -isystem 
$$r/$(TARGET_SUBDIR)/newlib/targ-include -isystem $$s/newlib/libc/include'
+      FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' 
-L$$r/$(TARGET_SUBDIR)/newlib/ -isystem 
$$r/$(TARGET_SUBDIR)/newlib/targ-include -isystem $$s/newlib/libc/include'

        # If we're building libgloss, find the startup file, simulator 
library
        # and linker script.
diff --git a/configure.ac b/configure.ac
index 51ee705..ca6fb88 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3086,7 +3086,7 @@ case " $target_configdirs " in
        # If we're building newlib, use its generic headers last, but search
        # for any libc-related directories first (so make it the last -B
        # switch).
-      FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' 
-B$$r/$(TARGET_SUBDIR)/newlib/ -isystem 
$$r/$(TARGET_SUBDIR)/newlib/targ-include -isystem $$s/newlib/libc/include'
+      FLAGS_FOR_TARGET=$FLAGS_FOR_TARGET' 
-L$$r/$(TARGET_SUBDIR)/newlib/ -isystem 
$$r/$(TARGET_SUBDIR)/newlib/targ-include -isystem $$s/newlib/libc/include'

        # If we're building libgloss, find the startup file, simulator 
library
        # and linker script.
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index c7b1eaf..9e8dea1 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3809,7 +3809,7 @@ site.exp: ./config.status Makefile
         @if [ -d $(objdir)/../$(target_subdir)/newlib ] \
             && [ "${host}" != "${target}" ]; then \
           echo "set newlib_cflags 
\"-I$(objdir)/../$(target_subdir)/newlib/targ-include 
-I\$$srcdir/../newlib/libc/include\"" >> ./site.tmp; \
-         echo "set newlib_ldflags 
\"-B$(objdir)/../$(target_subdir)/newlib/\"" >> ./site.tmp; \
+         echo "set newlib_ldflags 
\"-L$(objdir)/../$(target_subdir)/newlib/\"" >> ./site.tmp; \
           echo "append CFLAGS \" \$$newlib_cflags\"" >> ./site.tmp; \
           echo "append CXXFLAGS \" \$$newlib_cflags\"" >> ./site.tmp; \
           echo "append LDFLAGS \" \$$newlib_ldflags\"" >> ./site.tmp; \

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: Bug in gcc.c for_each_path() (leads to invalid crt0.o selection)?

[Joseph Myers]

On Tue, 6 Dec 2016, Sebastian Huber wrote:

> thanks for the detailed explanation. I guess, then the root cause for my
> problem is that the Newlib provided crt0.o files in the build tree are not in
> the same relative location of the installation tree. In the build tree, they
> reside in a "newlib" subdirectory. I changed the Newlib build flags set by GCC
> to use -L instead of -B and this seems to work (comment is wrong now). Does it
> make sense to submit such a patch or has it maybe some nasty side-effects?

I don't know what the effects of such a change might be.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Bug in gcc.c for_each_path() (leads to invalid crt0.o selection)?

[Sebastian Huber]

On 06/12/16 18:09, Joseph Myers wrote:
> On Tue, 6 Dec 2016, Sebastian Huber wrote:
>
>> thanks for the detailed explanation. I guess, then the root cause for my
>> problem is that the Newlib provided crt0.o files in the build tree are not in
>> the same relative location of the installation tree. In the build tree, they
>> reside in a "newlib" subdirectory. I changed the Newlib build flags set by GCC
>> to use -L instead of -B and this seems to work (comment is wrong now). Does it
>> make sense to submit such a patch or has it maybe some nasty side-effects?
> I don't know what the effects of such a change might be.
>

Using -L instead of -B had the same problems. My attempt to fix this 
problem is this patch:

https://sourceware.org/ml/newlib/2016/msg01145.html

-- 
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.