Archived issue #0027194
Possible division by zero in IntPatch_WLineTool
Description
This issue is observed in scope of the #0026329 bug.
aStepCoeff = Min(aStepOnS1, aStepOnS2) / Max(aStepOnS1, aStepOnS2);
Such division may lead to the problems when aStepOnS1 and aStepOnS2 are equal to 0.0.
It is necessary to investigate how this situation may happen and elaborate solution. Problem can be reproduced on the CR26329 branch only.
aStepCoeff = Min(aStepOnS1, aStepOnS2) / Max(aStepOnS1, aStepOnS2);
Such division may lead to the problems when aStepOnS1 and aStepOnS2 are equal to 0.0.
It is necessary to investigate how this situation may happen and elaborate solution. Problem can be reproduced on the CR26329 branch only.
Steps to reproduce
test bugs moddata_2 bug469
Public activity
17 archived notes
Participants are labeled by their role within this record.
Branch [archived branch] has been created by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Sat Feb 20 11:54:44 2016 +0300
0027194: Possible division by zero in IntPatch_WLineTool
Correct handling for division by zero is added. This prevents exception when FPE is enabled
[revision removed]
Detailed log of new commits:
Author: Author
Date: Sat Feb 20 11:54:44 2016 +0300
0027194: Possible division by zero in IntPatch_WLineTool
Correct handling for division by zero is added. This prevents exception when FPE is enabled
Dear msv,
Please check current state of the branch CR27194.
Please check current state of the branch CR27194.
Reviewed.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Dear Commenter 1,
Branch CR27194 was rebased on current master of occt git-repository.
[revision removed]
Branch CR27194 was rebased on current master of occt git-repository.
[revision removed]
Dear Commenter 1,
Branch CR27194 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
Number of compiler warnings:
occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)
products component :
Linux: 41 (41 on master)
Windows: 0 (0 on master)
Regressions/Differences/Improvements:
No regressions/differences
Testing cases:
Not needed
Testing on Linux:
occt component :
Total MEMORY difference: 90157515 / 90534753 [-0.42%]
Total CPU difference: 19392.780000000017 / 19576.420000000107 [-0.94%]
products component :
Total MEMORY difference: 25923801 / 25872807 [+0.20%]
Total CPU difference: 5361.569999999991 / 5316.139999999985 [+0.85%]
Testing on Windows:
occt component :
Total MEMORY difference: 57375213 / 57106337 [+0.47%]
Total CPU difference: 18744.31575509885 / 17710.10712559875 [+5.84%]
products component :
Total MEMORY difference: 17493005 / 17395600 [+0.56%]
Total CPU difference: 5411.159886699964 / 5095.257861699954 [+6.20%]
There are no differences in images found by testdiff.
Branch CR27194 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
Number of compiler warnings:
occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)
products component :
Linux: 41 (41 on master)
Windows: 0 (0 on master)
Regressions/Differences/Improvements:
No regressions/differences
Testing cases:
Not needed
Testing on Linux:
occt component :
Total MEMORY difference: 90157515 / 90534753 [-0.42%]
Total CPU difference: 19392.780000000017 / 19576.420000000107 [-0.94%]
products component :
Total MEMORY difference: 25923801 / 25872807 [+0.20%]
Total CPU difference: 5361.569999999991 / 5316.139999999985 [+0.85%]
Testing on Windows:
occt component :
Total MEMORY difference: 57375213 / 57106337 [+0.47%]
Total CPU difference: 18744.31575509885 / 17710.10712559875 [+5.84%]
products component :
Total MEMORY difference: 17493005 / 17395600 [+0.56%]
Total CPU difference: 5411.159886699964 / 5095.257861699954 [+6.20%]
There are no differences in images found by testdiff.
Dear Commenter 1,
Branch CR27194 is TESTED.
Branch CR27194 is TESTED.
Remarks on proposed fix:
1. Please avoid comparing real numbers for equality using operator == or != -- this will not work in general case. For instance, AFAIK, -0 != +0
2. In order to protect against division by zero, it would be sufficient to check the value of Max(aStepOnS1, aStepOnS2), so why checking both aStepOnS1 and aStepOnS2 individually?
3. What happens if aStepOnS1=1e-308 and aStepOnS2 = 10? Should not the behavior be the same as when aStepOnS1=0. and aStepOnS2=1.? (currently it will be different).
1. Please avoid comparing real numbers for equality using operator == or != -- this will not work in general case. For instance, AFAIK, -0 != +0
2. In order to protect against division by zero, it would be sufficient to check the value of Max(aStepOnS1, aStepOnS2), so why checking both aStepOnS1 and aStepOnS2 individually?
3. What happens if aStepOnS1=1e-308 and aStepOnS2 = 10? Should not the behavior be the same as when aStepOnS1=0. and aStepOnS2=1.? (currently it will be different).
1. Please avoid comparing real numbers for equality using operator == or != -- this will not work in general case. For instance, AFAIK, -0 != +0
if (+0.0 == -0.0)
cout << "EQUAL";
Returns true in MSVC2010. Here only +0.0 is used due to values nature.
2. In order to protect against division by zero, it would be sufficient to check the value of Max(aStepOnS1, aStepOnS2), so why checking both aStepOnS1 and aStepOnS2 individually?
Max(aStepOnS1, aStepOnS2) is not computed directly.
3. What happens if aStepOnS1=1e-308 and aStepOnS2 = 10? Should not the behavior be the same as when aStepOnS1=0. and aStepOnS2=1.? (currently it will be different).
Denormal numbers will be used.
if (+0.0 == -0.0)
cout << "EQUAL";
Returns true in MSVC2010. Here only +0.0 is used due to values nature.
2. In order to protect against division by zero, it would be sufficient to check the value of Max(aStepOnS1, aStepOnS2), so why checking both aStepOnS1 and aStepOnS2 individually?
Max(aStepOnS1, aStepOnS2) is not computed directly.
3. What happens if aStepOnS1=1e-308 and aStepOnS2 = 10? Should not the behavior be the same as when aStepOnS1=0. and aStepOnS2=1.? (currently it will be different).
Denormal numbers will be used.
1. I agree. It is better to compare with gp::Resolution(), that is RealSmall().
2. Checking of Max only is not enough, we must check also Min.
3. If we check both values this will not happen.
2. Checking of Max only is not enough, we must check also Min.
3. If we check both values this will not happen.
Branch [archived branch] has been created by Author.
[revision removed]
Detailed log of new commits:
Author: Author
Date: Sat Feb 20 11:54:44 2016 +0300
0027194: Possible division by zero in IntPatch_WLineTool
Correct handling for division by zero is added. This prevents exception when FPE is enabled
[revision removed]
Detailed log of new commits:
Author: Author
Date: Sat Feb 20 11:54:44 2016 +0300
0027194: Possible division by zero in IntPatch_WLineTool
Correct handling for division by zero is added. This prevents exception when FPE is enabled
Dear abv,
Please check current state of the CR27194_2 branch.
Please check current state of the CR27194_2 branch.
No remarks, please test
Dear Commenter 1,
Branch CR27194_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
Number of compiler warnings:
occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)
products component :
Linux: 72 (72 on master)
Windows: 4 (4 on master)
MacOS : 1144
Regressions/Differences/Improvements:
No regressions/differences
Testing cases:
Not needed
Testing on Linux:
occt component :
Total MEMORY difference: 88283719 / 87385561 [+1.03%]
Total CPU difference: 19284.279999999915 / 19257.83 [+0.14%]
products component :
Total MEMORY difference: 27790493 / 27718261 [+0.26%]
Total CPU difference: 5231.579999999983 / 5200.899999999987 [+0.59%]
Testing on Windows:
occt component :
Total MEMORY difference: 55779341 / 55791306 [-0.02%]
Total CPU difference: 18062.716185898837 / 17814.222192998932 [+1.39%]
products component :
Total MEMORY difference: 18925839 / 18889957 [+0.19%]
Total CPU difference: 5175.707577399963 / 4958.179782999945 [+4.39%]
There are no differences in images found by testdiff.
Branch CR27194_2 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]
Number of compiler warnings:
occt component :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)
products component :
Linux: 72 (72 on master)
Windows: 4 (4 on master)
MacOS : 1144
Regressions/Differences/Improvements:
No regressions/differences
Testing cases:
Not needed
Testing on Linux:
occt component :
Total MEMORY difference: 88283719 / 87385561 [+1.03%]
Total CPU difference: 19284.279999999915 / 19257.83 [+0.14%]
products component :
Total MEMORY difference: 27790493 / 27718261 [+0.26%]
Total CPU difference: 5231.579999999983 / 5200.899999999987 [+0.59%]
Testing on Windows:
occt component :
Total MEMORY difference: 55779341 / 55791306 [-0.02%]
Total CPU difference: 18062.716185898837 / 17814.222192998932 [+1.39%]
products component :
Total MEMORY difference: 18925839 / 18889957 [+0.19%]
Total CPU difference: 5175.707577399963 / 4958.179782999945 [+4.39%]
There are no differences in images found by testdiff.
Dear Commenter 1,
Branch CR27194_2 is TESTED.
Branch CR27194_2 is TESTED.
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Related records