Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unified Headers: LONG_BIT Undeclared in legacy_signal_inlines.h When APP_API < 21 #352

Closed
epicstar opened this issue Mar 30, 2017 · 16 comments
Assignees
Milestone

Comments

@epicstar
Copy link

epicstar commented Mar 30, 2017

Description

When compiling one of my company's products, when enabling Unified Headers and APP_API < 21, I get the following error:

Android/Sdk/ndk-bundle/sysroot/usr/include/android/legacy_signal_inlines.h:52:33: error: use of undeclared identifier 'LONG_BIT'

This seems to be happening when trying to compile one of my product's files, which depends on Boost's atomic library:

In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/atomic.hpp:12:
In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/atomic/atomic.hpp:19:
In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/atomic/capabilities.hpp:17:
In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/atomic/detail/config.hpp:18:
In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/config.hpp:57:
In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/config/platform/linux.hpp:74:
In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/config/posix_features.hpp:18:
In file included from ${HOME_FOLDER}/Android/Sdk/ndk-bundle/sysroot/usr/include/unistd.h:35:
In file included from ${HOME_FOLDER}/Android/Sdk/ndk-bundle/sysroot/usr/include/sys/select.h:36:
In file included from ${HOME_FOLDER}/Android/Sdk/ndk-bundle/sysroot/usr/include/signal.h:188:
${HOME_FOLDER}/Android/Sdk/ndk-bundle/sysroot/usr/include/android/legacy_signal_inlines.h:52:33: error: use of undeclared identifier 'LONG_BIT'
  return (int)((local_set[bit / LONG_BIT] >> (bit % LONG_BIT)) & 1);
                                ^
${HOME_FOLDER}Android/Sdk/ndk-bundle/sysroot/usr/include/android/legacy_signal_inlines.h:52:53: error: use of undeclared identifier 'LONG_BIT'
  return (int)((local_set[bit / LONG_BIT] >> (bit % LONG_BIT)) & 1);
                                                    ^
${HOME_FOLDER}Android/Sdk/ndk-bundle/sysroot/usr/include/android/legacy_signal_inlines.h:63:19: error: use of undeclared identifier 'LONG_BIT'
  local_set[bit / LONG_BIT] |= 1UL << (bit % LONG_BIT);
                  ^
${HOME_FOLDER}Android/Sdk/ndk-bundle/sysroot/usr/include/android/legacy_signal_inlines.h:63:46: error: use of undeclared identifier 'LONG_BIT'
  local_set[bit / LONG_BIT] |= 1UL << (bit % LONG_BIT);
                                             ^
${HOME_FOLDER}/Android/Sdk/ndk-bundle/sysroot/usr/include/android/legacy_signal_inlines.h:75:19: error: use of undeclared identifier 'LONG_BIT'
  local_set[bit / LONG_BIT] &= ~(1UL << (bit % LONG_BIT));
                  ^
${HOME_FOLDER}Android/Sdk/ndk-bundle/sysroot/usr/include/android/legacy_signal_inlines.h:75:48: error: use of undeclared identifier 'LONG_BIT'
  local_set[bit / LONG_BIT] &= ~(1UL << (bit % LONG_BIT));

Investigating this slightly further, I found a couple things....

  • If I set my APP_API level to 21, I no longer get this problem (obviously x86_64 and arm64-v8a will never get this problem anyway). This is probably because it never goes into the legacy_signal_inlines.h
  • I get the same issue without the unified headers disabled, at around the same equivalent header files:
In file included from ${HOME_FOLDER}/${LIB_FOLDER}/boost/include/boost/config/posix_features.hpp:18:
In file included from ${HOME_FOLDER}/Android/Sdk/ndk-bundle/platforms/android-19/arch-arm/usr/include/unistd.h:34:
In file included from ${HOME_FOLDER}/Android/Sdk/ndk-bundle/platforms/android-19/arch-arm/usr/include/sys/select.h:34:
${HOME_FOLDER}/Android/Sdk/ndk-bundle/platforms/android-19/arch-arm/usr/include/signal.h:67:36: error: use of undeclared identifier 'LONG_BIT'
    return (int)((local_set[signum/LONG_BIT] >> (signum%LONG_BIT)) & 1);
  • LONG_BIT is actually defined in the following header file: ${HOME_FOLDER}/Android/Sdk/ndk-bundle/sysroot/usr/include/limits.h
  • However, legacy_signals_inlines.h doesn't have limits.h included:
