Skip to content

Commit

Permalink
apacheGH-44276: [C++] Fix UBSAN error introduced by Decimal32 (apache…
Browse files Browse the repository at this point in the history
…#44280)

<!--
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: apache#44276
  • Loading branch information
zeroshade authored Oct 1, 2024
1 parent 6b59098 commit 5d689b4
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions cpp/src/arrow/util/decimal_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "arrow/util/decimal.h"
#include "arrow/util/endian.h"
#include "arrow/util/int128_internal.h"
#include "arrow/util/int_util_overflow.h"
#include "arrow/util/macros.h"

namespace arrow {
Expand Down Expand Up @@ -1724,7 +1725,7 @@ TYPED_TEST(TestBasicDecimalFunctionality, FitsInPrecision) {

TEST(Decimal32Test, LeftShift) {
auto check = [](int32_t x, uint32_t bits) {
auto expected = Decimal32(x << bits);
auto expected = Decimal32(arrow::internal::SafeLeftShift(x, bits));
auto actual = Decimal32(x) << bits;
ASSERT_EQ(actual.value(), expected.value());
};
Expand Down Expand Up @@ -1807,7 +1808,7 @@ TEST(Decimal32Test, Negate) {

TEST(Decimal64Test, LeftShift) {
auto check = [](int64_t x, uint32_t bits) {
auto expected = Decimal64(x << bits);
auto expected = Decimal64(arrow::internal::SafeLeftShift(x, bits));
auto actual = Decimal64(x) << bits;
ASSERT_EQ(actual.value(), expected.value());
};
Expand Down

0 comments on commit 5d689b4

Please sign in to comment.