Bug 430884

Summary: crash during construction of circle by three points
Product: [Applications] kig Reporter: Maurizio Paolini <maurizio.paolini>
Component: generalAssignee: David E. Narvaez <david.narvaez>
Status: RESOLVED FIXED    
Severity: crash    
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: https://invent.kde.org/education/kig/commit/fcabe991f637150921d15c5aa2ba6d0068a909b8 Version Fixed/Implemented In: 20.12.2
Sentry Crash Report:

Description Maurizio Paolini 2020-12-28 08:42:30 UTC 
SUMMARY

Under particular circumstances during creation of a circle by three points kig crashes with segmentation fault and the message:

QPainterPath::arcTo: Adding arc where a parameter is NaN, results are undefined


STEPS TO REPRODUCE
1. start kig and create a point at the origin (holding down the 'shift' key allows to position points exactly at grid intersections)
2. click on 'circle by three points button'
3. select the constructed point as first of the three points
4. hold down the shift key and hover pointer on the origin (on the constructed point)


OBSERVED RESULT

kig crashes with Segmentation fault and the message:

QPainterPath::arcTo: Adding arc where a parameter is NaN, results are undefined

on the console where kig was started

EXPECTED RESULT

No crash, of course!


SOFTWARE/OS VERSIONS

Linux/KDE Plasma: 
(available in About System)

KDE Plasma Version: kf5-plasma-5.75.0-1.fc33.x86_64
KDE Frameworks Version: kf5 5.75.0 (I think)
Qt Version: qt5 5.15.2

ADDITIONAL INFORMATION

Although seemingly difficult to trigger this bug, it can actually happen during a normal kig construction
Comment 1 Maurizio Paolini 2021-01-02 10:47:23 UTC 
There is an easy fix for this problem.  the isSingular function at the
end of misc/common.cpp should test whether a 2x2 matrix is (numerically) singular.  Currently it does so by testing whether the determinant is small relative to the product of the norm of the two row vectors, meaning that the two rows are "nearly parallel" vectors.
However it seems that the two rows where assumed not to be zero (as 2D vectors).
A quick fix is to change the '<' test into '<=', although here we should also use some nunmerical threshold value.
Anyway, this at least should fix the NaN problem when drawing a circle through 3 coincident points.  The fix is in a piece of code that I wrote a long long
time ago, so it could be my responsibility to fix it.

diff --git a/misc/common.cpp b/misc/common.cpp
index b13349cd..d841cb62 100644
--- a/misc/common.cpp
+++ b/misc/common.cpp
@@ -489,8 +489,10 @@ bool isSingular( const double& a, const double& b,
 /*
  * test must be done relative to the magnitude of the two
  * row (or column) vectors!
+ *
+ * (2021_01_02: we also need to cover for the case where norm1 or norm2 is numerically zero!)
  */
-  return ( std::fabs(det) < test_threshold*norm1*norm2 );
+  return ( std::fabs(det) <= test_threshold*norm1*norm2 );
 }
Comment 2 David E. Narvaez 2021-01-09 22:13:28 UTC 
Thank you for reporting, looking into, and fixing this bug! I trust your analysis so please go ahead and:

1) Commit the fix to the master branch but without the comment.
2) Make the contents of the comment part of the commit message instead.
3) Make sure the commit message also has the BUG tag.
4) Cherry-pick the commit to the release/20.12 branch and add the FIXED-IN tag. This would be fixed in 20.12.2 according to the schedule[0].

[0] https://community.kde.org/Schedules/release_service/20.12_Release_Schedule
Comment 3 Maurizio Paolini 2021-01-10 22:44:08 UTC 
Git commit ec283eab82b7c81e9ad3f5d62af375a7ce37d087 by Maurizio Paolini.
Committed on 10/01/2021 at 22:43.
Pushed by paolini into branch 'master'.

Prevent crash during construction of circle by three points

In an "invalid" test the case of exactly superposed points was not contemplated.
Specifically we also need to cover for the case where norm1 or norm2 is numerically zero.

M  +1    -1    misc/common.cpp

https://invent.kde.org/education/kig/commit/ec283eab82b7c81e9ad3f5d62af375a7ce37d087
Comment 4 Maurizio Paolini 2021-01-10 22:52:13 UTC 
Git commit fcabe991f637150921d15c5aa2ba6d0068a909b8 by Maurizio Paolini.
Committed on 10/01/2021 at 22:50.
Pushed by paolini into branch 'release/20.12'.

Prevent crash during construction of circle by three points

In an "invalid" test the case of exactly superposed points was not contemplated.
Specifically we also need to cover for the case where norm1 or norm2 is numerically zero.
FIXED-IN: 20.12.2

M  +1    -1    misc/common.cpp

https://invent.kde.org/education/kig/commit/fcabe991f637150921d15c5aa2ba6d0068a909b8