Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] Decimal32 support introduced an UBSAN error #44276

Closed
kou opened this issue Oct 1, 2024 · 3 comments
Closed

[C++] Decimal32 support introduced an UBSAN error #44276

kou opened this issue Oct 1, 2024 · 3 comments
Assignees
@zeroshade
Labels
Component: C++ Type: bug

Comments

@kou
Copy link
Member

kou commented Oct 1, 2024 •
edited
Loading

Describe the bug, including details regarding any error messages, version, and platform.

GH-43957 introduced the following UBSAN error:

https://github.com/apache/arrow/actions/runs/11114266653/job/30880385337#step:6:4315

[ RUN      ] Decimal32Test.LeftShift
/arrow/cpp/src/arrow/util/decimal_test.cc:1727:33: runtime error: left shift of 123 by 30 places cannot be represented in type 'int32_t' (aka 'int')
    #0 0x562b6abc3a04 in arrow::Decimal32Test_LeftShift_Test::TestBody()::$_0::operator()(int, unsigned int) const /arrow/cpp/src/arrow/util/decimal_test.cc:1727:33
    #1 0x562b6abbe4a3 in arrow::Decimal32Test_LeftShift_Test::TestBody() /arrow/cpp/src/arrow/util/decimal_test.cc:1741:3
    #2 0x7fc0d8b24e7e in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/usr/local/lib/libarrow_testing.so.1800+0x106de7e) (BuildId: 118793bf888d7168cdf3c1fc75787526ccb324c5)
    #3 0x7fc0d8b19935 in testing::Test::Run() (/usr/local/lib/libarrow_testing.so.1800+0x1062935) (BuildId: 118793bf888d7168cdf3c1fc75787526ccb324c5)
    #4 0x7fc0d8b19ab4 in testing::TestInfo::Run() (/usr/local/lib/libarrow_testing.so.1800+0x1062ab4) (BuildId: 118793bf888d7168cdf3c1fc75787526ccb324c5)
    #5 0x7fc0d8b1a068 in testing::TestSuite::Run() (/usr/local/lib/libarrow_testing.so.1800+0x1063068) (BuildId: 118793bf888d7168cdf3c1fc75787526ccb324c5)
    #6 0x7fc0d8b1a76e in testing::internal::UnitTestImpl::RunAllTests() (/usr/local/lib/libarrow_testing.so.1800+0x106376e) (BuildId: 118793bf888d7168cdf3c1fc75787526ccb324c5)
    #7 0x7fc0d8b25446 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/usr/local/lib/libarrow_testing.so.1800+0x106e446) (BuildId: 118793bf888d7168cdf3c1fc75787526ccb324c5)
    #8 0x7fc0d8b19b7b in testing::UnitTest::Run() (/usr/local/lib/libarrow_testing.so.1800+0x1062b7b) (BuildId: 118793bf888d7168cdf3c1fc75787526ccb324c5)
    #9 0x562b6b6576ed in RUN_ALL_TESTS() /usr/include/gtest/gtest.h:2490:46
    #10 0x562b6b657639 in main /arrow/cpp/src/arrow/util/logging_test.cc:129:10
    #0 0x7fc0b6b43d8f in
    #12 0x7fc0b6b43e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 490fef8403240c91833978d494d39e537409b92e)
    #13 0x562b6a697dc4 in _start (/build/cpp/debug/arrow-utility-test+0x13fddc4) (BuildId: aaabec8f2b98a2f0267cb51501c0340b2eec22e2)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /arrow/cpp/src/arrow/util/decimal_test.cc:1727:33 in
/build/cpp/src/arrow/util

Component(s)

C++
@kou kou added the Type: bug label Oct 1, 2024
@kou
Copy link
Member Author

kou commented Oct 1, 2024

@zeroshade Could you take a look at this?

@zeroshade
Copy link
Member

Damnit I thought I got all of them. I'll take care of it

zeroshade added a commit to zeroshade/arrow that referenced this issue Oct 1, 2024
@zeroshade
apacheGH-44276: [C++] Fix UBSAN error introduced by Decimal32
e842291
@github-actions github-actions bot mentioned this issue Oct 1, 2024
Merged
zeroshade added a commit that referenced this issue Oct 1, 2024
@zeroshade
GH-44276: [C++] Fix UBSAN error introduced by Decimal32 (#44280)
5d689b4 
<!--
Thanks for opening a pull request!
If this is your first pull request you can find detailed information on
how
to contribute here:
* [New Contributor's
Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
* [Contributing
Overview](https://arrow.apache.org/docs/dev/developers/overview.html)


If this is not a [minor
PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes).
Could you open an issue for this pull request on GitHub?
https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the
[Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.)
of the Apache Arrow project.

Then could you also rename the pull request title in the following
format?

    GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

    MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

    PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

-->

### Rationale for this change
Fixing UBSAN errors caused by Decimal32/Decimal64 addition
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->

### What changes are included in this PR?
Wrapping left shifts in `SafeLeftShift` to avoid UBSAN errors.

<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->

* GitHub Issue: #44276
@zeroshade
Copy link
Member

Fixed by #44280

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zeroshade zeroshade

Labels
Component: C++ Type: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kou @zeroshade