| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| { | ||
| "referenceCategory": "SECURITY", | ||
| "referenceLocator": "cpe:2.3:a:zlib:zlib:1.3.1:*:*:*:*:*:*:*", | ||
| "referenceLocator": "cpe:2.3:a:zlib-ng:zlib-ng:2.2.4:*:*:*:*:*:*:*", |
There was a problem hiding this comment.
I'll admit I just guessed this, in order to unblock my testing (couldn't build at all with an invalid SBOM). If it's not right, let me know
Sorry, something went wrong.
There was a problem hiding this comment.
This is likely the CPE that would get used, although we can't be for certain until a CVE exists with it... the only one I could find with some searching is cpe:...:zlib-ng:minizip-ng which is for a different component but at least the organization is a match.
Sorry, something went wrong.
| return -1; | ||
| } | ||
| #ifdef ZLIBNG_VERSION | ||
| if (PyModule_Add(mod, "ZLIBNG_VERSION", |
There was a problem hiding this comment.
Figured we'd want some way to detect this other than looking at the text in ZLIB_VERSION
Sorry, something went wrong.
There was a problem hiding this comment.
makes sense. annoying to have optional attributes that one would use hasattr or getattr on instead of blindly accessing, but realistically nobody should care as this is more internal informational so this is fine.
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, the main place it'll be used is in pythoninfo, which already handles absent attributes.
We have more offensive optional attributes in our extension modules 😆
Sorry, something went wrong.
|
I've reviewed the three header files added to PC and they seem generic. It's a shame they aren't part of the sources, but figured it's easier/safer to just copy them into our source tree so that the cpython-source-deps repo is a straight import. |
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @zooba for commit 08eecb1 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131438%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Sorry, something went wrong.
|
The two failing buildbots appear unrelated, and everything else passed. All I changed in the last commit was docs, so I'm not going to rerun the buildbots unless someone thinks it's necessary or worthwhile. |
Sorry, something went wrong.
There was a problem hiding this comment.
SBOM changes LGTM!
Sorry, something went wrong.
| { | ||
| "referenceCategory": "SECURITY", | ||
| "referenceLocator": "cpe:2.3:a:zlib:zlib:1.3.1:*:*:*:*:*:*:*", | ||
| "referenceLocator": "cpe:2.3:a:zlib-ng:zlib-ng:2.2.4:*:*:*:*:*:*:*", |
There was a problem hiding this comment.
This is likely the CPE that would get used, although we can't be for certain until a CVE exists with it... the only one I could find with some searching is cpe:...:zlib-ng:minizip-ng which is for a different component but at least the organization is a match.
Sorry, something went wrong.
|
I'm kicking off a pyperformance benchmarking run with this -- we have a non-zero usage of zlib in that suite. |
Sorry, something went wrong.
|
The results for this on pyperformance are basically within the noise / no change. Not all that surprising, given that there are no benchmarks specifically aimed at zlib. https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20250319-3.14.0a6+-548daa7 |
Sorry, something went wrong.
| return -1; | ||
| } | ||
| #ifdef ZLIBNG_VERSION | ||
| if (PyModule_Add(mod, "ZLIBNG_VERSION", |
There was a problem hiding this comment.
makes sense. annoying to have optional attributes that one would use hasattr or getattr on instead of blindly accessing, but realistically nobody should care as this is more internal informational so this is fine.
Sorry, something went wrong.
|
This has broken the tailcall builds on Windows: https://github.com/python/cpython/actions/runs/13967899073/job/39102464260 D:\a\cpython\cpython\externals\\zlib-ng-2.2.4\\functable.c(79,26): error : use of undeclared identifier 'slide_hash_sse2'; did you mean 'slide_hash_c'? [D:\a\cpython\cpython\PCbuild\zlib-ng.vcxproj] D:\a\cpython\cpython\externals\\zlib-ng-2.2.4\\functable.c(123,26): error : use of undeclared identifier 'slide_hash_avx2'; did you mean 'slide_hash_c'? [D:\a\cpython\cpython\PCbuild\zlib-ng.vcxproj] |
Sorry, something went wrong.
|
Probably needs some fixed preprocessor checks (hopefully only in what's in the PC directory) to better handle the intrinsics being present/absent. Though I notice that build warns NOTE: Visual Studio not detected. LLVM does not provide a C/C++ standard library and may be unable to locate MSVC headers., which might also be relevant. |
Sorry, something went wrong.
|
I think, this is because here Then, in Sorry, I cannot get the permalinks to work here the usual way I am doing it :( |
Sorry, something went wrong.
FWIW, when building with the bundled clang-cl of VS I do not have these warnings, but get the same error. |
Sorry, something went wrong.
|
Most likely zlib-ng would generate different versions of the header files I checked into the PC directory for Clang. Figuring out what differences belong in there and detecting it dynamically rather than using CMake is the way forward. This should get a new issue (because clang-cl isn't a supported configuration, so we don't have to revert this change until it's working). Tag me on it and we can continue there. |
Sorry, something went wrong.
| <PrecompiledHeader>NotUsing</PrecompiledHeader> | ||
| <AdditionalIncludeDirectories>$(zlibNgDir);$(PySourceDir)PC;$(GeneratedZlibNgDir);%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> | ||
| <PreprocessorDefinitions>%(PreprocessorDefinitions);ZLIB_COMPAT;WITH_GZFILEOP;NO_FSEEKO;HAVE_BUILTIN_ASSUME_ALIGNED;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;</PreprocessorDefinitions> | ||
| <PreprocessorDefinitions Condition="$(Platform) == 'Win32' or $(Platform) == 'x64'">%(PreprocessorDefinitions);X86_FEATURES;X86_HAVE_XSAVE_INTRIN;X86_SSE2;X86_SSSE3;X86_SSE42;X86_PCLMULQDQ_CRC;X86_AVX2;X86_AVX512;X86_AVX512VNNI;X86_VPCLMULQDQ_CRC</PreprocessorDefinitions> |
There was a problem hiding this comment.
Does this mean that for all executables X86_AVX512 code will be emitted and used?
See e.g. adler32_avx512.c, where the whole file is guarded with X86_AVX512 and there is code like __m512i in it.
Sorry, something went wrong.
There was a problem hiding this comment.
I believe it's detected and called dynamically based on availability, but it's all compiled at compile time.
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, I hope so, too.
Sorry, something went wrong.
| <PreprocessorDefinitions>%(PreprocessorDefinitions);ZLIB_COMPAT;WITH_GZFILEOP;NO_FSEEKO;HAVE_BUILTIN_ASSUME_ALIGNED;_CRT_SECURE_NO_DEPRECATE;_CRT_NONSTDC_NO_DEPRECATE;</PreprocessorDefinitions> | ||
| <PreprocessorDefinitions Condition="$(Platform) == 'Win32' or $(Platform) == 'x64'">%(PreprocessorDefinitions);X86_FEATURES;X86_HAVE_XSAVE_INTRIN;X86_SSE2;X86_SSSE3;X86_SSE42;X86_PCLMULQDQ_CRC;X86_AVX2;X86_AVX512;X86_AVX512VNNI;X86_VPCLMULQDQ_CRC</PreprocessorDefinitions> | ||
| <PreprocessorDefinitions Condition="$(Configuration) == 'Debug'">%(PreprocessorDefinitions);ZLIB_DEBUG</PreprocessorDefinitions> | ||
| <EnableEnhancedInstructionSet Condition="$(Platform) == 'Win32' or $(Platform) == 'x64'">AdvancedVectorExtensions2</EnableEnhancedInstructionSet> |
There was a problem hiding this comment.
Unconditionally letting the compiler generate code up to AVX2 for all *.c files here might result in the binary not running on all CPUs?
Sorry, something went wrong.
The big changes here are: - Switching to zlib-ng on Windows (python/cpython#131438) - Using hmac for hashing functions (python/cpython#130157) --------- Co-authored-by: Geoffrey Thomas <geofft@ldpreload.com>
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.