#include <errno.h>
#include <signal.h>
#include <string.h>
#include <sys/cdefs.h>

My opinions:

  • This is definitely not a regression at all since the old headers had the same exact issue. However, I would've expected unified headers to essentially fix problems like what I'm seeing now
  • Seems like the old platform headers were loading header files either in the wrong order or this this header file was never actually fully tested.

To me, this is obviously a bug in both the old headers and in the new unified headers code. How long will it take to fix this? I really hope there's a patch version of r14 to fix small issues like this.

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

  • NDK Version: 14.1.3816874
  • Build sytem: ndk-build
  • Host OS: Linux (Ubuntu 16.04)
  • Compiler: Clang
  • ABI: armeabi-v7a
  • STL: c++_shared
  • NDK API level: 19
  • Device API level: N/A
@DanAlbert DanAlbert self-assigned this Mar 30, 2017
@DanAlbert DanAlbert added this to the r15 milestone Mar 30, 2017
@enh
Copy link
Contributor

enh commented Mar 30, 2017

there's something weird going on here though. signal.h:38 includes limits.h, long before it includes android/legacy_signal_inlines.h on L173...

@epicstar
Copy link
Author

epicstar commented Mar 31, 2017

Seems like the limits.h file in ${ndk-root}/sysroot/usr/include isn't being loaded. I can confirm this by adding an #error "test" on the first lines of limits.h and I never hit that error. In addition, I put another #error "before including limits.h" inside signal.h, I do end up hitting this line. I do know that the bundled clang compiler is including a limits.h file; however, I have absolutely no idea where this limits.h is. I'll do some more investigation.

@epicstar
Copy link
Author

After even more investigation, it seems that a different limits.h file is indeed being loaded.... Turns out it was this flag causing this error: -ffreestanding

http://stackoverflow.com/questions/17692428/what-is-ffreestanding-option-in-gcc

The clang toolchain has its own limits.h file, and that is used instead of the one in the unified headers when the -ffreestanding flag is added. This worked fine in the 4.9 toolchain but not in clang.

Attached is a test project that shows this exact behavior: test-project_ffreestanding.zip

I'm no longer sure if this is a bug in the NDK anymore if it's just something that should be documented when people start converting from gcc to clang.

@DanAlbert
Copy link
Member

aiui -ffreestanding is something that shouldn't be set for building an app.

@alexcohn
Copy link

Speaking of <signal.h> for APP_API < 21, we were forced to introduce

#define __libc_current_sigrtmin() __SIGRTMIN

Are we missing some trick here?

@enh
Copy link
Contributor

enh commented Apr 24, 2017

no, that one's our fault. note that ((__SIGRTMIN) + 4) is less unsafe, since that's the current value. (the problem here being that the platform you run on might take more of these signals for itself than the platform you built against did.)

@alexcohn
Copy link

Should we expect the fixed android/legacy_signal_inlines.h to include (for APP_API < 21) something like

#define __libc_current_sigrtmin() (dlsym(NULL, "__libc_current_sigrtmin") ? ((int(*)(void))dlsym(NULL, "__libc_current_sigrtmin")() : (__SIGRTMIN)+4)

@alexcohn
Copy link

alexcohn commented Apr 24, 2017

Wait a second, with the old-style header platforms/android-14/arch-arm/usr/include/asm/signal.h we have

#define SIGRTMIN 32

Do you imply that we should use 36 to run safely on Lollipop and higher?

@enh
Copy link
Contributor

enh commented Apr 24, 2017

