Skip to content

nghttp3_conv: fix call to undeclared functions 'ntohl' and 'htons' - #49979

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MatteoBax:main
Oct 2, 2023
Merged

nghttp3_conv: fix call to undeclared functions 'ntohl' and 'htons'#49979
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
MatteoBax:main

Conversation

@MatteoBax

@MatteoBaxMatteoBax commented Sep 30, 2023

Copy link
Copy Markdown
Contributor

I don't think it's the right method but this solves these errors when compiling for Android arm64:


../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:49:21: error: call to undeclared function 'ntohs'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
return (int64_t)ntohs(n.n16);
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:53:21: error: call to undeclared function 'ntohl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
return (int64_t)ntohl(n.n32);
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:69:7: error: call to undeclared function 'ntohl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
n = nghttp3_htonl64(n);
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:73:32: note: expanded from macro 'nghttp3_htonl64'
# define nghttp3_htonl64(N) nghttp3_bswap64(N)
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:60:17: note: expanded from macro 'nghttp3_bswap64'
((uint64_t)(ntohl((uint32_t)(N))) << 32 | ntohl((uint32_t)((N) >> 32)))
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:74:7: error: call to undeclared function 'ntohl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
n = nghttp3_htonl64(n);
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:73:32: note: expanded from macro 'nghttp3_htonl64'
# define nghttp3_htonl64(N) nghttp3_bswap64(N)
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.h:60:17: note: expanded from macro 'nghttp3_bswap64'
((uint64_t)(ntohl((uint32_t)(N))) << 32 | ntohl((uint32_t)((N) >> 32)))
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:79:7: error: call to undeclared function 'htonl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
n = htonl(n);
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:84:7: error: call to undeclared function 'htonl'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
n = htonl(n);
^
../deps/ngtcp2/nghttp3/lib/nghttp3_conv.c:89:7: error: call to undeclared function 'htons'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
n = htons(n);
^
7 errors generated.

I added -Wno-implicit-function-declaration option to cflags

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-botnodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Sep 30, 2023
@MatteoBaxMatteoBax changed the title ngtcp2: fix call to undeclared function 'ntohl'nghttp3_conv: fix call to undeclared functions 'ntohl' and 'htons'Sep 30, 2023
@mscdex

mscdex commented Sep 30, 2023

Copy link
Copy Markdown
Contributor

This should be unnecessary as I believe this was already fixed upstream in v0.12.0. Our copy of nghttp3 just needs to be upgraded.

@bnoordhuis

Copy link
Copy Markdown
Member

That's not a proper fix, it possibly doesn't even produce a functional build. Can you try this patch?

