8000 Fix cross-compiling for android with the NDK by turistu · Pull Request #252 · strace/strace · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix cross-compiling for android with the NDK #252

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix cross-compiling for android with the NDK #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f256395
fix build with CC=ANDROID-NDK/toolchains/llvm/prebuilt/linux-x86_64/b…
turistu Apr 15, 2023
7ec3cc6
fix build with CC=ANDROID-NDK/toolchains/llvm/prebuilt/linux-x86_64/b…
turistu Apr 15, 2023
fe1b220
fix build with CC=ANDROID-NDK/toolchains/llvm/prebuilt/linux-x86_64/b…
turistu Apr 15, 2023
22c6890
kill mx32 personality support on android x86_64
turistu Apr 15, 2023
013f966
The _Nonnull att 8000 ribute from android's headers[1] in newer ndks (r27)
turistu Aug 8, 2024
cc343ea
apparently they made "nullptr" a keyword in C23
turistu Jan 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
< 8000 input type="text" id="file-tree-filter-field" class="form-control input-block pl-5 js-filterable-field" placeholder="Filter changed files" aria-label="Filter changed files" autocomplete="off" data-target="diff-layout.fileTreePathFilter" data-hydro-click-payload={"event_type":"pull_request.user_action","payload":{"category":"file_filter","data":{"file_count":4},"pull_request_id":1342691932,"user_id":null,"action":"filter_by_pathname","originating_url":"https://github.com/strace/strace/pull/252/files"}} data-hydro-click-hmac=e8deb97bc99683d6053297c2227468623bfc648a265c74a828e7e07807d96919 >
44 changes: 26 additions & 18 deletions configure.ac
View file
Open in desktop
< 8000 td class="blob-code blob-code-deletion js-file-line"> #
Original file line number Diff line number Diff line change
Expand Up @@ -770,23 +770,28 @@ Valid options are: yes, no, check, m32, mx32.])
[enable_mpers=yes])

