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

Undefined behavior in cleanup sstables #4718

Closed
raphaelsc opened this issue Jul 16, 2019 · 3 comments

Comments

@raphaelsc
Copy link
Contributor

commented Jul 16, 2019

git commit: 8774adb

Depending on the evaluation order, the following snippet in cleanup sstables may lead to use after move:
sstables::compaction_descriptor({ std::move(sst) }, sst->get_sstable_level());

@raphaelsc raphaelsc changed the title UB in cleanup sstables Undefined behavior in cleanup sstables Jul 16, 2019
@slivne slivne added the compaction label Jul 18, 2019
@slivne slivne added the bug label Jul 18, 2019
@slivne slivne added this to the 3.2 milestone Jul 18, 2019
@slivne

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

@raphaelsc is this relevant only to runs or regular cleanups as well

need to mark if its needed for 3.0/3.1

@raphaelsc

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

avikivity added a commit that referenced this issue Jul 31, 2019
"
Fixes #4663.
Fixes #4718.
"

* 'make_cleanup_run_aware_v3' of https://github.com/raphaelsc/scylla:
  tests/sstable_datafile_test: Check cleaned sstable is generated with expected run id
  table: Make SSTable cleanup run aware
  compaction: introduce constants for compaction descriptor
  compaction: Make it possible to config the identifier of the output sstable run
  table: do not rely on undefined behavior in cleanup_sstables
avikivity added a commit that referenced this issue Aug 7, 2019
It shouldn't rely on argument evaluation order, which is ub.

Fixes #4718.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
(cherry picked from commit 0e732ed)
@avikivity

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2019

Backporting to 3.1, 3.0, and 2.3.

avikivity added a commit that referenced this issue Aug 7, 2019
It shouldn't rely on argument evaluation order, which is ub.

Fixes #4718.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
(cherry-picked from commit 0e732ed)
avikivity added a commit that referenced this issue Aug 7, 2019
It shouldn't rely on argument evaluation order, which is ub.

Fixes #4718.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
(cherry-picked from commit 0e732ed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.