Bug 103166 - [12 regression] wrong dependency on getentropy on newlib-based targets
Summary: [12 regression] wrong dependency on getentropy on newlib-based targets
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 12.0
: P1 normal
Target Milestone: 12.0
Assignee: Hans-Peter Nilsson
URL: https://gcc.gnu.org/pipermail/gcc-pat...
Keywords: link-failure
Depends on:
Blocks:
 
Reported: 2021-11-10 09:05 UTC by Christophe Lyon
Modified: 2023-03-07 11:20 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 11.2.1
Known to fail: 12.0
Last reconfirmed: 2021-11-10 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christophe Lyon 2021-11-10 09:05:32 UTC
Since r12-5066, I've noticed numerous link failures on aarch64-none-elf like:

FAIL: g++.dg/concepts/expression.C  -std=gnu++17 (test for excess errors)
Excess errors:
/libstdc++-v3/src/c++11/random.cc:179: undefined reference to `getentropy'
/libstdc++-v3/src/c++11/random.cc:179:(.text._ZNSt12_GLOBAL__N_117__libc_getentropyEPv+0x10): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `getentropy'
/libstdc++-v3/src/c++11/random.cc:452: undefined reference to `getentropy'
/libstdc++-v3/src/c++11/random.cc:452:(.text._ZNSt13random_device7_M_initERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE+0x68): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `getentropy'
/aci-gcc-fsf/sources/newlib/newlib/libc/stdlib/arc4random.c:89: undefined reference to `getentropy'
/aci-gcc-fsf/sources/newlib/newlib/libc/stdlib/arc4random.c:89:(.text+0x558): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `getentropy'

Maybe there's something wrong with the detection of HAVE_GETENTROPY in configure?
Comment 1 Andrew Pinski 2021-11-10 09:33:06 UTC
Note the revision is r12-5056.

So there are two issues here both dealing with getentropy; one is a newlib issue and the other is a libstdc++ issue (well maybe still a newlib one).

Newlib issue:
Newlib includes a version of arc4random which requires getentropy.

libstdc++/Newlib issue:
libstdc++ detects getentropy in the header and thinks it can work.  newlib defines it unconditionally too.

Maybe the better fix is to fix newlib?
Comment 2 Jonathan Wakely 2021-11-10 09:43:47 UTC
(In reply to Christophe Lyon from comment #0)
> Maybe there's something wrong with the detection of HAVE_GETENTROPY in
> configure?

We only do a compile test, not link, so if newlib declares it in <unistd.h> but doesn't define it, we detect it incorrectly. But we avoid link tests in configure, because there are problems for cross-compilers.
Comment 3 sandra 2021-11-11 02:35:39 UTC
I'm seeing these failures in my nios2-elf test results today too.  :-(
Comment 4 Hans-Peter Nilsson 2021-11-12 17:15:22 UTC
I have a patch using GCC_TRY_COMPILE_OR_LINK instead of AC_TRY_COMPILE for those function, since I foolishly went about to fix this before looking in bugzilla. :)

Will post once it's successfully past the first point of failure, sparing others the churn of finding autoconf-2.69 and automake-2.15.1.  Unless a patch is already posted?
Comment 5 Jonathan Wakely 2021-11-12 19:02:57 UTC
There's no other patch as far as I know, so that would be great, thanks.
Comment 6 Hans-Peter Nilsson 2021-11-12 20:25:45 UTC
Yoink!
Comment 7 GCC Commits 2021-11-13 00:46:42 UTC
The master branch has been updated by Hans-Peter Nilsson <hp@gcc.gnu.org>:

https://gcc.gnu.org/g:60f761c7e54f96a287c73a71d0b09ee2b2f8426d

commit r12-5220-g60f761c7e54f96a287c73a71d0b09ee2b2f8426d
Author: Hans-Peter Nilsson <hp@axis.com>
Date:   Fri Nov 12 18:04:43 2021 +0100

    libstdc++: Use GCC_TRY_COMPILE_OR_LINK for getentropy, arc4random
    
    Since r12-5056-g3439657b0286, there has been a regression in
    test results; an additional 100 FAILs running the g++ and
    libstdc++ testsuite on cris-elf, a newlib target.  The
    failures are linker errors, not finding a definition for
    getentropy.  It appears newlib has since 2017-12-03
    declarations of getentropy and arc4random, and provides an
    implementation of arc4random using getentropy, but provides no
    definition of getentropy, not even a stub yielding ENOSYS.
    This is similar to what it does for many other functions too.
    
    While fixing newlib (like adding said stub) would likely help,
    it still leaves older newlib releases hanging.  Thankfully,
    the libstdc++ configury test can be improved to try linking
    where possible; using the bespoke GCC_TRY_COMPILE_OR_LINK
    instead of AC_TRY_COMPILE.  BTW, I see a lack of consistency;
    some tests use AC_TRY_COMPILE and some GCC_TRY_COMPILE_OR_LINK
    for no apparent reason, but this commit just amends
    r12-5056-g3439657b0286.
    
    libstdc++-v3:
            PR libstdc++/103166
            * acinclude.m4 (GLIBCXX_CHECK_GETENTROPY, GLIBCXX_CHECK_ARC4RANDOM):
            Use GCC_TRY_COMPILE_OR_LINK instead of AC_TRY_COMPILE.
            * configure: Regenerate.
Comment 8 Hans-Peter Nilsson 2021-11-13 00:50:14 UTC
Calling it fixed, as observed for cris-elf applying the same patch at r12-5056-g3439657b0286.
Comment 9 Hans-Peter Nilsson 2021-11-13 00:58:45 UTC
Elaboration: there may be (newlib) targets (build/test setups) where the test-setup can't link and thus erroneously gets a "yes" for these functions.  If that happens for you, look into fixing newlib (like, providing a stub that returns -1 and sets errno=ENOSYS, which would work for libstdc++ as it has a fallback option - actually several fallback options).

(Or, make newlib stop declaring functions it doesn't completely provide.)

That was my plan B, thankfully GCC_TRY_COMPILE_OR_LINK did the right thing for my setup. :)
Comment 10 Christophe Lyon 2023-03-07 10:24:20 UTC
(In reply to Jonathan Wakely from comment #2)
> (In reply to Christophe Lyon from comment #0)
> > Maybe there's something wrong with the detection of HAVE_GETENTROPY in
> > configure?
> 
> We only do a compile test, not link, so if newlib declares it in <unistd.h>
> but doesn't define it, we detect it incorrectly. But we avoid link tests in
> configure, because there are problems for cross-compilers.

Why do we avoid link tests? Is that because of something like https://lists.gnu.org/archive/html/autoconf/2007-03/msg00085.html ?

Also I am not sure to understand how this patch fixed the problem?

Before and after this patch we are happy if compilation succeeds, right?

What does the "OR_LINK" part gives us?
Comment 11 Jonathan Wakely 2023-03-07 11:20:56 UTC
(In reply to Christophe Lyon from comment #10)
> Why do we avoid link tests? Is that because of something like
> https://lists.gnu.org/archive/html/autoconf/2007-03/msg00085.html ?

No, I don't think it's related to that, but every time somebody explains it to me I instantly forget the answer.

Here's what Joseph explained last time I asked:

On some bare-metal targets, the compiler can't link unless you explicitly pass -T or similar options to select a particular BSP.
For any hosted system supporting shared libraries, at least, the cross configuration ought to be the same as the native configuration (running the same configure tests), and hardcoding in crossconfig.m4 tends to be a source of bugs.