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

Upgrade SQLite #5302

Closed
whalley opened this issue Nov 2, 2022 · 15 comments
Closed

Upgrade SQLite #5302

whalley opened this issue Nov 2, 2022 · 15 comments
Assignees
Milestone

Comments

@whalley
Copy link
Member

whalley commented Nov 2, 2022

Should look to update SQLite to latest version.

See #1447
Can we update to SQLite >= 3.35.0? Would be nice to be able to use the 'ALTER TABLE ... DROP COLUMN ...' syntax which is not available prior to that version.

@whalley whalley added this to the v1.6.3 milestone Nov 2, 2022
@whalley whalley modified the milestones: v1.6.3, v1.6.2 Nov 17, 2022
@n-stein
Copy link
Contributor

n-stein commented Nov 18, 2022

PR #5348 updates to SQLite 3.39.3

image

whalley added a commit that referenced this issue Nov 19, 2022
fix(#5302): update wxsqlite3 submodule
@whalley
Copy link
Member Author

whalley commented Nov 19, 2022

Are we missing the encryption support?
https://github.com/utelle/wxsqlite3#important-notes

On building I get....

[ 42%] Built target Lua
[ 42%] Built target generate_grm_files
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:258830:34: error: '__builtin_ia32_aeskeygenassist128' needs target feature aes
        temp = _mm_extract_epi32(_mm_aeskeygenassist_si128(_mm_setr_epi32(0, temp, 0, 0), 0), 1);
                                 ^
/Library/Developer/CommandLineTools/usr/lib/clang/14.0.0/include/__wmmintrin_aes.h:136:13: note: expanded from macro '_mm_aeskeygenassist_si128'
  ((__m128i)__builtin_ia32_aeskeygenassist128((__v2di)(__m128i)(C), (int)(R)))
            ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:258835:34: error: '__builtin_ia32_aeskeygenassist128' needs target feature aes
        temp = _mm_extract_epi32(_mm_aeskeygenassist_si128(_mm_setr_epi32(0, temp, 0, 0), 0), 0);
                                 ^
/Library/Developer/CommandLineTools/usr/lib/clang/14.0.0/include/__wmmintrin_aes.h:136:13: note: expanded from macro '_mm_aeskeygenassist_si128'
  ((__m128i)__builtin_ia32_aeskeygenassist128((__v2di)(__m128i)(C), (int)(R)))
            ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:258889:26: error: always_inline function '_mm_aesimc_si128' requires target feature 'aes', but would be inlined into function 'aesGenKeyDecrypt' that is compiled without support for 'aes'
        keySchedule[j] = _mm_aesimc_si128(tempKeySchedule[j]);
                         ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:258938:18: error: always_inline function '_mm_aesenc_si128' requires target feature 'aes', but would be inlined into function 'aesEncryptCBC' that is compiled without support for 'aes'
      feedback = _mm_aesenc_si128(feedback, key[j]);
                 ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:258940:16: error: always_inline function '_mm_aesenclast_si128' requires target feature 'aes', but would be inlined into function 'aesEncryptCBC' that is compiled without support for 'aes'
    feedback = _mm_aesenclast_si128(feedback, key[j]);
               ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:258961:18: error: always_inline function '_mm_aesenc_si128' requires target feature 'aes', but would be inlined into function 'aesEncryptCBC' that is compiled without support for 'aes'
      feedback = _mm_aesenc_si128(feedback, key[j]);
                 ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:258963:16: error: always_inline function '_mm_aesenclast_si128' requires target feature 'aes', but would be inlined into function 'aesEncryptCBC' that is compiled without support for 'aes'
    feedback = _mm_aesenclast_si128(feedback, key[j]);
               ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:259010:14: error: always_inline function '_mm_aesdec_si128' requires target feature 'aes', but would be inlined into function 'aesDecryptCBC' that is compiled without support for 'aes'
      data = _mm_aesdec_si128(data, key[numberOfRounds - j]);
             ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:259012:12: error: always_inline function '_mm_aesdeclast_si128' requires target feature 'aes', but would be inlined into function 'aesDecryptCBC' that is compiled without support for 'aes'
    data = _mm_aesdeclast_si128(data, key[numberOfRounds - j]);
           ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:259025:14: error: always_inline function '_mm_aesdec_si128' requires target feature 'aes', but would be inlined into function 'aesDecryptCBC' that is compiled without support for 'aes'
      data = _mm_aesdec_si128(data, key[numberOfRounds - j]);
             ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:259027:12: error: always_inline function '_mm_aesdeclast_si128' requires target feature 'aes', but would be inlined into function 'aesDecryptCBC' that is compiled without support for 'aes'
    data = _mm_aesdeclast_si128(data, key[numberOfRounds - j]);
           ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:259051:14: error: always_inline function '_mm_aesdec_si128' requires target feature 'aes', but would be inlined into function 'aesDecryptCBC' that is compiled without support for 'aes'
      data = _mm_aesdec_si128(data, key[numberOfRounds - j]);
             ^
/Users/mark/gitrepos/moneymanagerex/3rd/wxsqlite3/src/sqlite3mc_amalgamation.c:259053:12: error: always_inline function '_mm_aesdeclast_si128' requires target feature 'aes', but would be inlined into function 'aesDecryptCBC' that is compiled without support for 'aes'
    data = _mm_aesdeclast_si128(data, key[numberOfRounds - j]);
           ^
13 errors generated.
make[2]: *** [3rd/CMakeFiles/SQLite3.dir/wxsqlite3/src/sqlite3mc_amalgamation.c.o] Error 1
make[1]: *** [3rd/CMakeFiles/SQLite3.dir/all] Error 2

@whalley
Copy link
Member Author

whalley commented Nov 19, 2022

@n-stein
Copy link
Contributor

n-stein commented Nov 19, 2022

Hmm. Not seeing this in Windows and the AppVeyor builds completed successfully. Might be a CMake issue?

@n-stein
Copy link
Contributor

n-stein commented Nov 19, 2022

Ah, perhaps this is why:

wxSQLite3 uses now the new encryption extension SQLite3 Multiple Ciphers. Precompiled binaries of the SQLite3 DLL and the SQLite3 shell for Windows are now provided by this new separate project.

Seems we need more work for Mac and Linux.

@grumpy235
Copy link

Ulrich has just committed wxSQLite3 4.9.1 based on the very latest SQLite release. Perhaps ugrade to this version or is that too bleeding edge ?

@whalley
Copy link
Member Author

whalley commented Nov 20, 2022

If I change the Mac build to build just for M1 ARM64 then it builds OK.
Seems the x64 on Mac and Linux are the issues.

@n-stein
Copy link
Contributor

n-stein commented Nov 20, 2022

@grumpy235

You did some work on upgrading wxSQLite3 a few years ago in #3275. Looks like you made more changes in CMakeLists.txt than I did for this update. I admit I'm not a CMake expert. Anything you can see that is missing here?

@whalley
Copy link
Member Author

whalley commented Nov 20, 2022

I can get it building by forcing
#define HAS_AES_HARDWARE AES_HARDWARE_NONE

in /3rd/wxsqlite3/src/sqlite3mc_amalgamation.c

Obviously not optimal, and seemingly no way of doing this without tweaking code.

@whalley
Copy link
Member Author

whalley commented Nov 21, 2022

I'm also not sure what happens if at compile time it sets this to AES_HARDWARE_NI and the code ultimately executes on a CPU that does not have the instruction set extension.

@grumpy235
Copy link

@n-stein
I was only getting it to work for my copy of Windows, a knife & fork job as I simply commented out the Linux bits. I'm afraid my CMake knowledge is probably less than yours as I struggle to get my head around the syntax probably a sign of age!
What about reaching out to Ulrich, he may not be a CMake fan but he may have some ideas on how to compile on Mac/Linux?

@whalley
Copy link
Member Author

whalley commented Nov 21, 2022

What about reaching out to Ulrich, he may not be a CMake fan but he may have some ideas on how to compile on Mac/Linux?

Yes, I will do that.

@utelle
Copy link

utelle commented Nov 21, 2022

@n-stein

Hmm. Not seeing this in Windows and the AppVeyor builds completed successfully. Might be a CMake issue?

No, just a compiler issue.

Ah, perhaps this is why:

wxSQLite3 uses now the new encryption extension SQLite3 Multiple Ciphers. Precompiled binaries of the SQLite3 DLL and the SQLite3 shell for Windows are now provided by this new separate project.

Seems we need more work for Mac and Linux.

More work? Not really. Just add the compile time options

-msse4.2 -maes

when compiling for x86/x86_64 platforms. That should solve the issue.

@whalley

I can get it building by forcing
#define HAS_AES_HARDWARE AES_HARDWARE_NONE

in /3rd/wxsqlite3/src/sqlite3mc_amalgamation.c

Obviously not optimal, and seemingly no way of doing this without tweaking code.

Disabling AES support when it is actually available is not such a good idea. AES hardware support should be enabled if possible to accelerate the code execution. There is no harm in enabling the AES hardware support, even if the executable later runs on a processor without AES hardware support - the code checks at runtime whether AES hardware support is actually available.

@grumpy235

Ulrich has just committed wxSQLite3 4.9.1 based on the very latest SQLite release. Perhaps ugrade to this version or is that too bleeding edge ?

There were only very minor changes to wxSQLite3 and SQLite3 Multiple Ciphers. The underlying SQLite library is thoroughly tested by the SQLite developer team. Therefore upgrading to wxSQLite3 4.9.1 (based on SQLite3 Multiple Ciphers 1.5.4 and SQLite 3.40.0) should not impose noteworthy risks.

If critical bugs are detected either in SQLite, SQLite3 Multiple Ciphers, or wxSQLite3, I usually provide new releases fixing them in a timely manner.

whalley added a commit to whalley/moneymanagerex that referenced this issue Nov 21, 2022
@whalley
Copy link
Member Author

whalley commented Nov 21, 2022

@utelle Thanks for the speedy and concise response. All good, now builds.

whalley added a commit that referenced this issue Nov 21, 2022
fix(#5302): Non-win build options
@n-stein
Copy link
Contributor

n-stein commented Nov 21, 2022

Based on Ulrich's comment I updated to 4.9.1. Shouldn't be any major difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants