Possible unsound unsafe usage

[charlesxsh]

src/ptab/bit_array:90

impl<M: PrimaryTableMode> PrimaryTable<M> {
    #[allow(clippy::ptr_offset_with_cast)]
    pub fn split_chunk(
        &mut self,
        header: &Header,
        mut max_chunk_size: Option<usize>,
    ) -> Result<Vec<PartialPrimaryTable<M>>> {
        let data = M::get_mapping_address(self)?;

        if let Some(ref mut max_chunk_size) = max_chunk_size {
            *max_chunk_size -= *max_chunk_size % 8;
        }

        let bit_width = self.bit_width;

        let mut ret = vec![];
        let mut offset = 0;
        for chrom in header.chrom_list.iter() {
            let mut size = chrom.size;
            let mut start = 0;
            while size > 0 {
                let chunk_size = match max_chunk_size {
                    Some(size_limit) if size_limit < size => size_limit,
                    _ => size,
                };
                let nbytes = (chunk_size * bit_width + 7) / 8;
                ret.push(PartialPrimaryTable {
                    mapping_handle: self.mapping_handle.clone().unwrap(),
                    addr_start: unsafe { data.offset(offset as isize) as usize },
                    chunk_size: nbytes,
                    name: chrom.name.clone(),
                    start,
                    bit_width,
                    end: start + chunk_size as u32,
                    dictionary: self.dictionary.clone(),
                });
                size -= chunk_size;
                offset += nbytes;
                start += chunk_size as u32;
            }
        }
        Ok(ret)
    }
}

In the PrimaryTable::split_chunk, data is got from let data = M::get_mapping_address(self)?; and used in unsafe { data.offset(offset as isize) as usize },. If M, which can be any PrimaryTableMode, get_mapping_address returns an invalid address, the safety requirements of std::ptr::offset might not hold: "If the computed offset is non-zero, then self must be derived from a pointer to some allocation, and the entire memory range between self and the result must be in bounds of that allocation."

Reference: https://doc.rust-lang.org/std/primitive.pointer.html#method.offset

[brentp]

hi @charlesxsh , do you have a suggested fix?

[charlesxsh]

hi @charlesxsh , do you have a suggested fix?

Since I don't have deep understanding of this library, my straitforward thought would be mark this function unsafe to notify developers/users the potential risk of memory issue. Maybe there is better way to do that.