diff --git a/deps/ngtcp2/ngtcp2.gyp b/deps/ngtcp2/ngtcp2.gyp
index a47a791610..fd6eaddb01 100644
--- a/deps/ngtcp2/ngtcp2.gyp+++ b/deps/ngtcp2/ngtcp2.gyp@@ -112,7 +112,7 @@
},
},
}],
- ['OS=="linux"', {+ ['OS in "android linux"', {
'defines': [
'HAVE_ARPA_INET_H',
'HAVE_NETINET_IN_H',
@@ -162,7 +162,7 @@
},
},
}],
- ['OS=="linux"', {+ ['OS in "android linux"', {
'defines': [
'HAVE_ARPA_INET_H',
'HAVE_NETINET_IN_H',

@MatteoBax

Copy link
Copy Markdown
ContributorAuthor

That's not a proper fix, it possibly doesn't even produce a functional build. Can you try this patch?

diff --git a/deps/ngtcp2/ngtcp2.gyp b/deps/ngtcp2/ngtcp2.gyp
index a47a791610..fd6eaddb01 100644
--- a/deps/ngtcp2/ngtcp2.gyp+++ b/deps/ngtcp2/ngtcp2.gyp@@ -112,7 +112,7 @@
},
},
}],
- ['OS=="linux"', {+ ['OS in "android linux"', {
'defines': [
'HAVE_ARPA_INET_H',
'HAVE_NETINET_IN_H',
@@ -162,7 +162,7 @@
},
},
}],
- ['OS=="linux"', {+ ['OS in "android linux"', {
'defines': [
'HAVE_ARPA_INET_H',
'HAVE_NETINET_IN_H',

With this patch those problems no longer occur, but now the problems are:

../deps/v8/src/base/debug/stack_trace_posix.cc:156:9: error: use of undeclared identifier 'backtrace_symbols'
backtrace_symbols(trace, static_cast<int>(size)));
^
/home/matteo/Android/Sdk/ndk/26.0.10792818/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android24-clang++ -o /home/matteo/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/division-by-constant.o ../deps/v8/src/base/division-by-constant.cc '-D_GLIBCXX_USE_CXX11_ABI=1' '-DNODE_OPENSSL_CONF_NAME=nodejs_conf' '-DNODE_OPENSSL_HAS_QUIC' '-DICU_NO_USER_DATA_OVERRIDE' '-DV8_GYP_BUILD' '-DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64' '-D__STDC_FORMAT_MACROS' '-DOPENSSL_NO_PINSHARED' '-DOPENSSL_THREADS' '-DOPENSSL_NO_ASM' '-DV8_TARGET_ARCH_ARM64' '-DV8_HAVE_TARGET_OS' '-DV8_TARGET_OS_ANDROID' '-DV8_EMBEDDER_STRING="-node.15"' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT=1' '-DOBJECT_PRINT' '-DV8_INTL_SUPPORT' '-DV8_ATOMIC_OBJECT_FIELD_WRITES' '-DV8_ENABLE_LAZY_SOURCE_POSITIONS' '-DV8_USE_SIPHASH' '-DV8_SHARED_RO_HEAP' '-DV8_WIN64_UNWINDING_INFO' '-DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH' '-DV8_USE_ZLIB' '-DV8_ENABLE_TURBOFAN' '-DV8_ENABLE_WEBASSEMBLY' '-DV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS' '-DV8_ALLOCATION_FOLDING' '-DV8_ALLOCATION_SITE_TRACKING' '-DV8_SCRIPTORMODULE_LEGACY_LIFETIME' '-DV8_ADVANCED_BIGINT_ALGORITHMS' '-DBUILDING_V8_BASE_SHARED' '-D_GLIBCXX_USE_C99_MATH' -I../deps/v8 -I../deps/v8/include -msign-return-address=all -Wno-unused-parameter -Wno-return-type -fno-omit-frame-pointer -fPIC -I/home/matteo/Android/Sdk/ndk/26.0.10792818/sources/android/cpufeatures -fdata-sections -ffunction-sections -O2 -fno-rtti -fno-exceptions -std=gnu++17 -MMD -MF /home/matteo/node/out/Release/.deps//home/matteo/node/out/Release/obj.target/v8_libbase/deps/v8/src/base/division-by-constant.o.d.raw -c
../deps/v8/src/base/debug/stack_trace_posix.cc:371:32: error: use of undeclared identifier 'backtrace'; did you mean 'StackTrace'?
count_ = static_cast<size_t>(backtrace(trace_, arraysize(trace_)));
^
../deps/v8/src/base/debug/stack_trace.h:41:22: note: 'StackTrace' declared here
class V8_BASE_EXPORT StackTrace {
^
2 errors generated.
make[1]: *** [tools/v8_gypfiles/v8_libbase.host.mk:186: /home/matteo/node/out/Release/obj.host/v8_libbase/deps/v8/src/base/debug/stack_trace_posix.o] Errore 1
make[1]: *** Attesa per i processi non terminati....

@bnoordhuis

Copy link
Copy Markdown
Member

Covered in #46952.

@MatteoBax

Copy link
Copy Markdown
ContributorAuthor

Covered in #46952.

Ok. I revert the changes and applied the patch. Instead of using 'OS in "android linux"' I used 'OS=="linux" or OS=="android"' but I think it's the same thing.

This reverts commit 2070ad27a5fc674909f4bbd34e7d862c625fc54b.
deps: fix call to undeclared functions 'ntohl' and 'htons'
@lpincalpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 1, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@MatteoBax

Copy link
Copy Markdown
ContributorAuthor

What shall I do to fix failed checks?

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@bnoordhuis

Copy link
Copy Markdown
Member

They're flaky tests. Not much you can do.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@richardlaurichardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 1, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 2, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 61411bb into nodejs:mainOct 2, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 61411bb

targos pushed a commit that referenced this pull request Oct 28, 2023
This reverts commit 2070ad27a5fc674909f4bbd34e7d862c625fc54b.
deps: fix call to undeclared functions 'ntohl' and 'htons'
PR-URL: #49979
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <rlau@redhat.com>
targos pushed a commit that referenced this pull request Nov 11, 2023
This reverts commit 2070ad27a5fc674909f4bbd34e7d862c625fc54b.
deps: fix call to undeclared functions 'ntohl' and 'htons'
PR-URL: #49979
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <rlau@redhat.com>
@targostargos mentioned this pull request Nov 12, 2023
@targostargos mentioned this pull request Nov 28, 2023
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
This reverts commit 2070ad27a5fc674909f4bbd34e7d862c625fc54b.
deps: fix call to undeclared functions 'ntohl' and 'htons'
PR-URL: nodejs#49979
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <rlau@redhat.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
This reverts commit 2070ad27a5fc674909f4bbd34e7d862c625fc54b.
deps: fix call to undeclared functions 'ntohl' and 'htons'
PR-URL: nodejs/node#49979
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <rlau@redhat.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
This reverts commit 2070ad27a5fc674909f4bbd34e7d862c625fc54b.
deps: fix call to undeclared functions 'ntohl' and 'htons'
PR-URL: nodejs/node#49979
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <rlau@redhat.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to build files or the CI.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@MatteoBax@nodejs-github-bot@mscdex@bnoordhuis@richardlau@lpinca