Skip to content

Fixed undefined left shifting of negative number#23503

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
seanm:issue13729
Apr 20, 2023
Merged

Fixed undefined left shifting of negative number#23503
asmorkalov merged 1 commit intoopencv:4.xfrom
seanm:issue13729

Conversation

@seanm
Copy link
Copy Markdown
Contributor

@seanm seanm commented Apr 17, 2023 •
edited by opencv-alalek
Loading

Added explicit cast to unsigned before doing the left shift.

This was caught by UBSan which reported things like:

drawing.cpp:361:22: runtime error: left shift of negative value -26214
drawing.cpp:383:22: runtime error: left shift of negative value -78642

force_builders=Custom
build_image:Custom=riscv-gcc-rvv
test_modules:Custom=core,imgproc,dnn
buildworker:Custom=linux-4
test_timeout:Custom=600
build_contrib:Custom=OFF

@seanm
Fixed undefined left shifting of negative number
aa2fabc 
Added explicit cast to unsigned before doing the left shift.

This was caught by UBSan which reported things like:

drawing.cpp:361:22: runtime error: left shift of negative value -26214
drawing.cpp:383:22: runtime error: left shift of negative value -78642
@seanm seanm mentioned this pull request Apr 17, 2023
Open
@mshabunin
Copy link
Copy Markdown
Contributor

mshabunin commented Apr 18, 2023

@asmorkalov , have we decided a policy regarding OpenCV/C++ type usage? Is uint64_t OK here or should it be replaced with uint64? 👇

opencv/modules/core/include/opencv2/core/hal/interface.h

Lines 55 to 65 in b68aa12

#if defined _MSC_VER || defined __BORLANDC__
typedef __int64 int64;
typedef unsigned __int64 uint64;
# define CV_BIG_INT(n) n##I64
# define CV_BIG_UINT(n) n##UI64
#else
typedef int64_t int64;
typedef uint64_t uint64;
# define CV_BIG_INT(n) n##LL
# define CV_BIG_UINT(n) n##ULL
#endif

@seanm
Copy link
Copy Markdown
Contributor Author

seanm commented Apr 18, 2023

According to cvdef.h "OpenCV 4.x+ requires enabled C++11 support". Are there c++11 compilers that don't support uint64_t? But I can change it if y'all want...

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Apr 18, 2023

@mshabunin No. There was issue specific to libtiff. Nothing global for OpenCV.

@asmorkalov asmorkalov requested a review from mshabunin April 19, 2023 05:46
mshabunin
mshabunin approved these changes Apr 19, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@mshabunin mshabunin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for me, though I still think that it would be nicer to use uint64 instead of uint64_t for consistency with surrounding code, but it is not critical.

@asmorkalov asmorkalov added this to the 4.8.0 milestone Apr 19, 2023
@asmorkalov asmorkalov merged commit 4c06a72 into opencv:4.x Apr 20, 2023
@asmorkalov asmorkalov mentioned this pull request May 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mshabunin mshabunin mshabunin approved these changes

Assignees

@mshabunin mshabunin

Labels

category: build/install category: imgproc

Projects

None yet

Milestone

4.8.0

Development

Successfully merging this pull request may close these issues.

3 participants

@seanm @mshabunin @asmorkalov