Use built-ins for addition and subtraction on Windows #17472
Conversation
For Clang, we just need to define the respective macros, since these built-ins are available in all supported Clang versions (>= 4.0.0, currently)[1]. For MSVC (and possibly other compilers) we use the respective APIs of intsafe.h[2] which are available as of Windows 7/Server 2008 R2. This avoids the UB due to signed integer overflow that may happen with our fallback implementations. [1] <https://releases.llvm.org/4.0.0/tools/clang/docs/LanguageExtensions.html> [2] <https://learn.microsoft.com/en-us/windows/win32/api/intsafe/>
This shouldn't be defined unconditionally, but since it is apparently unused, we remove it altogether.
|
Need to check the test failures. |
|
I'll follow up with a similar improvement for |
If this is not a problem, I don't see other problems.
@nielsdos could you please take a look. This is probably may be fixed similar to your fix for the IR. |
No problem. We require Windows 7/2008 RC2 as of PHP 7.2.0 already (and Windows 8/Server 2012 as of PHP 8.3.0).
Background: I've noticed the UB when running the Zend/test suite with Clang UBSan on Windows. At first I was very surprised that this hasn't come up already, but when looking at the code, I saw that there is already special casing for the most important compilers/systems (using either inline ASM or _builtin*()). So it might not be that important to fix the fallbacks. |
This was asked to be checked in php#17472 (comment) There are 3 issues: 1) The constant LONG_SIGN_MASK is wrong. Commit 98df5c9 changed this to avoid UB but did this incorrectly. The right fix was using zend_ulong. 2) The UB in the if can overflow, and can be fixed by using zend_ulong for the sum/sub. 3) fast_long_sub_function() has a problem when result aliases. This is fixed in the same way as fast_long_add_function() works.
This was asked to be checked in #17472 (comment) There are 2 issues: 1) The UB in the if can overflow, and can be fixed by using zend_ulong for the sum/sub. 2) fast_long_sub_function() has a problem when result aliases. This is fixed in the same way as fast_long_add_function() works. Closes GH-17666.
This was asked to be checked in php#17472 (comment) There are 2 issues: 1) The UB in the if can overflow, and can be fixed by using zend_ulong for the sum/sub. 2) fast_long_sub_function() has a problem when result aliases. This is fixed in the same way as fast_long_add_function() works. Closes phpGH-17666.
For Clang, we just need to define the respective macros, since these built-ins are available in all supported Clang versions (>= 4.0.0, currently)[1].
For MSVC (and possibly other compilers) we use the respective APIs of intsafe.h[2] which are available as of Windows 7/Server 2008 R2.
This avoids the UB due to signed integer overflow that may happen with our fallback implementations.
[1] https://releases.llvm.org/4.0.0/tools/clang/docs/LanguageExtensions.html
[2] https://learn.microsoft.com/en-us/windows/win32/api/intsafe/