Closed KJTsanaktsidis closed 10 months ago
@KJTsanaktsidis @ioquatix This changeset caused build failure on cross-compiling to android30
compiling zlib.c
zlib.c:48:53: warning: type specifier missing, defaults to 'int' [-Wimplicit-int]
typedef uLong (*checksum_func)(uLong, const Bytef*, z_size_t);
^
zlib.c:436:70: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
sum = checksum_long(func, sum, (Bytef*)RSTRING_PTR(buf), RSTRING_LEN(buf));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
../.././include/ruby/internal/core/rstring.h:46:27: note: expanded from macro 'RSTRING_LEN'
#define RSTRING_LEN RSTRING_LEN
^
zlib.c:406:72: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
~ ^~~
zlib.c:441:59: warning: implicit conversion loses integer precision: 'long' to 'int' [-Wshorten-64-to-32]
sum = checksum_long(func, sum, (Bytef*)RSTRING_PTR(str), RSTRING_LEN(str));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
../.././include/ruby/internal/core/rstring.h:46:27: note: expanded from macro 'RSTRING_LEN'
#define RSTRING_LEN RSTRING_LEN
^
zlib.c:406:72: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
~ ^~~
zlib.c:469:36: error: use of undeclared identifier 'adler32_z'; did you mean 'adler32'?
return do_checksum(argc, argv, adler32);
^~~~~~~
adler32
zlib.c:50:18: note: expanded from macro 'adler32'
# define adler32 adler32_z
^
/home/chkbuild/opt/android-ndk-r21e/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/zlib.h:1552:23: note: 'adler32' declared here
ZEXTERN uLong ZEXPORT adler32 OF((uLong adler, const Bytef *buf, uInt len));
^
zlib.c:509:36: error: use of undeclared identifier 'crc32_z'; did you mean 'crc32'?
return do_checksum(argc, argv, crc32);
^~~~~
crc32
zlib.c:49:16: note: expanded from macro 'crc32'
# define crc32 crc32_z
^
/home/chkbuild/opt/android-ndk-r21e/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/zlib.h:1583:23: note: 'crc32' declared here
ZEXTERN uLong ZEXPORT crc32 OF((uLong crc, const Bytef *buf, uInt len));
^
zlib.c:2093:50: error: use of undeclared identifier 'adler32_z'; did you mean 'adler32'?
VALUE checksum = do_checksum(1, &dictionary, adler32);
^~~~~~~
adler32
zlib.c:50:18: note: expanded from macro 'adler32'
# define adler32 adler32_z
^
/home/chkbuild/opt/android-ndk-r21e/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/zlib.h:1552:23: note: 'adler32' declared here
ZEXTERN uLong ZEXPORT adler32 OF((uLong adler, const Bytef *buf, uInt len));
^
zlib.c:2442:15: warning: implicit declaration of function 'crc32_z' is invalid in C99 [-Wimplicit-function-declaration]
gz->crc = crc32(0, Z_NULL, 0);
^
zlib.c:49:16: note: expanded from macro 'crc32'
# define crc32 crc32_z
^
zlib.c:2473:15: warning: implicit declaration of function 'crc32_z' is invalid in C99 [-Wimplicit-function-declaration]
gz->crc = crc32(0, Z_NULL, 0);
^
zlib.c:49:16: note: expanded from macro 'crc32'
# define crc32 crc32_z
^
zlib.c:2817:26: error: use of undeclared identifier 'crc32_z'; did you mean 'crc32'?
gz->crc = checksum_long(crc32, gz->crc, str, len);
^~~~~
crc32
zlib.c:49:16: note: expanded from macro 'crc32'
# define crc32 crc32_z
^
zlib.c:406:45: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
^
/home/chkbuild/opt/android-ndk-r21e/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/zlib.h:1583:23: note: 'crc32' declared here
ZEXTERN uLong ZEXPORT crc32 OF((uLong crc, const Bytef *buf, uInt len));
^
zlib.c:2817:47: warning: implicit conversion loses integer precision: 'long' to 'uInt' (aka 'unsigned int') [-Wshorten-64-to-32]
gz->crc = checksum_long(crc32, gz->crc, str, len);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
zlib.c:406:72: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
~ ^~~
zlib.c:2854:26: error: use of undeclared identifier 'crc32_z'; did you mean 'crc32'?
gz->crc = checksum_long(crc32, gz->crc, (Bytef*)RSTRING_PTR(str) + gz->ungetc,
^~~~~
crc32
zlib.c:49:16: note: expanded from macro 'crc32'
# define crc32 crc32_z
^
zlib.c:406:45: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
^
/home/chkbuild/opt/android-ndk-r21e/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/zlib.h:1583:23: note: 'crc32' declared here
ZEXTERN uLong ZEXPORT crc32 OF((uLong crc, const Bytef *buf, uInt len));
^
zlib.c:2855:22: warning: implicit conversion loses integer precision: 'long' to 'uInt' (aka 'unsigned int') [-Wshorten-64-to-32]
RSTRING_LEN(str) - gz->ungetc);
~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
zlib.c:406:72: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
~ ^~~
zlib.c:4553:26: error: use of undeclared identifier 'crc32_z'; did you mean 'crc32'?
gz->crc = checksum_long(crc32, gz->crc, ptr, len);
^~~~~
crc32
zlib.c:49:16: note: expanded from macro 'crc32'
# define crc32 crc32_z
^
zlib.c:406:45: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
^
/home/chkbuild/opt/android-ndk-r21e/toolchains/llvm/prebuilt/linux-x86_64/bin/../sysroot/usr/include/zlib.h:1583:23: note: 'crc32' declared here
ZEXTERN uLong ZEXPORT crc32 OF((uLong crc, const Bytef *buf, uInt len));
^
zlib.c:4553:47: warning: implicit conversion loses integer precision: 'long' to 'uInt' (aka 'unsigned int') [-Wshorten-64-to-32]
gz->crc = checksum_long(crc32, gz->crc, ptr, len);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
zlib.c:406:72: note: expanded from macro 'checksum_long'
#define checksum_long(func, sum, ptr, len) (func)(mask32(sum), (ptr), (len))
~ ^~~
8 warnings and 6 errors generated.
gmake[2]: *** [Makefile:254: zlib.o] Error 1
Eurgh - thanks for letting me know. Looking at it now...
OK, so this is pretty amusing.
When extconf runs (with the toolchain from android-ndk-r21e), it actually detects that the crc32_z
and adler32_z
functions are available. This is the mkmf.log output (I also added have_type("z_size_t", "zlib.h")
back for comparison:
have_type: checking for z_size_t in zlib.h... -------------------- no
LD_LIBRARY_PATH=.:../.. "x86_64-linux-android30-clang -I../../.ext/include/x86_64-linux-android-android -I../.././include -I../.././ext/zlib -fdeclspec -O3 -fno-fast-math -ggdb3 -Wall -Wextra -Wextra-tokens -Wdeprecated-declarations -Wdivision-by-zero -Wdiv-by-zero -Wimplicit-function-declaration -Wimplicit-int -Wpointer-arith -Wshorten-64-to-32 -Wwrite-strings -Wold-style-definition -Wmissing-noreturn -Wno-constant-logical-operand -Wno-long-long -Wno-missing-field-initializers -Wno-overlength-strings -Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare -Wno-unused-parameter -Wno-unused-value -Wunused-variable -Wundef -c conftest.c"
conftest.c:6:9: error: unknown type name 'z_size_t'
typedef z_size_t conftest_type;
^
1 error generated.
checked program was:
/* begin */
1: #include "ruby.h"
2:
3: #include <zlib.h>
4:
5: /*top*/
6: typedef z_size_t conftest_type;
7: int conftestval[sizeof(conftest_type)?1:-1];
/* end */
--------------------
have_func: checking for crc32_z() in zlib.h... -------------------- yes
LD_LIBRARY_PATH=.:../.. "x86_64-linux-android30-clang -o conftest -I../../.ext/include/x86_64-linux-android-android -I../.././include -I../.././ext/zlib -fdeclspec -O3 -fno-fast-math -ggdb3 -Wall -Wextra -Wextra-tokens -Wdeprecated-declarations -Wdivision-by-zero -Wdiv-by-zero -Wimplicit-function-declaration -Wimplicit-int -Wpointer-arith -Wshorten-64-to-32 -Wwrite-strings -Wold-style-definition -Wmissing-noreturn -Wno-constant-logical-operand -Wno-long-long -Wno-missing-field-initializers -Wno-overlength-strings -Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare -Wno-unused-parameter -Wno-unused-value -Wunused-variable -Wundef conftest.c -L. -L../.. -L. -fstack-protector-strong -rdynamic -Wl,-export-dynamic -lz -Wl,-rpath,/home/kj/ruby -lruby-static -lz -ldl -lm -lz -lm -lc"
conftest.c:16:57: error: use of undeclared identifier 'crc32_z'
int t(void) { void ((*volatile p)()); p = (void ((*)()))crc32_z; return !p; }
^
1 error generated.
checked program was:
/* begin */
1: #include "ruby.h"
2:
3: #include <zlib.h>
4:
5: /*top*/
6: extern int t(void);
7: int main(int argc, char **argv)
8: {
9: if (argc > 1000000) {
10: int (* volatile tp)(void)=(int (*)(void))&t;
11: printf("%d", (*tp)());
12: }
13:
14: return !!argv[argc];
15: }
16: int t(void) { void ((*volatile p)()); p = (void ((*)()))crc32_z; return !p; }
/* end */
LD_LIBRARY_PATH=.:../.. "x86_64-linux-android30-clang -o conftest -I../../.ext/include/x86_64-linux-android-android -I../.././include -I../.././ext/zlib -fdeclspec -O3 -fno-fast-math -ggdb3 -Wall -Wextra -Wextra-tokens -Wdeprecated-declarations -Wdivision-by-zero -Wdiv-by-zero -Wimplicit-function-declaration -Wimplicit-int -Wpointer-arith -Wshorten-64-to-32 -Wwrite-strings -Wold-style-definition -Wmissing-noreturn -Wno-constant-logical-operand -Wno-long-long -Wno-missing-field-initializers -Wno-overlength-strings -Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare -Wno-unused-parameter -Wno-unused-value -Wunused-variable -Wundef conftest.c -L. -L../.. -L. -fstack-protector-strong -rdynamic -Wl,-export-dynamic -lz -Wl,-rpath,/home/kj/ruby -lruby-static -lz -ldl -lm -lz -lm -lc"
checked program was:
/* begin */
1: #include "ruby.h"
2:
3: #include <zlib.h>
4:
5: /*top*/
6: extern int t(void);
7: int main(int argc, char **argv)
8: {
9: if (argc > 1000000) {
10: int (* volatile tp)(void)=(int (*)(void))&t;
11: printf("%d", (*tp)());
12: }
13:
14: return !!argv[argc];
15: }
16: extern void crc32_z();
17: int t(void) { crc32_z(); return 0; }
/* end */
--------------------
We can see:
zlib.h
header doesn't contain z_size_t
, OK. That's why this was building fine before.crc32_z
though!extern void crc32_z()
and links it. That actually succeeds!The problem is that in android-ndk-r21e:
zlib.h
really doesn't have z_size_t
or crc32_z
:
kj@kj-thinkpad ruby % grep -c crc32_z ~/android-ndk-r21e/sysroot/usr/include/zlib.h
0
But the provided zlib.a
archive does have the symbol in it!
kj@kj-thinkpad ruby % nm ~/android-ndk-r21e/sysroot/usr/lib/x86_64-linux-android/libz.a | grep crc32_z
0000000000000000 T crc32_z
So the link test succeeds, and thus have_func("crc32_z", "zlib.h")
is considered to be true, and then it blows up at compile time when the extension tries to actually use the definitions in the header file.
There's two things which could be considered to be "the problem" here:
have_func
shouldn't be checking if the program is linkable if the symbol is declared extern; it should fail if the symbol isn't in the header file (especially if a header file is actually passed to it)Obviously we can't fix the NDK... so I think our choices are:
have_func
in mkmf behaveshave_type("z_size_t", "zlib.h")
like how it was before my change.Obviously point 1 requires a ticket, some analysis as to what will break in the ecosystem, and probably presentation at a dev meeting, so I'll open a PR to hack around it with option 2.
We use the Cloudflare fork of zlib (https://github.com/cloudflare/zlib), which we find gives improved performance on AWS Graviton ARM instances. That fork does not define crc32_z and alder32_z functions.
Until two days ago, Ruby's zlib gem worked fine, because cloudflare zlib also did not define z_size_t, which meant Ruby did not try and use these functions.
Since https://github.com/cloudflare/zlib/commit/a3ba99596d6271224d39ef9d6853511f51821e05 however, cloudflare zlib does define z_size_t (but NOT crc32_z or alder32_z). The zlib gem would try and use these nonexistant functions and not compile.
This patch fixes it by actually specifically detecting the functions that the gem wants to call, rather than just the presence of the z_size_t type.