View Issue Details

IDProjectCategoryView StatusLast Update
0030875CommunityOCCT:Foundation Classespublic2020-10-08 11:02
Reporterdrazmyslovich Assigned Toabv 
PrioritynormalSeverityminor 
Status closedResolutionno change required 
PlatformWindowsOSVC++ 2015 
Product Version7.4.0 
Summary0030875: Foundation Classes - BSplCLib crashes in case of infinite upper knot
DescriptionBSplCLib crashes in case of infinite upper knot
Steps To ReproduceN/A
TagsNo tags attached.
Test case number

Relationships

related to 0026308 closedbugmaster Community Segmentation fault in BSplCLib::LocateParameter 
related to 0025602 assignedskl Open CASCADE Data Exchange - It is important for IGES and STEP file format to add check, if wrong data are read from the file 

Activities

git

2019-08-07 15:37

administrator   ~0086068

Branch CR30875 has been created by drazmyslovich.

SHA-1: be00a0482ba0bb382f27dae7afee660cbe4219a5


Detailed log of new commits:

Author: drazmyslovich
Date: Wed Aug 7 14:35:20 2019 +0200

    0030875: BSplCLib - check if an upper know is infinite

drazmyslovich

2019-08-07 15:38

developer   ~0086069

The changes are submitted for review

kgv

2019-08-07 19:54

developer   ~0086076

Last edited: 2019-08-07 19:54

+template<typename T> bool myisfinite(T arg)
+{
+  return arg == arg && arg != std::numeric_limits<T>::infinity() && arg != -std::numeric_limits<T>::infinity();
+}

Looks like function actually checks IsNotInfinite(), isn't it?

Do you have some test data for reproducing the issue?
Maybe it contains also NaNs?

drazmyslovich

2019-08-07 20:29

developer   ~0086078

Dear kgv,

unfortunately, I have no test data for it.

Yes, NaNs are also considered in this case. So, it's basically std::isfinite, which is a part of C++11.

Regards,
Dima

kgv

2019-08-07 20:47

developer   ~0086079

Ah, I have misread the name "myisfinite"...

> So, it's basically std::isfinite, which is a part of C++11.
The better place for this method is IsFinite( or IsRealFinite) in Standard_Real.hxx.

abv

2019-08-12 13:49

manager   ~0086182

Hello Dmitry,

Can you please provide more details on why you need to work with such ill-formed BSplines, and whether you expect other algorithms to deal with them gracefully?

Was this infinite knot encoded for purpose or due to error?
If this is error, perhaps it is quite reasonable and expected that LocateParameter() triggers signal.

Do you expect that protecting single function LocateParameter will be sufficient for OCCT to deal safely with such b-splines?

Andrey

drazmyslovich

2019-08-15 14:11

developer   ~0086262

Hello Andrey,

I have a similar situation as in the related ticket 25602 - the illformed bspline comes from step file in my case. LocateParameter function was in my case a milestone, which enabled OCCT library to proceed working with the whole model instead of producing some useless output values. Also, I would like to point out, that considering of upper knot during epsilon calculation was introduced recently in the context of 0026308...

Regards,
Dima

abv

2019-09-25 23:56

manager   ~0087520

Hello Dmitry,

Sorry I do not see how infinite value of U knot could harm the operation that you have protected in your patch (calculation of Epsilon in BSplCLib::LocateParameter(), in BSplCLib.cxx line 279).

The code takes minimum of U parameter and last U knot, thus if the last U knot is infinite, U parameter shall be used normally.

I have tried to reproduce the problem in refined environment with the following code, using MSVC 2015 (as reported in the issue description):

  double U = sqrt(4.);
  double KUpper = get_infinite();
  const Standard_Real Eps = Epsilon(Min(Abs(KUpper), Abs(U)));
  std::cout << "KUpper = " << KUpper << ", Eps = " << Eps << std::endl;

I have tried several variants for the value returned by get_infinite() and all work as expected:

KUpper = inf, Eps = 4.4408920985006262e-16
KUpper = -inf, Eps = 4.4408920985006262e-16
KUpper = -nan(ind), Eps = 4.4408920985006262e-16

Indeed if I enable FPE signal (using OSD::SetSignal(true)), then exception is raised if get_infinite() returns NAN. However, this case does not match description of the issue, thus possible irrelevant. If you need your program to deal with NAN values without raising signals, just do not enable them on FPE. This is generally the recommended mode of work in most real-world environments since many existing standard components (OpenGL drivers, .NET runtime etc.) are known to generate FPEs and thus may crash without a good reason if you enable FPE signals.

Thus I believe there is no issue, thus nothing to fix. Please confirm if you agree with the above, or provide a reproducer if you still believe the problem exists.

Best Regards,
Andrey

abv

2019-09-26 00:57

manager   ~0087521

I experimented with NANs in VS 2015 and noticed one thing: when FPE signals are set, comparison of NAN against itself or other number by == does not raise FPE, but comparison using > or < does raise. This turns to be valid behavior of floating point in C/C++, see e.g. https://www.gnu.org/software/libc/manual/html_node/FP-Comparison-Functions.html#FP-Comparison-Functions

C++ 11 introduced dedicated functions for non-signaling comparisons, like isgreater (http://www.cplusplus.com/reference/cmath/isgreater/) which could be used to compare floats instead of < and > in functions like Min() and Max() without raising exceptions in case if one of arguments is NAN. However, these functions are not supported in MSVC 2010 (which we still support) and can be slow (according to the first link above). Even more, systematic use of such macros does not seem to be feasible, as the desired result may depend on a case.

Thus I do not see what we could reasonably improve in regard to dealing with NANs.

drazmyslovich

2019-10-01 18:12

developer   ~0087671

Dear Andrey,

surely, you are right and I agree with the above. I'm sorry, that this bug caused some confusion, as I missed this particular detail - for some reasons the signals are enabled in our code base for the scenario, where I were able to observe this problem.

Thank you for your detailed analysis of the situation.

Regards,
Dima

abv

2019-10-01 18:51

manager   ~0087672

Thank you Dmitry for feedback; I am closing the issue then

git

2020-10-08 11:02

administrator   ~0095816

Branch CR30875 has been deleted by inv.

SHA-1: be00a0482ba0bb382f27dae7afee660cbe4219a5

Issue History

Date Modified Username Field Change
2019-08-07 15:36 drazmyslovich New Issue
2019-08-07 15:36 drazmyslovich Assigned To => abv
2019-08-07 15:36 drazmyslovich Assigned To abv => drazmyslovich
2019-08-07 15:37 git Note Added: 0086068
2019-08-07 15:38 drazmyslovich Note Added: 0086069
2019-08-07 15:38 drazmyslovich Assigned To drazmyslovich => abv
2019-08-07 15:38 drazmyslovich Status new => resolved
2019-08-07 15:38 drazmyslovich Steps to Reproduce Updated
2019-08-07 19:54 kgv Note Added: 0086076
2019-08-07 19:54 kgv Note Edited: 0086076
2019-08-07 19:54 kgv Note Edited: 0086076
2019-08-07 20:29 drazmyslovich Note Added: 0086078
2019-08-07 20:47 kgv Note Added: 0086079
2019-08-12 13:49 abv Note Added: 0086182
2019-08-12 13:49 abv Assigned To abv => drazmyslovich
2019-08-12 13:49 abv Status resolved => feedback
2019-08-13 12:57 kgv Relationship added related to 0025602
2019-08-15 14:11 drazmyslovich Note Added: 0086262
2019-08-15 14:11 drazmyslovich Assigned To drazmyslovich => abv
2019-08-16 10:53 kgv Summary BSplCLib crashes in case of infinite upper knot => Foundation Classes - BSplCLib crashes in case of infinite upper knot
2019-08-24 09:51 abv Relationship added related to 0026308
2019-09-25 23:23 abv Assigned To abv => drazmyslovich
2019-09-25 23:56 abv Note Added: 0087520
2019-09-26 00:57 abv Note Added: 0087521
2019-10-01 18:12 drazmyslovich Note Added: 0087671
2019-10-01 18:12 drazmyslovich Assigned To drazmyslovich => abv
2019-10-01 18:12 drazmyslovich Status feedback => resolved
2019-10-01 18:51 abv Note Added: 0087672
2019-10-01 18:51 abv Status resolved => closed
2019-10-01 18:51 abv Resolution open => no change required
2019-10-01 18:51 abv Target Version 7.4.0 =>
2020-10-08 11:02 git Note Added: 0095816