ASM zlib build on Windows gives erroneous results #41
Comments
|
I found a similar issue with the ASM optimized builds on Windows x86 and x64 while debugging a Subversion issue. A test program shows quite different behavior depending on how much data is feeded into the inflate() function. |
|
Therefore, TortoiseSVN disabled using ASM 2 days ago. |
The real issue is a bug in asm zlib build, reported here madler/zlib#41 . Non ASM builds behave more predictable.
madler/zlib#41 http://svn.haxx.se/dev/archive-2013-08/0386.shtml git-svn-id: http://tortoisesvn.googlecode.com/svn/trunk@24920 4cfb86a4-2c7a-11df-bd14-c1e331e6ceac
* Patch from Sergey Azarkevich: Pass the peg revision to the revert command. (Fixes issue #584) : Revert from log dialog bottom pane fails if the file was deleted * Do not use assembly optimized zlib build since it's buggy: madler/zlib#41 http://svn.haxx.se/dev/archive-2013-08/0386.shtml
* Patch from Sergey Azarkevich: Pass the peg revision to the revert command. (Fixes issue #584) : Revert from log dialog bottom pane fails if the file was deleted * Do not use assembly optimized zlib build since it's buggy: madler/zlib#41 http://svn.haxx.se/dev/archive-2013-08/0386.shtml
|
What assembly code is being used? There are a few in zlib's contrib directory. By the way, the stuff in the contrib directory is not part of zlib. It is just there as a convenience and is supported (or not) by those third-party contributors. What I will do is simply remove the offending code from the next distribution. |
* Patch from Sergey Azarkevich: Pass the peg revision to the revert command. (Fixes issue #584) : Revert from log dialog bottom pane fails if the file was deleted * Do not use assembly optimized zlib build since it's buggy: madler/zlib#41 http://svn.haxx.se/dev/archive-2013-08/0386.shtml
|
Due to bugs and other issues, the assembler code in the contrib directory has been removed. |
|
To mention were also, that we stopped to use any ASM implementations in PHP back then. C versions do no issues for years already, and combined with PGO there's no impact on performance. If those ASM files aren't supported anymore, it is right' that they shouldn't show up in the distribution. Thanks. |
That's probably because the asm isn't tuned very well for modern x86 (http://agner.org/optimize/). I had a look at The
|
|
@madler: Is zlib interested in "officially" including any new asm? Or at least in an optional but easy-to-use way that some Linux distro packages might enable? And what about runtime dispatching to use BMI1/BMI2 instructions, and/or AVX instructions? Intel CPUs run plain variable-count shifts as 3 uops (because of x86 legacy baggage with FLAGS semantics in case the count is zero), and there's a lot of bit shifting, so Compilers know how to use BMI1/BMI2 instructions when compiling ordinary C, for example |
* Apply suggestions from code review * make the copy constructors explicitly defaulted * split compound assert * Smd loads a single animation file Can't load without mesh before * Fix smd animation mess aiMatrix4x4t<TReal>::FromEulerAnglesXYZ modified to row order * Issue 1117: Smd load multiple animations Read an animation list from a file * Fix: Smd Cannot read bone names containing spaces * Issue 1639: Smd mesh vertex bone assignment bone.mOffsetMatrix not set when bone.iParent == -1 * Update SMDLoader.cpp Fix compiler warning and do some reformattings. * closes assimp#2228: prepare pull-request. * Fix CMake target alias typo * Off-Importer: introduce unittest. * closes assimp#2247: change include folder from debian package from /usr/lib/include to /usr/include * collada export: Use Camera local coordinate system Fixes assimp#2255 * Use ADD_ASSIMP_IMPORTER for STEP importer * Fix failed assimp validation for glTF2 sample animations * Add check for NULL texcoord values. * closes assimp#817: use emmisive factor instead of color. * Add minimal unit-test for LWO file format * Add unit-test for STL format auto detection * Add formatDetection unit-test for 3DS importer * Add silent flag to assimp cmd Add a new command line flag to assimp cmd in order to only display minimal info when loading a file. This is especially useful for quick profiling/fuzzing. * Fix animations import in gltf2 * added arm64e to iOS build script added arm64e iOS cmake file * added bitcode ios * Update Info.cpp Add a simple check to avoid verbose + silent mode together, * Update Readme.md Add issue solution time-tracking. * Update Readme.md Add codacy badge. * Update .travis.yml make coverity scan looking for master. * JAssimp: fix simple code analysis issues. * Fix next finding. * Fix next finding. * Review: fix wrong interpretation of methods for JNI. * Update Jassimp.java Fix another misinterpretation from the JNI-interface. * python: fix review findings. * Testing: improve test coverage: * Update .travis.yml revert name of branch * Coverity-findings * more fixes for coverity-findings. * remove dead code. * fix more findings. * Add a check for the resule of a dynamic cast. * eremove useless assertion test. * Fix: except `SyntaxError` for py3 viewer Following https://www.python.org/dev/peps/pep-3110/ * Fix unnecessary allocation. * Fix memory leak. * remove unique_ptr ina local function / method. * fix streamreader::end definition for iterators. * fix review findings. * fix review findings. * fix out-of-bound access. * fix review warning: wrapper object used after free. * fix review finding: Wrapper object use after free. * fix review finding: fix resource leak. * next try. * fix finding: possible override. * fix review finding: dereference after null-check. * Correction on clipper crash on some IFC files when points are empty * Various additions/fixes (FBX blend-shapes support added) Added animMesh name assignment at ColladaLoader Fixed animMesh post-processing on ConvertToLhProcess (blend-shapes weren't being affected by post-processing) Added WindowsStore define. This is used to change some incompatible WinRT methods Added FBX blend-shapes and blend-shapes animations support Added Maya FBX specific texture slots parsing Added extra FBX metadata parsing Added GLTF2 vertex color parsing Fixed IFC-Loader zip-buffer reading rountine Fixed OBJ file parsing line-breaker bug Fixed IOStreamBuffer cache over-read bug Added mName field to aiAnimMesh Reverted EmissiveFactor, TransparencyFactor and SpecularFactor assignment on FBXConverter. Really, the commit assimp#817 breaks a lot of old code. * Test fixes Test fixes * Change access of getData() to package-private for access in JaiDebug. Move private field to fix compile error. * Add ProgressHandler support. * Make this public for JaiDebug and to pass quality checks. * Fix progress reporting in ObjFileParser. Remove old unused code which is claiming to still take up "1/3" of the total progress. * add FBX Line Element support add FBX Line Element support * Update FBXConverter.cpp * Preserve all the material parameters from FBX models * Disable step. * Adapt MemoryIOSystem to delegate unhandled calls to shadowed IO system * Fill in rest of interface; switch created_stream to a unique_ptr * Update MemoryIOWrapper.h Fix leak. * Update MemoryIOWrapper.h Make code more readable. * Update MemoryIOWrapper.h Make code more readable. * Closes assimp#2251: introduce AI_CONFIG_PP_FID_IGNORE_TEXTURECOORDS to avoid removing textures. * Update README.md Update doc. * Update FindInvalidDataProcess.cpp -Fix some review findings. * Fix glTF2 export with no texture coordinates * Update copyrights. * Fix review findings. * [pyassimp] Bumped to 4.1.4 Main changes: - Support for metadata fields (Vincent Fazio, Wojciech Matyjewicz) - added support for aiExportSceneToBlob (Vincent Fazio) - a few bug fix + code beautification * Some more findings. * closes assimp#2297: introduce obj-unittest to validate working importer. * Merged PR 2811682: buffer grow changes and large files support Buffer grow changes: The exporting of relatevely large data could take a few days, because reallocation of a buffer every time for a few new bytes is overkill. I've introduced standard capacity member for the buffer and growth it by 1.5 times every time. That helps a lot, descrease exporting to a minute (from a few days). Large file support: glTF is a json file, all lengths and offsets don't have a type, just numbers, but code was always reading it as uint32, this doesn't work for files bigger than int32 (almost all files we have as an example). So, I've changed it to be reading as size_t. Specification doesn't specify the type for it, so it's not against spec. * warning fix for gcc * fix openfiledialog in modelviewer. * remove dead code * Update Readme.md closes assimp#2248: Add lgtm badge. * Fix potential security issues. * Add tokenization of 'c' type arrays, both compressed and uncompressed. * Remove unneeded newline. * Add two test files which require 'c' handling to be tokenized correctly. * cast size_t to unsigned int * Devil: replace this by header-only lib. * Update README - Update the readme for the exampes. * [Issue_2340] Added ASSIMP_ANDROID_JNIIOSYSTEM precheck to only remain set to ON in proper ANDROID enabled toolchain environment * [Issue_2340] Wrong precondition layout... missed ANDROID=ON and ASSIMP_ANDROID_JNIIOSYSTEM=OFFvalid constellation * [Issue_2340] Proper closing endif()... * closes assimp#305': fix viewer * Add test file. * Update appveyor.yml Disable vs2015-test * Update appveyor.yml Disable vs2017 * closes assimp#2115: rollback setup of FBX-camera. * closes assimp#1593: fix computation of percentf for 3DS. * introduce simple skin-test +some findings. * fix compiler warnings. * ignoring invalid normals and uvs indices instead of canceling the import completely * In ColladaLoader in version 3.2.0 (release est. 2015) the name assignment was based on name attribute (xml) with optional fallbacks to id and sid. Over time this was changed to fix a bug and support for names was removed, only to be later (re)added using an optional `useColladaName` setting. We discovered a problem that with when using the name based assignment it doesn't assign automatically generated names like what it did in the logic in v3.2.0 and on the id based approach on current master. We discovered this causes problems with matching the aiCamera and aiLight. So we rectified this problem by auto generating names also on the name based method. * Ensure our include directories get added in the correct order If you have assimp installed already and in the include path (e.g. I have it via homebrew), it can pick up the wrong headers. This forces the include order so our local ones are found first when building assimp. * Add aiNode::mName to ValidateDataStructure error reporting to ease debugging * Make syntax used to call ReportError & ReportWarning a bit more consistent * Typo Fix a typo in a comment. * ValidateDataStructure.cpp: * Fixed warnings introduced by last commit (hopefully) * Fixed case fallthrough (due to exception flow, it didn't make a practical difference, but hopefully will remove a warning) * Minor formatting consistency improvements * Initial pass * Hopefully fix all warnings? * remove 1 more of the warnings * remove last warning? * Reenable vs2015 and vs1017 * Update appveyor.yml Fix build image. * Update appveyor.yml Fix cmake generation. * Fix compiler warnings. * Fix compiler warnings: var declarations hides other var. * closes assimp#934: introduce material keys for shader types. * add missing changes. * Add short documentation. * fix: change ScaleProcess priority * Fix some coverity findings. * fix loading 3D uvs from obj * Update MD3Loader.cpp Fix typo. * Adds a way to select which exporters you want to compile Mimics the ASSIMP_BUILD_ALL_IMPORTERS_BY_DEFAULT / ASSIMP_BUILD_XXX_IMPORTER code but for exporters. This works exactly the same way with one exception - ASSIMP_NO_EXPORT still overrides everything and turns off all exporting. Fixes assimp#2377 * Fix 3MF importer * INSTALL: Out of source build and improve Windows * INSTALL: update bullet point numbers * Update Readme.md Add the HAXE-port. * {cmake} Explicitly turn off ASM686 and AMD64 cmake options when ASSIMP_BUILD_ZLIB is on The AMD64 option causes a build failure on MSVC (assimp#1760) and the ASM builds seem to have problems: madler/zlib#41 (comment) This change also prevents these from "polluting" the cmake options if assimp is being included as a submodule.
* Patch from Sergey Azarkevich: Pass the peg revision to the revert command. (Fixes issue #584) : Revert from log dialog bottom pane fails if the file was deleted * Do not use assembly optimized zlib build since it's buggy: madler/zlib#41 http://svn.haxx.se/dev/archive-2013-08/0386.shtml
…cy with the ASM code
Make the note about the use of "deflate" to refer to the "zlib" format more prominent by rendering it as a note. Also regenerate index.html, which has the side-effect of adding MDN annotations.
and others
The bug happens using an ASM optimized static zlib 1.2.5 or 1.2.7 on Windows x86. Supposed is to have no output, but currently it outs "%Cë". Non ASM versions on Windows do that right, as well both ASM and non ASM builds on Linux. The bad data can be fetched from http://188.40.74.4/corrupted.gz .
Using the snippet and data from
https://gist.github.com/anonymous/2c5a88ca9ac6f4c2a064
the erroneous behavior is reproducible.
It is possible, that the same issue does exist on Darwin, but sadly I've no such OS to test. This bug is originally detected in PHP https://bugs.php.net/bug.php?id=61677 .
The text was updated successfully, but these errors were encountered: