Vérification de l'encapsuleur OpenCvSharp pour OpenCV avec PVS-Studio

Figure 2

OpenCV est une bibliothèque open source d'algorithmes de vision par ordinateur et de traitement d'images et d'algorithmes numériques à usage général. La bibliothèque est bien connue des développeurs C ++. Outre C ++, il existe également des versions pour Python, Java, Ruby, Matlab, Lua et d'autres langages. Comme C #, qui est le langage dans lequel je me spécialise, ne figure pas sur cette liste, j'ai choisi OpenCvSharp, un wrapper C # d'OpenCV, pour le vérifier avec PVS-Studio. Les résultats de cette vérification sont discutés dans cet article.

Présentation


Avant de faire partie de l'équipe PVS-Studio, j'avais participé à la fabrication de robots à présenter lors d'expositions. Mes tâches comprenaient les travaux de réparation les plus élémentaires (les pannes majeures ont été gérées par une autre personne) ainsi que le développement de logiciels et d'utilitaires de toutes sortes.

Figure 1

Moi, fatigué et nouveau en ville, avec un robot KIKI fraîchement déballé.

Au fait, la partie développement était assez drôle. Chaque fois que l'un de nous a eu une idée d'une nouvelle façon de surprendre les visiteurs de l'exposition, nous l'avons évoquée et si tout le monde l'aimait, nous nous mettions au travail. Une fois, il nous est venu à l'esprit de créer un robot capable de reconnaître un visage humain et de répondre par un discours de bienvenue.

J'ai cherché sur Google une bibliothèque pour mes besoins et suis tombé sur OpenCV, une bibliothèque d'algorithmes de vision par ordinateur. Mais j'ai été déçu très vite en comprenant que OpenCV était implémenté en C ++. Ma connaissance du C ++, que j'avais étudiée à l'université, n'était évidemment pas suffisante. J'ai donc googlé un peu plus et trouvé OpenCvSharp, un wrapper de la bibliothèque pour C #, qui est le langage dans lequel je me spécialise. Cela fait environ six mois depuis lors, le programme a longtemps été écrit et utilisé, et maintenant j'ai finalement décidé de jeter un œil "sous le capot" d'OpenCvSharp et de scanner son code source avec l'analyseur statique PVS-Studio.

Le projet en cours d'analyse


OpenCvSharp est un wrapper d'OpenCV pour une utilisation dans les projets C #. Soit dit en passant, nous avons déjà vérifié OpenCV dans le passé. Les points forts d'OpenCvSharp sont la grande collection d'échantillons de code, la prise en charge multiplateforme (il s'exécute sur n'importe quelle plate-forme prise en charge par Mono) et une installation facile.

L'encapsuleur est un petit projet d'environ 112 200 lignes de code C #. 1,2% d'entre eux sont des commentaires qui, je dois dire, sont étrangement peu nombreux. D'un autre côté, il y a pas mal de bugs pour un projet aussi petit. J'ai choisi plus de 20 exemples pour cet article, mais l'analyseur en a trouvé beaucoup d'autres, qui ne sont ni intéressants ni évidents.

PVS-Studio


PVS-Studio est un outil pour détecter les bogues et les vulnérabilités potentielles dans le code source des programmes écrits en C, C ++, C # et Java. Il fonctionne sur Windows, Linux et macOS. Outre le code inaccessible, les erreurs de programmation et les fautes de frappe, PVS-Studio, comme cela a déjà été mentionné, est capable de détecter les problèmes de sécurité potentiels. Par conséquent, il peut être considéré comme un outil SAST (Static Application Security Testing).

Les avertissements les plus intéressants


Ce qui rend la méthode WriteableBitmapConverter spéciale, c'est qu'elle a déclenché quatre avertissements du même type à la fois:

  • V3005 La variable 'optimumChannels [PixelFormats.Indexed1]' est assignée à elle-même. WriteableBitmapConverter.cs 22
  • V3005 La variable 'optimumChannels [PixelFormats.Indexed8]' est assignée à elle-même. WriteableBitmapConverter.cs 23
  • V3005 La variable 'optimumTypes [PixelFormats.Indexed1]' est assignée à elle-même. WriteableBitmapConverter.cs 50
  • V3005 La variable 'optimumTypes [PixelFormats.Indexed8]' est assignée à elle-même. WriteableBitmapConverter.cs 51

static WriteableBitmapConverter() { optimumChannels = new Dictionary <PixelFormat, int>(); optimumChannels[PixelFormats.Indexed1] = // <= optimumChannels[PixelFormats.Indexed8] = // <= optimumChannels[PixelFormats.Gray2] = optimumChannels[PixelFormats.Gray4] = optimumChannels[PixelFormats.Gray8] = optimumChannels[PixelFormats.Gray16] = optimumChannels[PixelFormats.Gray32Float] = optimumChannels[PixelFormats.Indexed1] = // <= optimumChannels[PixelFormats.Indexed2] = optimumChannels[PixelFormats.Indexed4] = optimumChannels[PixelFormats.Indexed8] = // <= .... optimumTypes = new Dictionary <PixelFormat, MatType>(); optimumTypes[PixelFormats.Indexed1] = // <= optimumTypes[PixelFormats.Indexed8] = // <= optimumTypes[PixelFormats.Gray2] = optimumTypes[PixelFormats.Gray4] = optimumTypes[PixelFormats.Gray8] = optimumTypes[PixelFormats.Indexed1] = // <= optimumTypes[PixelFormats.Indexed2] = optimumTypes[PixelFormats.Indexed4] = optimumTypes[PixelFormats.Indexed8] = // <= optimumTypes[PixelFormats.BlackWhite] = .... } .... public static class PixelFormats { .... public static PixelFormat Indexed8 { get; } .... public static PixelFormat Indexed1 { get; } .... } 

La classe PixelFormats est définie dans l'espace de noms System.Windows.Media et est une collection de différents formats de pixels. L'analyseur souligne que les éléments optimumChannels [PixelFormats.Indexed1] et optimumChannels [PixelFormats.Indexed8] se voient attribuer une deuxième fois des valeurs dans la méthode WriteableBitmapConverter , ce qui n'a aucun sens. On ne sait pas si c'est juste une faute de frappe ou si le programmeur voulait dire autre chose. Soit dit en passant, cet extrait est un exemple frappant de la façon dont les analyseurs statiques peuvent être utiles: en regardant un tas de lignes similaires, vous êtes moins concentré - il n'est pas étonnant que les fautes de frappe restent inaperçues malgré la révision du code. Les analyseurs statiques, cependant, n'ont pas de difficulté à maintenir leur attention et n'ont pas besoin de repos, ils peuvent donc attraper des bugs comme ça sans effort.

Figure 5

Ressentez la puissance de l'analyse statique.

Message de diagnostic PVS-Studio : V3021 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la méthode return. Cela signifie que la deuxième instruction «if» est insensée InputArray.cs 394

 private static MatType EstimateType(Type t) { .... if (t == typeof(Vec2b)) return MatType.CV_8UC2; if (t == typeof(Vec3b)) return MatType.CV_8UC3; if (t == typeof(Vec4b)) return MatType.CV_8UC4; if (t == typeof(Vec6b)) return MatType.CV_8UC(6); if (t == typeof(Vec2s)) // <= return MatType.CV_16SC2; .... if (t == typeof(Vec2s)) // <= return MatType.CV_32SC2; .... } 

Ce bug est quelque peu similaire au précédent. Le développeur vérifie deux fois la même condition. Cela n'a pas de sens ici car la branche then de l'instruction "duplicate" if ne s'exécutera jamais car:

  • si la première condition est vraie, la méthode retournera;
  • si la première condition est fausse, la seconde sera fausse aussi parce que la variable vérifiée, t , ne change pas entre les deux vérifications.

Ce code doit être révisé; il est très probable que la deuxième copie de Vec2s était en fait censée être une autre variable.

Message de diagnostic PVS-Studio : V3010 La valeur de retour de la fonction 'ToString' doit être utilisée. ImgProcTest.cs 80

 public static RectanglesIntersectTypes RotatedRectangleIntersection(RotatedRect rect1, RotatedRect rect2, out Point2f[] intersectingRegion) { using (var intersectingRegionVec = new VectorOfPoint2f()) { int ret = NativeMethods .imgproc_rotatedRectangleIntersection_vector( rect1, rect2, intersectingRegionVec.CvPtr); intersectingRegion = intersectingRegionVec.ToArray(); return (RectanglesIntersectTypes) ret; } } public void RotatedRectangleIntersectionVector() { var rr1 = new RotatedRect(new Point2f(100, 100), new Size2f(100, 100), 45); var rr2 = new RotatedRect(new Point2f(130, 100), new Size2f(100, 100), 0); Cv2.RotatedRectangleIntersection(rr1, rr2, out var intersectingRegion); .... intersectingRegion.ToString(); } 

La méthode RotatedRectangleIntersection est accessible via le paramètre intersectingRegion et renvoie un tableau d'éléments de type Point2f . Une fois que intersectingRegion a été rempli de valeurs, la méthode ToString () est appelée sur le tableau. Cela n'affecte en rien les éléments du tableau et aucun travail utile n'est effectué dans la dernière ligne, il serait donc juste de supposer que le développeur a simplement oublié de supprimer cette pièce.

Messages de diagnostic PVS-Studio:

  • V3021 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la méthode return. Cela signifie que la deuxième instruction «if» est insensée Cv2_calib3d.cs 1370
  • V3022 L'expression 'objectPoints == null' est toujours fausse. Cv2_calib3d.cs 1372

 public static double CalibrateCamera(....) { if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); if (objectPoints == null) throw new ArgumentNullException(nameof(objectPoints)); .... } 

Nous avons cloné du code ici, d'où les deux avertissements. Le premier dit que les deux instructions if vérifient la même condition. Si cette condition est vraie, la méthode retournera dans la branche alors de la première instruction if . Par conséquent, la deuxième condition sera toujours fausse, ce que nous dit le deuxième avertissement. Il semble que le programmeur ait cloné ce fragment à l'aide du copier-coller, mais a oublié de le modifier.

Figure 6


Copier-coller mignon.

Autres avertissements de ce type:

  • V3021 Il existe deux instructions 'if' avec des expressions conditionnelles identiques. La première instruction «if» contient la méthode return. Cela signifie que la deuxième instruction «si» est insensée Cv2_calib3d.cs 1444
  • V3022 L'expression 'objectPoints == null' est toujours fausse. Cv2_calib3d.cs 1446

Message de diagnostic PVS-Studio: l' expression V3022 'label == MarkerValue' est toujours fausse. Labeller.cs 135

 internal static class Labeller { .... private const int MarkerValue = -1; public static int Perform(Mat img, CvBlobs blobs) { .... int label = 0; int lastLabel = 0; CvBlob lastBlob = null; for (int y = 0; y < h; y++) { for (int x = 0; x < w; x++) { if (imgIn[x + y * step] == 0) continue; bool labeled = labels[y, x] != 0; if (....) { labeled = true; // Label contour. label++; if (label == MarkerValue) // <= throw new Exception(); .... } .... } .... } } } 

Une variable nommée label est créée et initialisée à 0. Si une certaine condition est vraie, elle sera incrémentée d'une unité. De plus, cette variable n'est jamais décrémentée dans cet extrait. Par conséquent, le vérifier pour la constante -1, comme dans la ligne pointée par l'analyseur, n'a aucun sens.

Message de diagnostic PVS-Studio: V3038 L'argument a été transmis à la méthode plusieurs fois. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 124

 public static void FastNlMeansDenoisingMulti(....) { .... NativeMethods.photo_fastNlMeansDenoisingMulti( srcImgPtrs, srcImgPtrs.Length, dst.CvPtr, imgToDenoiseIndex, templateWindowSize, h, templateWindowSize, searchWindowSize); .... } 

Pour comprendre ce que l'analyseur nous dit, jetons un coup d'œil aux paramètres de la méthode photo_fastNlMeansDenoisingMulti :

 public static extern void photo_fastNlMeansDenoisingMulti( IntPtr[] srcImgs, int srcImgsLength, IntPtr dst, int imgToDenoiseIndex, int temporalWindowSize, float h, int templateWindowSize, int searchWindowSize) 

Simplifions-le encore plus pour le rendre complètement simple. Comparez ces lignes:

 NativeMethods.photo_fastNlMeansDenoisingMulti( .... templateWindowSize, .... templateWindowSize, ....); public static extern void photo_fastNlMeansDenoisingMulti( .... int temporalWindowSize, .... int templateWindowSize, ....) 

La variable templateWindowSize est déclarée deux fois, mais la première fois qu'elle est mentionnée doit en fait être la déclaration de temporalWindowSize . Une autre chose que l'analyseur n'a pas aimé est que la valeur de temporalWindowSize n'est pas du tout utilisée dans la méthode photo_fastNlMeansDenoisingMulti . Cela pourrait être une décision consciente, mais je regarderais de plus près ce code si j'étais l'auteur.

Autres avertissements de ce type:

  • V3038 L'argument a été passé plusieurs fois à la méthode. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 149
  • V3038 L'argument a été passé plusieurs fois à la méthode. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 180
  • V3038 L'argument a été passé plusieurs fois à la méthode. Il est possible que d'autres arguments soient passés à la place. Cv2_photo.cs 205

L'exemple suivant est quelque peu similaire au précédent.

Message de diagnostic PVS-Studio: V3066 Ordre incorrect possible des arguments passés à la méthode 'calib3d_Rodrigues_MatToVec': 'matrixM.CvPtr' et 'vectorM.CvPtr'. Cv2_calib3d.cs 86

 public static void Rodrigues(double[,] matrix, out double[] vector, out double[,] jacobian) { .... using (var jacobianM = new Mat<double>()) { NativeMethods.calib3d_Rodrigues_MatToVec (matrixM.CvPtr, vectorM.CvPtr, jacobianM.CvPtr); .... } } 

Regardons les paramètres de la méthode calib3d_Rodrigues_MatToVec :

 public static extern void calib3d_Rodrigues_MatToVec( IntPtr vector, IntPtr matrix, IntPtr jacobian) 

Il semble que la méthode calib3d_Rodrigues_MatToVec soit appelée avec les arguments matrixM.CvPtr et vectorM.CvPtr échangés accidentellement. Les auteurs doivent vérifier cet extrait: il peut y avoir une erreur qui empêche les calculs corrects.

Message de diagnostic PVS-Studio: V3063 Une partie de l'expression conditionnelle est toujours fausse si elle est évaluée: data == null. Mat.cs 3539

 private void CheckArgumentsForConvert(....) { .... if (data == null) throw new ArgumentNullException(nameof(data)); MatType t = Type(); if (data == null || (data.Length * dataDimension) // <= (data.Length * dataDimension) % t.Channels != 0) .... } 

L'analyseur signale que la deuxième vérification des données == null ne sera jamais vraie car si les données sont égales à null dans la première condition, une exception sera levée et l'exécution n'atteindra jamais la deuxième vérification.

Figure 7

Je sais que tu es fatigué, mais nous avons presque fini.

Message de diagnostic PVS-Studio: V3127 Deux fragments de code similaires ont été trouvés. Il s'agit peut-être d'une faute de frappe et la variable 'window' devrait être utilisée à la place de 'src2' Cv2_imgproc.cs 1547

 public static Point2d PhaseCorrelateRes(....) { if (src1 == null) throw new ArgumentNullException(nameof(src1)); if (src2 == null) throw new ArgumentNullException(nameof(src2)); if (window == null) throw new ArgumentNullException(nameof(src2)); // <= .... } 

L'analyseur a repéré une faute de frappe dans cet extrait. Les variables sont vérifiées pour null et, si elles sont vraies, chaque vérification lève une exception. Cependant, cela ne fonctionne pas tout à fait correctement pour la variable de fenêtre . Si sa valeur est égale à null , une exception correspondante est également levée mais avec le mauvais texte. Il ne sera pas question de fenêtre ; ce sera plutôt src2 . La condition devrait apparemment être révisée comme suit:

 if (window == null) throw new ArgumentNullException(nameof(window)); 

Message de diagnostic PVS-Studio: V3142 Code inaccessible détecté. Il est possible qu'une erreur soit présente. MatOfT.cs 873

Maintenant, juste pour un changement, examinons le cas où l'analyseur est techniquement correct à propos du code inaccessible, mais il n'y a en fait aucune erreur. C'est un avertissement qui peut être appelé à la fois vrai et faux en même temps.

 public new Mat<TElem> SubMat(params Range[] ranges) { Mat result = base.SubMat(ranges); return Wrap(result); } 

L'analyseur nous indique que l'instruction de retour est inaccessible. Regardons le corps de la méthode SubMat pour voir si l'analyseur dit la vérité.

 public Mat SubMat(params Range[] ranges) { throw new NotImplementedException(); /* if (ranges == null) throw new ArgumentNullException(); ThrowIfDisposed(); CvSlice[] slices = new CvSlice[ranges.Length]; for (int i = 0; i < ranges.Length; i++) { slices[i] = ranges[i]; } IntPtr retPtr = NativeMethods.core_Mat_subMat1(ptr, ranges.Length, ranges); Mat retVal = new Mat(retPtr); return retVal;*/ } 

Comme vous pouvez le voir, la fonction est actuellement incomplète et générera toujours une exception. L'analyseur est absolument correct en soulignant le code inaccessible - mais ce n'est pas un véritable bug.

Les trois défauts suivants sont du même type, mais ils sont tellement cool que je n'ai pas pu m'empêcher d'inclure les trois.

Message de diagnostic PVS-Studio: l' expression V3022 'String.IsNullOrEmpty ("winName")' est toujours fausse. Cv2_highgui.cs 46

 public static void DestroyWindow(string winName) { if (String.IsNullOrEmpty("winName")) .... } 

Message de diagnostic PVS-Studio: l' expression V3022 'string.IsNullOrEmpty ("fileName")' est toujours fausse. FrameSource.cs 37

 public static FrameSource CreateFrameSource_Video(string fileName) { if (string.IsNullOrEmpty("fileName")) .... } 

Message de diagnostic PVS-Studio: l' expression V3022 'string.IsNullOrEmpty ("fileName")' est toujours fausse. FrameSource.cs 53

 public static FrameSource CreateFrameSource_Video_CUDA(string fileName) { if (string.IsNullOrEmpty("fileName")) .... } 

Parfois, les avertissements V3022 (à propos d'expressions toujours vraies / fausses) pointent vers des bogues vraiment étranges ou drôles. Les trois exemples ci-dessus contiennent la même erreur. La méthode a un paramètre de type chaîne dont la valeur doit être vérifiée. Ce qui est vérifié à la place, cependant, est un littéral de chaîne dont le texte est le nom de la variable, c'est-à-dire le nom de la variable entre guillemets.

Figure 18

Le programmeur doit avoir écrit une fois un bloc de code défectueux, puis cloné par copier-coller.

Conclusion


Les développeurs d'OpenCvSharp ont fait un travail important et important et, en tant qu'utilisateur de leur bibliothèque, j'en suis totalement reconnaissant. Merci les gars!

Mais maintenant que je fais partie de l'équipe PVS-Studio et que j'ai vu le code de la bibliothèque, je dois dire que l'aspect qualité n'a pas été pris en compte. Le projet ne ressemble pas à un contrôle régulier avec des analyseurs statiques, et de nombreux bogues sont apparemment corrigés à l'aide de techniques plus coûteuses (telles que les tests ou les commentaires des utilisateurs), et certains bogues continuent de vivre dans le code et ce sont eux que nous capturons avec notre analyseur. Ce sujet est abordé plus en détail dans ce petit billet sur la philosophie de l'analyse statique.

Étant donné qu'OpenCvSharp est open-source et disponible gratuitement sur GitHub, ses auteurs peuvent utiliser l' une des options de licence gratuites pour PVS-Studio pour commencer à l'utiliser régulièrement.

Merci d'avoir lu. N'hésitez pas à télécharger une copie d'essai de PVS-Studio pour vérifier vos propres projets.

Source: https://habr.com/ru/post/fr473478/


All Articles