Skip to content

GH-39778: [C++] Fix tail-byte access cross buffer boundary in key hash avx2 - #39800

Merged
pitrou merged 7 commits into
apache:mainfrom
zanmato1984:fix-39778
Jan 26, 2024
Merged

GH-39778: [C++] Fix tail-byte access cross buffer boundary in key hash avx2#39800
pitrou merged 7 commits into
apache:mainfrom
zanmato1984:fix-39778

Conversation

@zanmato1984

@zanmato1984zanmato1984 commented Jan 25, 2024

Copy link
Copy Markdown
Contributor

Rationale for this change

Issue #39778 seems caused by a careless (but hard to spot) bug in key hash avx2.

What changes are included in this PR?

Fix the careless bug.

Are these changes tested?

UT included.

Are there any user-facing changes?

No.

@zanmato1984

Copy link
Copy Markdown
ContributorAuthor

cc @pitrou

Comment threadcpp/src/arrow/compute/key_hash_avx2.cc
@github-actionsgithub-actionsBot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 25, 2024

@pitroupitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@pitrou

Copy link
Copy Markdown
Member

@github-actions crossbow submit -g cpp

@github-actions

Copy link
Copy Markdown

Revision: 5fea9bb

Submitted crossbow builds: ursacomputing/crossbow @ actions-cfeb62c110

TaskStatus
test-alpine-linux-cppGitHub Actions
test-build-cpp-fuzzGitHub Actions
test-conda-cppGitHub Actions
test-conda-cpp-valgrindAzure
test-cuda-cppGitHub Actions
test-debian-11-cpp-amd64GitHub Actions
test-debian-11-cpp-i386GitHub Actions
test-fedora-38-cppGitHub Actions
test-ubuntu-20.04-cppGitHub Actions
test-ubuntu-20.04-cpp-bundledGitHub Actions
test-ubuntu-20.04-cpp-minimal-with-formatsGitHub Actions
test-ubuntu-20.04-cpp-thread-sanitizerGitHub Actions
test-ubuntu-22.04-cppGitHub Actions
test-ubuntu-22.04-cpp-20GitHub Actions
test-ubuntu-22.04-cpp-no-threadingGitHub Actions

@pitroupitrou added this to the 15.0.1 milestone Jan 26, 2024
@pitrou
pitrou merged commit 13b2234 into apache:mainJan 26, 2024
@pitroupitrou removed the awaiting committer review Awaiting committer review label Jan 26, 2024
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 13b2234.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 202 possible false positives for unstable benchmarks that are known to sometimes produce them.

@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 13b2234.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 202 possible false positives for unstable benchmarks that are known to sometimes produce them.

dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…ey hash avx2 (apache#39800)
### Rationale for this change
Issue apache#39778 seems caused by a careless (but hard to spot) bug in key hash avx2.
### What changes are included in this PR?
Fix the careless bug.
### Are these changes tested?
UT included.
### Are there any user-facing changes?
No.
* Closes: apache#39778
Authored-by: Ruoxi Sun <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
raulcd pushed a commit that referenced this pull request Feb 20, 2024
…h avx2 (#39800)
### Rationale for this change
Issue #39778 seems caused by a careless (but hard to spot) bug in key hash avx2.
### What changes are included in this PR?
Fix the careless bug.
### Are these changes tested?
UT included.
### Are there any user-facing changes?
No.
* Closes: #39778
Authored-by: Ruoxi Sun <zanmato1984@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++][Acero] ASAN reports heap buffer overflow in arrow::compute::Hashing32::ProcessStripes_avx2

2 participants

@zanmato1984@pitrou