Repository: opencv
Issue: UBSan runtime errors in imgproc/src/drawing.cpp
Labels: priority: low

##### System information (version)
- OpenCV => 3.1.0
- Operating System / Platform => Fedora 28
- Compiler => Clang 7.0 @2018/10/09

##### Detailed description

The imgproc drawing.cpp code relies upon undefined behavior in the C++ compiler to handle its translation between integral and fixed point position representations, which cause failures in UndefinedBehaviorSanitizer and indicate that the code could break in other compiler versions.

From examining and trying to patch up the code, there are some systemic issues with this code. It converts integral points to an internal fixed-point representation by left-shifting the incoming coordinates by 16 bits, or less if the incoming position data from the API call is specified to already be in a fixed point format. It then shifts them back right to the integral format as appropriate during and after execution.

This happens to work when those values are negative because C++ compilers historically treat such operations on negative values as if they were casted to unsigned first and then casted back. However, that is undefined behavior in the C++ standard and could theoretically break, which is why the sanitizer complains.

This breaks OpenCV in codebases that are compiling it from source with these sanitizers enabled.

A possible fix would be to add helper functions for doing the conversions to/from these representations that does the casting to unsigned before the shifts and then back to the signed representation explicitly. Even better would be to create internal type wrappers for these representations that do these conversions automatically to make it harder to forget to do them in the proper way at each conversion point.

Also, line 1118 should change from:
`ptr += img.step*y;`
...to:
`ptr += static_cast<int>(img.step)*y;`
...as the undefined behavior sanitizer is detecting the result of this operation as an underflow when y is negative because of the lack of explicit casting as img.step is unsigned.

##### Steps to reproduce

1. Build OpenCV with [UndefinedBehaviorSanitizer](http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html).
2. Run code that uses the imgproc drawing module to draw shapes near borders of an image.

Result:
Errors like:
```
opencv/src/modules/imgproc/src/drawing.cpp:701:37: runtime error: left shift of negative value -158218
#0  0x000000000d22f160 in __ubsan::ScopedReport::~ScopedReport() ()
#1  0x000000000d23116e in handleShiftOutOfBoundsImpl(__ubsan::ShiftOutOfBoundsData*, unsigned long, unsigned long, __ubsan::ReportOptions) ()
#2  0x000000000d2337c0 in __ubsan_handle_shift_out_of_bounds_abort ()
#3  0x000000000903c40f in cv::Line2 (img=..., pt1=..., pt2=..., color=0x7fffc1e7a680) at .../opencv/src/modules/imgproc/src/drawing.cpp:701
#4  0x000000000900f9fd in cv::FillConvexPoly (img=..., v=0x7fffc1e79da0, npts=4, color=0x7fffc1e7a680, line_type=8, shift=16) at .../opencv/src/modules/imgproc/src/drawing.cpp:1105
#5  0x000000000900bbd2 in cv::ThickLine (img=..., p0=..., p1=..., color=0x7fffc1e7a680, thickness=98304, line_type=8, flags=3, shift=0) at .../opencv/src/modules/imgproc/src/drawing.cpp:1615
#6  0x00000000090097a3 in cv::line (_img=..., pt1=..., pt2=..., color=..., thickness=3, line_type=8, shift=0) at .../opencv/src/modules/imgproc/src/drawing.cpp:1746
#7  0x000000000900c9f8 in cv::arrowedLine (img=..., pt1=..., pt2=..., color=..., thickness=3, line_type=8, shift=0, tipLength=0.10000000000000001) at .../opencv/src/modules/imgproc/src/drawing.cpp:1760
...
```