correct. we're not trying to move you over to unified headers just to keep you busy --- the old headers are wildly out of date.

your suggestion above sounds like our least worst choice for compatibility, so we'll get that into r15beta2.

@enh enh reopened this Apr 25, 2017
@enh
Copy link
Contributor

enh commented Apr 25, 2017

https://android-review.googlesource.com/380365 fixes this in bionic.

@enh
Copy link
Contributor

enh commented Apr 26, 2017

okay, that's submitted now and cherrypicked onto the right branch for r15beta2. if you have time, feel free to manually hack your r15beta1 in case there's another problem lurking behind that one :-)

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Apr 26, 2017
Bug: android/ndk#352
Test: built new NDK test
Change-Id: Iacebe574bbf693701949e038005a40ba6520d592
@alexcohn
Copy link

alexcohn commented Apr 27, 2017

@enh Yes, it builds clean now, but I must confess that we don't have test case for this piece, so I cannot confirm that the fix works on all supported devices. Actually, we have not yet seen complaints from API >= 21, where hardcoded 32 may clash with libbacktrace or libcore, so it may take time until Crashlytics will assure us that all is well here.

@enh
Copy link
Contributor

enh commented Apr 27, 2017

given that only one of Crashlytics and debuggerd(libbacktrace) are likely to be in play at once, it's probably unlikely that they'd have trouble even if they did use the same signal. libcore actually only uses its signal for the side-effect of EINTR to unblock some network operations, so the worst you'd see there is slightly degraded performance (but if your other user is Crashlytics, well, you're probably crashing anyway...).

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
Bug: android/ndk#352
Test: ran tests (with and without a hacked sysroot containing the fix)
Change-Id: Iedac09f406112ea1e37cc0560a0863820242786f
@JakeSmarter
Copy link

I am getting the same error.

In file included from ndk-release-r17/platform/sysroot/usr/include/signal.h:37,
                 from ndk-release-r17/platform/sysroot/usr/include/sys/select.h:36,
                 from ndk-release-r17/platform/sysroot/usr/include/sys/time.h:37,
                 from ndk-release-r17/platform/sysroot/usr/include/time.h:33,
                 from ndk-release-r17/platform/sysroot/usr/include/pthread.h:37,
ndk-release-r17/platform/sysroot/usr/include/bits/signal_types.h:65:53: error: ‘LONG_BIT’ undeclared here (not in a function); did you mean ‘LONG_MIN’?
   65 | typedef struct { unsigned long __bits[_KERNEL__NSIG/LONG_BIT]; } sigset64_t;

And, I am not sure how to work around it. Because we are using GCC, we are also rather stuck on NDK r17. Btw, we are not using the -ffreestanding option and yet it looks like limits.h is not processed.

@enh-google
Copy link
Collaborator

Because we are using GCC, we are also rather stuck on NDK r17.

why are you stuck on gcc? you can build the linux kernel with clang these days, and they're pretty demanding users of weird compiler extensions... might be worth filing a separate bug for that if there's some gap that might be generally useful.

@JakeSmarter
Copy link

why are you stuck on gcc?

We are not stuck on GCC, it is a policy and legacy decision. We are stuck on NDK r17.

I have been able to work around this issue by using the -nostdinc option, so that select NDK supplied headers take precedence over the compiler supplied ones.

sepehrst pushed a commit to spsforks/android-bionic-libc that referenced this issue Apr 22, 2024
Bug: android/ndk#352
Test: built new NDK test
Change-Id: Iacebe574bbf693701949e038005a40ba6520d592
sepehrst pushed a commit to spsforks/android-bionic-libc that referenced this issue Apr 22, 2024
Bug: android/ndk#352
Test: built new NDK test
Change-Id: Iacebe574bbf693701949e038005a40ba6520d592
sepehrst pushed a commit to spsforks/android-bionic-libc that referenced this issue Apr 22, 2024
Bug: android/ndk#352
Test: built new NDK test
Change-Id: Iacebe574bbf693701949e038005a40ba6520d592
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants