add missing alignvalue calls, make an overload for data_ptr_t of Alig…

…nValue
Tishj · Jan 14, 2025 · 8bece82 · 8bece82
1 parent 16d1d64
commit 8bece82
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 2 deletions.
diff --git a/src/include/duckdb/common/helper.hpp b/src/include/duckdb/common/helper.hpp
@@ -195,6 +195,12 @@ static inline T AlignValue(T n) {
 	return ((n + (val - 1)) / val) * val;
 }
 
+template<uintptr_t alignment>
+inline data_ptr_t AlignValue(data_ptr_t addr) {
+	static_assert((alignment & (alignment - 1)) == 0, "'alignment' has to be a power of 2");
+	return reinterpret_cast<data_ptr_t>((reinterpret_cast<uintptr_t>(addr) + alignment - 1) & ~(alignment - 1));
+}
+
 template<class T, T val=8>
 constexpr inline T AlignValueFloor(T n) {
 	return (n / val) * val;

diff --git a/src/storage/compression/roaring/compress.cpp b/src/storage/compression/roaring/compress.cpp
@@ -113,6 +113,7 @@ void ContainerCompressionState::OverrideArray(data_ptr_t destination, bool nulls
 		destination += sizeof(uint8_t) * COMPRESSED_SEGMENT_COUNT;
 		compressed_arrays[nulls] = reinterpret_cast<uint8_t *>(destination);
 	} else {
+		destination = AlignValue<sizeof(uint16_t)>(destination);
 		arrays[nulls] = reinterpret_cast<uint16_t *>(destination);
 	}
 }
@@ -126,6 +127,7 @@ void ContainerCompressionState::OverrideRun(data_ptr_t destination, idx_t count)
 		destination += sizeof(uint8_t) * COMPRESSED_SEGMENT_COUNT;
 		compressed_runs = reinterpret_cast<uint8_t *>(destination);
 	} else {
+		destination = AlignValue<sizeof(RunContainerRLEPair)>(destination);
 		runs = reinterpret_cast<RunContainerRLEPair *>(destination);
 	}
 }
@@ -217,7 +219,7 @@ bool RoaringCompressState::CanStore(idx_t container_size, const ContainerMetadat
 	idx_t required_space = 0;
 	if (metadata.IsUncompressed()) {
 		// Account for the alignment we might need for this container
-		required_space += (AlignValue<idx_t>(reinterpret_cast<idx_t>(data_ptr))) - reinterpret_cast<idx_t>(data_ptr);
+		required_space += (AlignValue<sizeof(data_ptr_t)>(data_ptr) - data_ptr);
 	}
 	required_space += metadata.GetDataSizeInBytes(container_size);
 
@@ -261,7 +263,7 @@ void RoaringCompressState::InitializeContainer() {
 
 	// Override the pointer to write directly into the block
 	if (metadata.IsUncompressed()) {
-		data_ptr = reinterpret_cast<data_ptr_t>(AlignValue<idx_t>(reinterpret_cast<idx_t>(data_ptr)));
+		data_ptr = AlignValue<sizeof(idx_t)>(data_ptr);
 		FastMemset(data_ptr, ~0, sizeof(validity_t) * (container_size / ValidityMask::BITS_PER_VALUE));
 		container_state.OverrideUncompressed(data_ptr);
 	} else if (metadata.IsRun()) {

diff --git a/src/storage/compression/roaring/scan.cpp b/src/storage/compression/roaring/scan.cpp
@@ -281,6 +281,7 @@ ContainerScanState &RoaringScanState::LoadContainer(idx_t container_index, idx_t
 			current_container = make_uniq<CompressedRunContainerScanState>(container_index, container_size,
 			                                                               number_of_runs, segments, data_ptr);
 		} else {
+			data_ptr = AlignValue<sizeof(RunContainerRLEPair)>(data_ptr);
 			current_container =
 			    make_uniq<RunContainerScanState>(container_index, container_size, number_of_runs, data_ptr);
 		}
@@ -297,6 +298,7 @@ ContainerScanState &RoaringScanState::LoadContainer(idx_t container_index, idx_t
 				    container_index, container_size, cardinality, segments, data_ptr);
 			}
 		} else {
+			data_ptr = AlignValue<sizeof(uint16_t)>(data_ptr);
 			if (metadata.IsInverted()) {
 				current_container =
 				    make_uniq<ArrayContainerScanState<NULLS>>(container_index, container_size, cardinality, data_ptr);