minor improvements starting from NDK r16 (#1458) #1466

wongsyrone · 2017-11-28T00:46:29Z

Signed-off-by: Syrone Wong wong.syrone@gmail.com

Type of changes

Put an x inside the [ ] that applies.

Bugfix
New feature
Translation
General refinement
Other:

Details

Please provide more details about changes if necessary.

Mygod

LGTM. As long as everything still works.

wongsyrone · 2017-11-28T00:49:21Z

Working well on Nexus 6P Android 8.0 with self-built android-21 apk

wongsyrone · 2017-11-28T00:56:45Z

I wonder if you would like to insert another sbt command when building the apk:

sbt clean-files, this one deletes more files than clean

Mygod · 2017-11-28T00:58:16Z

We already have go-clean. Does that answer your question?

wongsyrone · 2017-11-28T01:02:34Z

This is the example output when using it, I think it's different from go-clean

[info] plugin/*:cleanFiles

[info] 	List(/home/travis/build/example/example-android/lib_managed, /home/travis/build/example/example-android/plugin/target, /home/travis/build/example/example-android/plugin/target/android)

[info] mobile/*:cleanFiles

[info] 	List(/home/travis/build/example/example-android/lib_managed, /home/travis/build/example/example-android/mobile/target, /home/travis/build/example/example-android/mobile/target/android)

[info] example-android/*:cleanFiles

[info] 	List(/home/travis/build/example/example-android/lib_managed, /home/travis/build/example/example-android/target)

Mygod · 2017-11-28T01:07:14Z

Oh do you mean add that command to the Travis build script or README? I don't think those folders are cached in the Travis environment and I don't know if the clean-file command really does anything useful.

By the way, it seems the build is failing. Care to fix? :)

wongsyrone · 2017-11-28T01:12:48Z

Add to README obviously, since you're not using it to generate apk, cache got dropped when finished.

/home/travis/build/shadowsocks/shadowsocks-android/mobile/target/android/intermediates/ndk/obj/local/armeabi-v7a/libsodium.a(scalarmult_curve25519.o):/home/travis/build/shadowsocks/shadowsocks-android/mobile/src/main/jni/../jni/libsodium/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c:implementation: error: undefined reference to 'crypto_scalarmult_curve25519_ref10_implementation'

/home/travis/build/shadowsocks/shadowsocks-android/mobile/src/main/jni/../jni/libsodium/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c:66: error: undefined reference to 'crypto_scalarmult_curve25519_ref10_implementation'

clang++: error: linker command failed with exit code 1 (use -v to see invocation)

make: *** [/home/travis/build/shadowsocks/shadowsocks-android/mobile/target/android/intermediates/ndk/obj/local/armeabi-v7a/libss-local.so] Error 1

make: *** Waiting for unfinished jobs....

/home/travis/build/shadowsocks/shadowsocks-android/mobile/target/android/intermediates/ndk/obj/local/armeabi-v7a/libsodium.a(scalarmult_curve25519.o):/home/travis/build/shadowsocks/shadowsocks-android/mobile/src/main/jni/../jni/libsodium/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c:implementation: error: undefined reference to 'crypto_scalarmult_curve25519_ref10_implementation'

/home/travis/build/shadowsocks/shadowsocks-android/mobile/src/main/jni/../jni/libsodium/src/libsodium/crypto_scalarmult/curve25519/scalarmult_curve25519.c:66: error: undefined reference to 'crypto_scalarmult_curve25519_ref10_implementation'

clang++: error: linker command failed with exit code 1 (use -v to see invocation)

It seems you forgot to update libsodium files list after upgrading its version.

Mygod · 2017-11-28T01:15:21Z

Base branch is building though.

Mygod · 2017-11-28T01:16:06Z

So is there a case where using clean-file actually fixes a problem?

wongsyrone · 2017-11-28T01:16:09Z

Part of my libsodium changes and configured via aarch64 standalone toolchain, you might want to do so to android-19

diff --git a/mobile/src/main/jni/Android.mk b/mobile/src/main/jni/Android.mk
index ee7c03c32..5c5d21c39 100755
--- a/mobile/src/main/jni/Android.mk
+++ b/mobile/src/main/jni/Android.mk
@@ -27,52 +27,140 @@ include $(CLEAR_VARS)
 SODIUM_SOURCE := \
 	crypto_aead/chacha20poly1305/sodium/aead_chacha20poly1305.c \
 	crypto_aead/xchacha20poly1305/sodium/aead_xchacha20poly1305.c \
+	crypto_auth/crypto_auth.c \
+	crypto_auth/hmacsha256/auth_hmacsha256.c \
+	crypto_auth/hmacsha512/auth_hmacsha512.c \
+	crypto_auth/hmacsha512256/auth_hmacsha512256.c \
+	crypto_box/crypto_box.c \
+	crypto_box/crypto_box_easy.c \
+	crypto_box/crypto_box_seal.c \
+	crypto_box/curve25519xsalsa20poly1305/box_curve25519xsalsa20poly1305.c \
+	crypto_core/curve25519/ref10/curve25519_ref10.c \
 	crypto_core/hchacha20/core_hchacha20.c \
+	crypto_core/hsalsa20/ref2/core_hsalsa20_ref2.c \
+	crypto_core/hsalsa20/core_hsalsa20.c \
 	crypto_core/salsa/ref/core_salsa_ref.c \
+	crypto_generichash/crypto_generichash.c \
+	crypto_generichash/blake2b/generichash_blake2.c \
 	crypto_generichash/blake2b/ref/blake2b-compress-ref.c \
 	crypto_generichash/blake2b/ref/blake2b-ref.c \
 	crypto_generichash/blake2b/ref/generichash_blake2b.c \
+	crypto_hash/crypto_hash.c \
+	crypto_hash/sha256/hash_sha256.c \
+	crypto_hash/sha256/cp/hash_sha256_cp.c \
+	crypto_hash/sha512/hash_sha512.c \
+	crypto_hash/sha512/cp/hash_sha512_cp.c \
+	crypto_kdf/blake2b/kdf_blake2b.c \
+	crypto_kdf/crypto_kdf.c \
+	crypto_kx/crypto_kx.c \
+	crypto_onetimeauth/crypto_onetimeauth.c \
 	crypto_onetimeauth/poly1305/onetimeauth_poly1305.c \
 	crypto_onetimeauth/poly1305/donna/poly1305_donna.c \
-	crypto_pwhash/crypto_pwhash.c \
 	crypto_pwhash/argon2/argon2-core.c \
-	crypto_pwhash/argon2/argon2.c \
 	crypto_pwhash/argon2/argon2-encoding.c \
 	crypto_pwhash/argon2/argon2-fill-block-ref.c \
+	crypto_pwhash/argon2/argon2.c \
 	crypto_pwhash/argon2/blake2b-long.c \
 	crypto_pwhash/argon2/pwhash_argon2i.c \
+	crypto_pwhash/argon2/pwhash_argon2id.c \
+	crypto_pwhash/crypto_pwhash.c \
+	crypto_scalarmult/crypto_scalarmult.c \
 	crypto_scalarmult/curve25519/scalarmult_curve25519.c \
+	crypto_secretbox/crypto_secretbox.c \
+	crypto_secretbox/crypto_secretbox_easy.c \
+	crypto_secretbox/xsalsa20poly1305/secretbox_xsalsa20poly1305.c \
+	crypto_secretstream/xchacha20poly1305/secretstream_xchacha20poly1305.c \
+	crypto_shorthash/crypto_shorthash.c \
+	crypto_shorthash/siphash24/shorthash_siphash24.c \
+	crypto_shorthash/siphash24/ref/shorthash_siphash24_ref.c \
+	crypto_sign/crypto_sign.c \
+	crypto_sign/ed25519/sign_ed25519.c \
+	crypto_sign/ed25519/ref10/keypair.c \
+	crypto_sign/ed25519/ref10/open.c \
+	crypto_sign/ed25519/ref10/sign.c \
 	crypto_stream/chacha20/stream_chacha20.c \
 	crypto_stream/chacha20/ref/chacha20_ref.c \
+	crypto_stream/crypto_stream.c \
 	crypto_stream/salsa20/stream_salsa20.c \
-	crypto_stream/salsa20/ref/salsa20_ref.c \
+	crypto_stream/xsalsa20/stream_xsalsa20.c \
 	crypto_verify/sodium/verify.c \
 	randombytes/randombytes.c \
-	randombytes/sysrandom/randombytes_sysrandom.c \
+	sodium/codecs.c \
 	sodium/core.c \
 	sodium/runtime.c \
 	sodium/utils.c \
-	sodium/version.c
+	sodium/version.c \
+	randombytes/salsa20/randombytes_salsa20_random.c \
+	randombytes/sysrandom/randombytes_sysrandom.c \
+	crypto_scalarmult/curve25519/ref10/x25519_ref10.c \
+	crypto_stream/salsa20/ref/salsa20_ref.c \
+	crypto_box/curve25519xchacha20poly1305/box_curve25519xchacha20poly1305.c \
+	crypto_box/curve25519xchacha20poly1305/box_seal_curve25519xchacha20poly1305.c \
+	crypto_pwhash/scryptsalsa208sha256/crypto_scrypt-common.c \
+	crypto_pwhash/scryptsalsa208sha256/scrypt_platform.c \
+	crypto_pwhash/scryptsalsa208sha256/pbkdf2-sha256.c \
+	crypto_pwhash/scryptsalsa208sha256/pwhash_scryptsalsa208sha256.c \
+	crypto_pwhash/scryptsalsa208sha256/nosse/pwhash_scryptsalsa208sha256_nosse.c \
+	crypto_secretbox/xchacha20poly1305/secretbox_xchacha20poly1305.c \
+	crypto_shorthash/siphash24/shorthash_siphashx24.c \
+	crypto_shorthash/siphash24/ref/shorthash_siphashx24_ref.c \
+	crypto_sign/ed25519/ref10/obsolete.c \
+	crypto_stream/salsa2012/ref/stream_salsa2012_ref.c \
+	crypto_stream/salsa2012/stream_salsa2012.c \
+	crypto_stream/salsa208/ref/stream_salsa208_ref.c \
+	crypto_stream/salsa208/stream_salsa208.c \
+	crypto_stream/xchacha20/stream_xchacha20.c
 
 LOCAL_MODULE := sodium
-LOCAL_CFLAGS += -O2 -I$(LOCAL_PATH)/libsodium/src/libsodium/include \
+LOCAL_CFLAGS += -I$(LOCAL_PATH)/libsodium/src/libsodium/include \
 				-I$(LOCAL_PATH)/include \
 				-I$(LOCAL_PATH)/include/sodium \
 				-I$(LOCAL_PATH)/libsodium/src/libsodium/include/sodium \
 				-DPACKAGE_NAME=\"libsodium\" -DPACKAGE_TARNAME=\"libsodium\" \
-				-DPACKAGE_VERSION=\"1.0.7\" -DPACKAGE_STRING=\"libsodium\ 1.0.7\" \
+				-DPACKAGE_VERSION=\"1.0.15\" -DPACKAGE_STRING=\"libsodium\ 1.0.15\" \
 				-DPACKAGE_BUGREPORT=\"https://github.com/jedisct1/libsodium/issues\" \
 				-DPACKAGE_URL=\"https://github.com/jedisct1/libsodium\" \
-				-DPACKAGE=\"libsodium\" -DVERSION=\"1.0.7\" -DSTDC_HEADERS=1 \
-				-DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 \
-				-DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 \
-				-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 \
-				-D__EXTENSIONS__=1 -D_ALL_SOURCE=1 -D_GNU_SOURCE=1 \
-				-D_POSIX_PTHREAD_SEMANTICS=1 -D_TANDEM_SOURCE=1 \
-				-DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" \
-				-DHAVE_SYS_MMAN_H=1 -DNATIVE_LITTLE_ENDIAN=1 \
-				-DHAVE_WEAK_SYMBOLS=1 -DHAVE_ARC4RANDOM=1 -DHAVE_ARC4RANDOM_BUF=1 \
-				-DHAVE_MLOCK=1 -DHAVE_MPROTECT=1 -DHAVE_POSIX_MEMALIGN=1
+				-DPACKAGE=\"libsodium\" -DVERSION=\"1.0.15\" \
+				-DHAVE_PTHREAD=1                  \
+				-DSTDC_HEADERS=1                  \
+				-DHAVE_SYS_TYPES_H=1              \
+				-DHAVE_SYS_STAT_H=1               \
+				-DHAVE_STDLIB_H=1                 \
+				-DHAVE_STRING_H=1                 \
+				-DHAVE_MEMORY_H=1                 \
+				-DHAVE_STRINGS_H=1                \
+				-DHAVE_INTTYPES_H=1               \
+				-DHAVE_STDINT_H=1                 \
+				-DHAVE_UNISTD_H=1                 \
+				-D__EXTENSIONS__=1                \
+				-D_ALL_SOURCE=1                   \
+				-D_GNU_SOURCE=1                   \
+				-D_POSIX_PTHREAD_SEMANTICS=1      \
+				-D_TANDEM_SOURCE=1                \
+				-DHAVE_DLFCN_H=1                  \
+				-DLT_OBJDIR=\".libs/\"            \
+				-DHAVE_SYS_MMAN_H=1               \
+				-DNATIVE_LITTLE_ENDIAN=1          \
+				-DASM_HIDE_SYMBOL=.hidden         \
+				-DHAVE_WEAK_SYMBOLS=1             \
+				-DHAVE_ATOMIC_OPS=1               \
+				-DHAVE_ARC4RANDOM=1               \
+				-DHAVE_ARC4RANDOM_BUF=1           \
+				-DHAVE_MMAP=1                     \
+				-DHAVE_MLOCK=1                    \
+				-DHAVE_MADVISE=1                  \
+				-DHAVE_MPROTECT=1                 \
+				-DHAVE_NANOSLEEP=1                \
+				-DHAVE_POSIX_MEMALIGN=1           \
+				-DHAVE_GETPID=1                   \
+				-DCONFIGURED=1

wongsyrone · 2017-11-28T01:17:13Z

Just suggestion, depends on your thoughts.

wongsyrone · 2017-11-28T01:24:21Z

The missing ref locates at crypto_scalarmult/curve25519/ref10/x25519_ref10.c.

So, don't forget to update files list after upgrading components.

madeye · 2017-11-28T01:29:50Z

Could you fix the build error here?

wongsyrone · 2017-11-28T01:32:11Z

I will just paste the diff above then since I'm using git-for-windows to send this patch. Also, I'm not sure configured android-21 setting also applies to android-19.

wongsyrone · 2017-11-28T01:36:09Z

If you think ready to go, I will copy & paste & update this branch & do force push.

Signed-off-by: Syrone Wong <wong.syrone@gmail.com>

wongsyrone · 2017-11-28T01:41:47Z

force pushed, let's see whether it compiles.

wongsyrone · 2017-11-28T01:54:06Z

OK. Fixed. Please configure libsodium via correct standalone toolchain to make sure all works as expected.

madeye · 2017-11-28T02:03:58Z

Thanks, merged.

madeye · 2017-11-28T02:13:24Z

It looks not an issue of missing source files. I can get the original commit passed locally. The original file list contains the minimal functions used by shadowsocks-libev, which should work.

Maybe there is something wrong with the ccache.

wongsyrone · 2017-11-28T02:14:45Z

minimal functions

It's static library, so doesn't matter to include all features

madeye · 2017-11-28T02:19:55Z

But it still takes more time to build.

wongsyrone · 2017-11-28T02:22:29Z

I'm using $SBTPATH/sbt go-clean clean clean-files go-build android:package-release for travis, sometimes use it to release apk to github, never have this kind of issue.

You can revert sodium changes and try this on travis again.

wongsyrone · 2017-11-28T02:33:50Z

I can get the original commit passed locally

You'd better check submodule status and see where the detached HEAD locates and clear untracked files in submodule.

Mygod · 2017-11-28T04:46:56Z

Well it seems like force push is a bad idea. We had the option to squash the commits while merging. :)

The original file list contains the minimal functions used by shadowsocks-libev, which should work.

Yes. It took me quite a while to tune the minimal file set. C/C++ sucks. :/

wongsyrone · 2017-11-28T23:17:28Z

Just took from https://github.com/jedisct1/libsodium/blob/master/src/libsodium/Makefile.am

If you really want official minimal build: check if !MINIMAL in that file.

Mygod · 2017-12-01T06:13:42Z

@wongsyrone The minimal refers to the minimal files required to make all the cryptos in shadowsocks work instead of the official minimal build. Fixed via ae83321.

wongsyrone · 2017-12-01T08:30:45Z

https://github.com/jedisct1/libsodium/blob/b45d52a8cf430bc7d14dd160555d87a7161038df/src/libsodium/sodium/core.c#L33

Just placebo, It doesn't fix anything and improves anything IMHO. It makes no sense at all.

takes more time to build

Any benchmark on this? any noticeable improvements?

Mygod · 2017-12-05T03:40:14Z

@wongsyrone Probably not. Just think of it as an OCD.

minor improvements starting from NDK r16 (shadowsocks#1458)

Mygod approved these changes Nov 28, 2017

View reviewed changes

minor improvements starting from NDK r16 (#1458)

4168bd6

Signed-off-by: Syrone Wong <wong.syrone@gmail.com>

madeye merged commit d4c9e25 into shadowsocks:master Nov 28, 2017

wongsyrone deleted the improve branch November 28, 2017 02:06

FakeTrader pushed a commit to FakeTrader/shadowsocks-android that referenced this pull request Aug 21, 2018

Merge pull request shadowsocks#1466 from wongsyrone/improve

448f10f

minor improvements starting from NDK r16 (shadowsocks#1458)

bannedbook pushed a commit to bannedbook/SpeedUp.VPN that referenced this pull request Dec 25, 2019

Merge pull request shadowsocks#1466 from wongsyrone/improve

66298b4

minor improvements starting from NDK r16 (shadowsocks#1458)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

minor improvements starting from NDK r16 (#1458) #1466

minor improvements starting from NDK r16 (#1458) #1466

wongsyrone commented Nov 28, 2017

Mygod left a comment

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017 •

edited

Loading

madeye commented Nov 28, 2017

wongsyrone commented Nov 28, 2017 •

edited

Loading

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

madeye commented Nov 28, 2017

madeye commented Nov 28, 2017 •

edited

Loading

wongsyrone commented Nov 28, 2017

madeye commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Dec 1, 2017

wongsyrone commented Dec 1, 2017

Mygod commented Dec 5, 2017

minor improvements starting from NDK r16 (#1458) #1466

minor improvements starting from NDK r16 (#1458) #1466

Conversation

wongsyrone commented Nov 28, 2017

Type of changes

Details

Mygod left a comment

Choose a reason for hiding this comment

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017 • edited Loading

madeye commented Nov 28, 2017

wongsyrone commented Nov 28, 2017 • edited Loading

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

madeye commented Nov 28, 2017

madeye commented Nov 28, 2017 • edited Loading

wongsyrone commented Nov 28, 2017

madeye commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Nov 28, 2017

wongsyrone commented Nov 28, 2017

Mygod commented Dec 1, 2017

wongsyrone commented Dec 1, 2017

Mygod commented Dec 5, 2017

wongsyrone commented Nov 28, 2017 •

edited

Loading

wongsyrone commented Nov 28, 2017 •

edited

Loading

madeye commented Nov 28, 2017 •

edited

Loading