Support MSVC 2022 (version 17.3) #1136
Conversation
MSVC (17.3 or earlier) reports the following warning lz4\lib\lz4.c(527): warning C6385: Reading invalid data from 'v'. Line 527 is : LZ4_memcpy(&v[4], v, 4); But, obviously v[0..3] is always filled with meaningful value. Therefore, this warning report is wrong. We must revisit this issue with future version of MSVC.
Since rc.exe (the resource compiler) is legacy compiler, it truncates preprocessor symbol name length to 32 chars. And it reports the following warning lz4\build\VS2022\..\..\lib\lz4.h(314): warning RC4011: identifier truncated to 'LZ4_STATIC_LINKING_ONLY_DISABLE' lz4\build\VS2022\..\..\lib\lz4.h(401): warning RC4011: identifier truncated to 'LZ4_STATIC_LINKING_ONLY_DISABLE' This patch detects rc.exe and just skips long symbol.
MSVC 2022 reports the follwing false positve warnings: lz4\tests\datagencli.c(110): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2). lz4\tests\datagencli.c(134): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2). lz4\tests\datagencli.c(146): warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2). Although they're absolutely compiler's and static analyzer's bug, it'd always be nice to use the standard library.
Suppress the following false positive warnings from MSVC: - Disable all arithmetic overflow (C26451) - Suppress C6385: Reading invalid data from 'compressedBuffer'. - Add ULL suffix to unsigned 64-bits constants.
This patch fixes the following error from "make staticAnalyze"
datagencli.c:106:21: warning: Value stored to 'size' is never read
size=0;
^ ~
|
Ugh, I don't understand why AppVeyor reports the following error https://ci.appveyor.com/project/YannCollet/lz4-1lndh/builds/44453956 But f7e9593 removes this line 😕 |
|
This topic raises a larger question about maintenance of Visual Studio projects. One issue with having multiple supported projects for multiple versions of Visual Studio is maintenance workload. As a consequence, I would prefer a reduction in support maintenance of build systems. I'm not yet sure if this is a sensible plan. But the more important objective is : it should be possible to add a source file into the |
|
I'm not sure about older version of MSVC, but recent versions of VS/MSVC is supporting https://docs.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio So it seems good time to introduce Since lz4/build/cmake/CMakeLists.txt is not identical to lz4/build/VS*, I think we should create new CMakeLists.txt for MSVC firstly and maintain it for while. I'd like to avoid including new cmake build script for MSVC in the next release because it may require some time to stabilize. |
|
Agreed |
| @@ -311,10 +311,12 @@ LZ4LIB_API int LZ4_decompress_safe_partial (const char* src, char* dst, int srcS | |||
| ***********************************************/ | |||
| typedef union LZ4_stream_u LZ4_stream_t; /* incomplete type (defined later) */ | |||
|
|
|||
| #if !defined(RC_INVOKED) /* https://docs.microsoft.com/en-us/windows/win32/menurc/predefined-macros */ | |||
There was a problem hiding this comment.
I don't understand the relation between RC_INVOKED and following declarations.
Could you explain ?
There was a problem hiding this comment.
(1) Truncation causes the warning "RC4011: identifier truncated"
rc.exe reports the following warnings
lz4\build\VS2022\..\..\lib\lz4.h(314): warning RC4011: identifier truncated to 'LZ4_STATIC_LINKING_ONLY_DISABLE'
lz4\build\VS2022\..\..\lib\lz4.h(401): warning RC4011: identifier truncated to 'LZ4_STATIC_LINKING_ONLY_DISABLE'
The reason of the warning is trunction. Since rc is a legacy resource compiler, it truncates long symbols.
(2) rc.exe predefines RC_INVOKED
https://docs.microsoft.com/en-us/windows/win32/menurc/predefined-macros
rc.exe predefines RC_INVOKED. And we can use it to determine that header is being processed by rc.exe or not.
(3) Use RC_INVOKED to avoid truncation warnings.
The following #if !defined() block means "skip this block when rc.exe is trying to read this block."
#if !defined(RC_INVOKED)
...
#endif
I surround LZ4_STATIC_LINKING_ONLY_DISABLE with this block to avoid the warning.
Other possible solution may be surrounding entire header except version number with #if !defined(RC_INVOKED).
There was a problem hiding this comment.
It's a good explanation.
I believe it does not transpire from the provided link only.
Could you add a word about the reason of this presence in code documentation ?
This PR adds MSVC 2022 project file and minor fixes for false positive warnings.