AS_IF([test x$arch = xaarch64],
[# So far, only aarch64 needs a separate compiler for its compat
# personality (which is AArch32, that is more or less ARMv7 EABI)
# Some heuristics regarding possible compiler name:
# Debian: arm-linux-gnueabi{,hf}-gcc
# SuSE: arm7{,hl}-linux-gnueabi-gcc
# Red Hat: arm-redhat-linux-gnu-gcc
# Fedora: arm-linux-gnu-gcc
# ALT: armh-alt-linux-gnueabi-gcc
m4_foreach([triplet1], [arm, arm7, arm7hl, armh], dnl
[m4_foreach([triplet2], [, $host_vendor-], dnl
[m4_foreach([triplet3], [gnu, gnueabi, gnueabihf], dnl
[m4_foreach([triplet_cc], [gcc, cc], dnl
[m4_append([arm_compat_compilers], dnl
triplet1[-]triplet2[linux-]triplet3[-]triplet_cc)])])])])
AC_CHECK_PROGS(CC_FOR_M32, arm_compat_compilers)
AS_IF([test -n "$CC_FOR_M32"],
[# So far, only aarch64 needs a separate compiler for its compat
# personality (which is AArch32, that is more or less ARMv7 EABI)
# First try $CC with 'aarch64' replaced with 'arm' and 'android'
# replaced with 'androideabi'
cc=${CC%/aarch64-*}/armv7a-${CC##*/aarch64-}
cc=${cc%-android*}-androideabi${cc##*-android}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why exactly should it take priority over all the other options? Also, since the CC differs from the one that ./configure would detect, I guess it is provided explicitly, why not provide CC_FOR_M32 explicitly as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see androideabi is not present among the currently checked ABI options, would simply adding it to triplet3 set help?

AS_IF([$cc --version >/dev/null],[CC_FOR_M32=$cc],[
# Some heuristics regarding possible compiler name:
# Debian: arm-linux-gnueabi{,hf}-gcc
# SuSE: arm7{,hl}-linux-gnueabi-gcc
# Red Hat: arm-redhat-linux-gnu-gcc
# Fedora: arm-linux-gnu-gcc
# ALT: armh-alt-linux-gnueabi-gcc
m4_foreach([triplet1], [arm, arm7, arm7hl, armh], dnl
[m4_foreach([triplet2], [, $host_vendor-], dnl
[m4_foreach([triplet3], [gnu, gnueabi, gnueabihf], dnl
[m4_foreach([triplet_cc], [gcc, cc], dnl
[m4_append([arm_compat_compilers], dnl
triplet1[-]triplet2[linux-]triplet3[-]triplet_cc)])])])])
AC_CHECK_PROGS(CC_FOR_M32, arm_compat_compilers)])
AS_UNSET(cc)
AS_IF([test -n "$CC_FOR_M32"],
[: ${CPP_FOR_M32=$CC_FOR_M32 -E}
: ${CFLAGS_FOR_M32=-std=gnu99}
: ${CPPFLAGS_FOR_M32=}
Expand All @@ -803,7 +808,10 @@ m4_foreach([pers], [M32, MX32], dnl
AC_SUBST(var[_FOR_]pers)])])

st_MPERS([m32], [aarch64|powerpc64|s390x|sparc64|tile|x32|x86_64])
st_MPERS([mx32], [x86_64])
case $host_os in
linux-android*) st_MPERS([mx32], [none]);;
*) st_MPERS([mx32], [x86_64]) ;;
esac

AC_ARG_ENABLE([install-tests],
[AS_HELP_STRING([--enable-install-tests=yes|no],
Expand Down
9 changes: 8 additions & 1 deletion src/affinity.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,17 @@ get_cpuset_size(void)
* This undocumented kernel feature can be used to probe
* the kernel and find out the minimal valid cpuset size
* without allocating any memory for the CPU affinity mask.
*
*/
cpuset_size = 128;
/*
* As an extra kludge, use a dummy variable in order
* to dodge the _Nonnull pointer attribute from
* sched_getaffinity()'s declaration
*/
cpu_set_t *null_ptr = NULL;
while (cpuset_size &&
sched_getaffinity(0, cpuset_size, NULL) == -1 &&
sched_getaffinity(0, cpuset_size, null_ptr) == -1 &&
EINVAL == errno) {
cpuset_size <<= 1;
}
Expand Down
13 changes: 12 additions & 1 deletion src/defs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ str_strip_prefix_len(const char *str, const char *prefix, size_t prefix_len)
* part.
*/
# define QUOTE_HEXSTR_MASK (0x3 << QUOTE_HEXSTR_SHIFT)
static_assert((NUM_HEXSTR_OPTS - 1) <= (QUOTE_HEXSTR_MASK >> QUOTE_HEXSTR_SHIFT),
_Static_assert((NUM_HEXSTR_OPTS - 1) <= (QUOTE_HEXSTR_MASK >> QUOTE_HEXSTR_SHIFT),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change is needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the build will fail without it?

The android headers only define static_assert when in c11 mode, thence:

        ./mpers.sh m32 "-std=gnu99 " $f || exit; \
done
In file included from mpers-m32/struct_blk_user_trace_setup.c:10:
./defs.h:899:32: error: expected ')'
static_assert((NUM_HEXSTR_OPTS - 1) <= (QUOTE_HEXSTR_MASK >> QUOTE_HEXST...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

We have static_assert.h (included by defs.h) to handle this on various platforms.
Please have a look why it didn't work out in your case.

"xflag options do not fit into QUOTE_HEXSTR_MASK");

# define QUOTE_HEXSTR_NONE (HEXSTR_NONE << QUOTE_HEXSTR_SHIFT)
Expand Down Expand Up @@ -2158,4 +2158,15 @@ print_big_u64_addr(const uint64_t addr)
# include "syscall.h"
# endif

#ifdef __ANDROID__
/* pulled in by stdio.h, clash with bundled/linux/include/uapi/linux/fs.h */
#undef RENAME_EXCHANGE
#undef RENAME_WHITEOUT
#undef RENAME_NOREPLACE

/* defined in linux/in.h on android; shadowed by its bundled/... counterpart */
typedef uint32_t in_addr_t;

#endif

#endif /* !STRACE_DEFS_H */
2 changes: 1 addition & 1 deletion src/sigreturn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#endif

/* The following function might be unused, hence the inline qualifier. */
static inline void
static inline void __attribute__((unused))
Comment on lines 18 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this change is not needed, the compiler shouldn't issue any warnings for static inline functions.
On the contrary, the compiler will issue a warning if a function marked as unused is actually used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your supposition is false:

 echo 'static inline void foo(){}' | clang -Wall -Werror -c -xc -
<stdin>:1:20: error: unused function 'foo'
      [-Werror,-Wunused-function]
static inline void foo(){}
                   ^
1 error generated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this is a compiler bug, a lot of software relies on this static inline thing, please report it to compiler people instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to gcc documentation,

-Wunused-function

    Warn whenever a static function is declared but not defined or a non-inline static function is unused. This warning is enabled by -Wall.

Sorry, I'm not going to workaround incorrect warnings reported by non-compliant compilers.

print_sigmask_addr_size(const void *const addr, const unsigned int size)
{
tprint_struct_begin();
Expand Down
0