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.

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

[SPARK-35224][SQL][TESTS] Fix buffer overflow in MutableProjectionSuite #32339

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 25, 2021

What changes were proposed in this pull request?

In the test "unsafe buffer with NO_CODEGEN" of MutableProjectionSuite, fix unsafe buffer size calculation to be able to place all input fields without buffer overflow + meta-data.

Why are the changes needed?

To make the test suite MutableProjectionSuite more stable.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running the affected test suite:

$ build/sbt "test:testOnly *MutableProjectionSuite"

@github-actions github-actions bot added the SQL label Apr 25, 2021
@SparkQA
Copy link

SparkQA commented Apr 25, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/42445/

@SparkQA
Copy link

SparkQA commented Apr 26, 2021

Test build #137925 has finished for PR 32339 at commit 47cb0e5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

val unsafeBuffer = UnsafeRow.createFromByteArray(numBytes, fixedLengthTypes.length)
val numFields = fixedLengthTypes.length
val numBytes = Platform.BYTE_ARRAY_OFFSET + UnsafeRow.calculateBitSetWidthInBytes(numFields) +
UnsafeRow.WORD_SIZE * numFields
Copy link
Contributor

Choose a reason for hiding this comment

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

why did the test pass before?

Copy link
Contributor

Choose a reason for hiding this comment

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

different JVM settings?

Copy link
Member Author

Choose a reason for hiding this comment

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

It depends how underlying byte array allocated. If you are lucky, you overwrite not important memory of neighbours.

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 26, 2021

Merging to master, 3.1/3.0/2.4.
Thank you, @cloud-fan for review.

@MaxGekk MaxGekk closed this in d572a85 Apr 26, 2021
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Apr 26, 2021
…ite`

In the test `"unsafe buffer with NO_CODEGEN"` of `MutableProjectionSuite`, fix unsafe buffer size calculation to be able to place all input fields without buffer overflow + meta-data.

To make the test suite `MutableProjectionSuite` more stable.

No

By running the affected test suite:
```
$ build/sbt "test:testOnly *MutableProjectionSuite"
```

Closes apache#32339 from MaxGekk/fix-buffer-overflow-MutableProjectionSuite.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit d572a85)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 26, 2021

Here is the backport to 3.1/3.0: #32347
branch-2.4 doesn't have the test suite which was added by 04046e5 since 3.0. @cloud-fan @maropu Could you review the backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants