You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2020/05/29 17:38:15 UTC
[Impala-ASF-CR] IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16004
Change subject: IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
......................................................................
IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
A recent fix (IMPALA-9781) switched DecodeFromFixedLenByteArray() to
use memset() to eliminate an unaligned store under GCC7. However,
this caused crashes in the UBSAN tests.
Added a cast to void* for the first argument for memset().
Testing:
- Ran parquet-plain-test in UBSAN mode
Change-Id: I7c7dd8a813c5ff8c77b57e2f8e42cbed39eff38f
---
M be/src/util/decimal-util.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/16004/1
--
To view, visit http://gerrit.cloudera.org:8080/16004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7c7dd8a813c5ff8c77b57e2f8e42cbed39eff38f
Gerrit-Change-Number: 16004
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
[Impala-ASF-CR] IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has abandoned this change. ( http://gerrit.cloudera.org:8080/16004 )
Change subject: IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/16004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I7c7dd8a813c5ff8c77b57e2f8e42cbed39eff38f
Gerrit-Change-Number: 16004
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/16004 )
Change subject: IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
......................................................................
Patch Set 1:
I'm running a UBSAN core job right now
--
To view, visit http://gerrit.cloudera.org:8080/16004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c7dd8a813c5ff8c77b57e2f8e42cbed39eff38f
Gerrit-Change-Number: 16004
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 May 2020 17:38:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/16004 )
Change subject: IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
......................................................................
Patch Set 1:
> Hm, I don't understand why it's crashing or why this fixes it, was
> it a UBSAN violation?
On second thought, I'm just going to revert. I think memset() has undefined behavior if its argument is not trivially copyable, and I'm thinking DecimalValue may not be. https://en.cppreference.com/w/cpp/string/byte/memset
I think the real fix may be for DecimalValue's operator=() to use memcpy rather than doing this memset.
Either way, I'll circle back after the revert.
--
To view, visit http://gerrit.cloudera.org:8080/16004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c7dd8a813c5ff8c77b57e2f8e42cbed39eff38f
Gerrit-Change-Number: 16004
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 May 2020 19:39:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16004 )
Change subject: IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
......................................................................
Patch Set 1:
Hm, I don't understand why it's crashing or why this fixes it, was it a UBSAN violation?
--
To view, visit http://gerrit.cloudera.org:8080/16004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c7dd8a813c5ff8c77b57e2f8e42cbed39eff38f
Gerrit-Change-Number: 16004
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 May 2020 19:18:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16004 )
Change subject: IMPALA-9800: Fix undefined behavior in DecimalUtil::DecodeFromFixedLenByteArray()
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6165/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/16004
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7c7dd8a813c5ff8c77b57e2f8e42cbed39eff38f
Gerrit-Change-Number: 16004
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 May 2020 18:25:48 +0000
Gerrit-HasComments: No