0029911: Porting to Debian80-64 : Regressions in Foundation Classes

Test for handles is updated according to behavior expected from modern compilers.

Performance test perf ncollection A1 is updated (valid range extended) according to measurements made on Debian 8 with GCC 4.9 and on Ubuntu 16.04 with CLang 3.8 and 6.0 and GCC 4.7, 4.9, and 5.0.

diff --git a/src/QANCollection/QANCollection_Handle.cxx b/src/QANCollection/QANCollection_Handle.cxx
index 01c586a6c0935766ef400f43e53653b97f3955e8..829e3b04182714801c29561d7ea30a70e0b35538 100644 (file)
--- a/src/QANCollection/QANCollection_Handle.cxx
+++ b/src/QANCollection/QANCollection_Handle.cxx
@@ -34,8 +34,14 @@
 #include <memory>
 #include <typeinfo>
 
-// auxiliary macro to check and report status
-#define CHECK(di,ok,what) di << "Checking " << what << (ok ? ": OK\n" : ": Error\n")
+// Auxiliary macro to check and report status.
+// Note that if() is used to ensure that condition is
+// evaluated before calls to stream output functions, to
+// prevent side effects from such calls in cases when condition
+// may contain references to freed objects in the stack.
+#define CHECK(di,ok,what) \
+  if (ok) di << "Checking " << what << ": OK\n";\
+  else    di << "Checking " << what << ": Error\n"
 
 //=======================================================================
 //function : QAHandleOps
@@ -117,12 +123,23 @@ static Standard_Integer QAHandleOps (Draw_Interpretor& theDI,
 #endif
 
   const Handle(Geom_Curve)& aCurve2 = aLine; // cast to base const ref
+  CHECK (theDI, !aCurve2.IsNull (), "cast to base class const reference");
 
   Handle(Geom_Line) qLine = cpLine; // constructor from const pointer -- could be made explicit...
-
-  // check whether compiler will destroy reference to temporary handle
-  const Handle(Geom_Curve)& aTmpRef (Handle(Geom_Line)::DownCast (aCurve2));
-  CHECK(theDI, ! aTmpRef.IsNull(),  "local reference of handle to base type to temporary handle object");
+  
+  // check that compiler keeps temporary object referenced by local variable
+  const Handle(Geom_Line)& aTmpRef (Handle(Geom_Line)::DownCast (aCurve2));
+  // note that here and in similar checks below we use comparison of pointers instead 
+  // of checking handle for Null, since such check may fail if temporary object is
+  // destroyed prematurely and its location is used for other object. 
+  CHECK(theDI, aTmpRef.get() == aCurve2.get(),  "local reference of to temporary handle object");
+
+  // check undesired but logical situation: 
+  // compiler does not keep temporary object referenced by local variable of base type;
+  // here compiler does not recognize that it should keep the temporary object because handle
+  // classes do not inherit each other and they use hard cast for references to simulate inheritance
+  const Handle(Geom_Curve)& aTmpRefBase (Handle(Geom_Line)::DownCast (aCurve2));
+  CHECK(theDI, aTmpRefBase.get() != aCurve2.get(),  "local reference to temporary handle object (base type)");
 
   // check operations with Handle_* classes
   Handle(Geom_Line) hLine = aLine;
@@ -159,9 +176,22 @@ static Standard_Integer QAHandleOps (Draw_Interpretor& theDI,
 
   Handle_Geom_Line qhLine = cpLine; // constructor from const pointer -- could be made explicit...
 
-  // check whether compiler will destroy reference to temporary handle
-  const Handle_Geom_Curve& hTmpRef (Handle(Geom_Line)::DownCast (aCurve2));
-  CHECK(theDI, ! hTmpRef.IsNull(),  "local reference of handle to base type to temporary handle object");
+  // check that compiler keeps temporary object referenced by local variable
+  const Handle_Geom_Line& hTmpRef (Handle(Geom_Line)::DownCast (aCurve2));
+  CHECK(theDI, hTmpRef.get() == aCurve2.get(),  "local reference to temporary object (Handle_)");
+
+  // check lifetime of temporary object referenced by local variable (base type)
+  const Handle_Geom_Curve& hTmpRefBase (Handle(Geom_Line)::DownCast (aCurve2));
+  // here we have different behavior for MSVC 2013+ where Handle_ is a class
+  // (compiler creates temporary object of approprtiate type and keeps it living
+  // until the reference is valid) and other compilers where Handle_ is
+  // typedef to handle<> (compiler does not know that the reference that is being
+  // assigned is pointing to temporary object, due to involved type cast operation)
+#if (defined(_MSC_VER) && _MSC_VER >= 1800) 
+  CHECK(theDI, hTmpRefBase.get() == aCurve2.get(),  "local reference to temporary handle object (Handle_ to base type)");
+#else
+  CHECK(theDI, hTmpRefBase.get() != aCurve2.get(),  "local reference to temporary handle object (Handle_ to base type)");
+#endif
 
   Handle(Geom_Surface) aSurf;
   (void)aSurf;

diff --git a/tests/perf/fclasses/handle b/tests/perf/fclasses/handle
index d43046bc40e73337dbcfc1413f8392b45eb1a7d0..9682ab08c648d7170d2875c59eba9140d5167ea0 100644 (file)
--- a/tests/perf/fclasses/handle
+++ b/tests/perf/fclasses/handle
@@ -1,5 +1,3 @@
-puts "TODO OCC24023 Windows: Checking local reference of handle to base type to temporary handle object"
-
 puts "========"
 puts "CR24023, check operability and performance of OCCT RTTI and handles"
 puts "========"

diff --git a/tests/perf/ncollection/A1 b/tests/perf/ncollection/A1
index 9c5092447355917862fec58aea67a3d58aa32764..3d084683f45d62cdfca32af438b0d446d5bd420c 100644 (file)
--- a/tests/perf/ncollection/A1
+++ b/tests/perf/ncollection/A1
@@ -42,13 +42,13 @@ if { [checkplatform -windows] } {
     }
 } else {
   set check_values  { 1.2363286058767904
-                      2.7537414143534
+                      5.0
                       1.5596260162601621
-                      3.937043746844462
+                      7.0
                       1.2133020329576465
                       1.2164522569168656
                       1.2495457282327385
-                      0.10352433841051313
+                      0.2
                       0.45175659293697572
                     }
 }
@@ -57,9 +57,9 @@ set index 0
 foreach key $keys {
   set value [lindex $values $index]
   if { $value > [lindex $check_values $index] } {
-    puts "Error: performance of $key become worse"
+    puts "Error: performance of $key become worse than before"
   } else {
-    puts "OK: performance of $key is OK"
+    puts "OK: performance of $key is within expected limits"
   }
   incr index
 }