You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2023/11/23 20:42:44 UTC
[Impala-ASF-CR] IMPALA-12575: test executor group num queries executing metric fails in UBSAN build
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20726
Change subject: IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
......................................................................
IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
test_executor_group_num_queries_executing_metric fails in UBSAN builds
with the Small String Optimization. It fails because it is possible to
invoke Smallify() on a StringValue where ptr is nullptr and len = 0. In
this case we call 'memcpy(rep.small_rep.buf, nullptr, 0)' internally
which is undefined behavior according to the specs. The memcpy we use
doesn't do any harm though.
This patch substitutes mempcy() to Ubsan::MemCpy() which does null
checking and only invokes memcpy() if neither of its arguments are
nulls.
There are other memcpy() invocations in SmallableString, but these
only make sense if neither of their arguments are nulls, so it's
good to have UBSAN-validation for them.
Testing:
* tested in UBSAN build
* added backend test for this case
Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
---
M be/src/runtime/smallable-string.h
M be/src/runtime/string-value-test.cc
2 files changed, 9 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/20726/1
--
To view, visit http://gerrit.cloudera.org:8080/20726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
Gerrit-Change-Number: 20726
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12575: test executor group num queries executing metric fails in UBSAN build
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20726 )
Change subject: IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/20726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
Gerrit-Change-Number: 20726
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Nov 2023 12:06:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12575: test executor group num queries executing metric fails in UBSAN build
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20726 )
Change subject: IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/14515/ : 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/20726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
Gerrit-Change-Number: 20726
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Nov 2023 21:11:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12575: test executor group num queries executing metric fails in UBSAN build
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20726 )
Change subject: IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/20726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
Gerrit-Change-Number: 20726
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Nov 2023 01:16:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12575: test executor group num queries executing metric fails in UBSAN build
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20726 )
Change subject: IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
......................................................................
IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
test_executor_group_num_queries_executing_metric fails in UBSAN builds
with the Small String Optimization. It fails because it is possible to
invoke Smallify() on a StringValue where ptr is nullptr and len = 0. In
this case we call 'memcpy(rep.small_rep.buf, nullptr, 0)' internally
which is undefined behavior according to the specs. The memcpy we use
doesn't do any harm though.
This patch substitutes mempcy() to Ubsan::MemCpy() which does null
checking and only invokes memcpy() if neither of its arguments are
nulls.
There are other memcpy() invocations in SmallableString, but these
only make sense if neither of their arguments are nulls, so it's
good to have UBSAN-validation for them.
Testing:
* tested in UBSAN build
* added backend test for this case
Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
Reviewed-on: http://gerrit.cloudera.org:8080/20726
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Csaba Ringhofer <cs...@cloudera.com>
---
M be/src/runtime/smallable-string.h
M be/src/runtime/string-value-test.cc
2 files changed, 9 insertions(+), 1 deletion(-)
Approvals:
Impala Public Jenkins: Verified
Csaba Ringhofer: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/20726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
Gerrit-Change-Number: 20726
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-12575: test executor group num queries executing metric fails in UBSAN build
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/20726 )
Change subject: IMPALA-12575: test_executor_group_num_queries_executing_metric fails in UBSAN build
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9956/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/20726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id937b9a975ac657e63ddb848e8c61eee75cfe17d
Gerrit-Change-Number: 20726
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Nov 2023 20:44:11 +0000
Gerrit-HasComments: No