CUDA `copy_strided_src` copies past the end of an offset source view, silently corrupting `slice_scatter` (CPU disagrees)
Summary
On the CUDA backend, copy_strided_src sizes its copy from the whole storage lengths rather than from the source view. When the source is an offset view whose storage continues past the view (anything produced by narrow, which is stride-contiguous but starts partway into its parent), and the destination has room, it copies too many elements.
Tensor::slice_scatter is the visible casualty: scattering an offset source into position p also overwrites the elements after p with whatever follows the source in its own storage. No error is raised, and the CPU backend gives a different (correct) answer for the same call.
Observed on 0.11.0 and on current main (d5fee525); the code below is unchanged between them.
The discrepancy
// candle-core/src/cpu_backend/mod.rs:902 -- copies exactly the view's length
StridedBlocks::SingleBlock { start_offset, len } =>
dst[dst_offset..dst_offset + len]
.copy_from_slice(&src[start_offset..start_offset + len])// candle-core/src/cuda_backend/mod.rs:1208 -- derives the length from storage
fn slice_src_and_dst<'a, T>(
src: &'a CudaSlice<T>,
src_l: &Layout,
dst: &'a mut CudaSlice<T>,
dst_offset: usize,
) -> (cudarc::driver::CudaView<'a, T>, cudarc::driver::CudaViewMut<'a, T>) {
let src_offset = src_l.start_offset();
let to_copy = dst
.len()
.saturating_sub(dst_offset)
.min(src.len().saturating_sub(src_offset));
let src = src.slice(src_offset..src_offset + to_copy);
let dst = dst.slice_mut(dst_offset..dst_offset + to_copy);
(src, dst)
}to_copy should be src_l.shape().elem_count(). It only equals that by accident, when the destination is no larger than the source view. That happens to hold for Tensor::copy / try_clone, which is presumably why this has gone unnoticed. It does not hold for slice_scatter0, where dst is the whole output and dst_offset points into it.
Reproduction
use candle_core::{Device, Tensor};
fn main() -> candle_core::Result<()> {
for device in [Device::Cpu, Device::new_cuda(0)?] {
// Recipient [1, 4, 3].
let base = Tensor::new(
&[[[0f32, 1., 2.], [3., 4., 5.], [6., 7., 8.], [9., 10., 11.]]],
&device,
)?;
// Donor [4, 3]; take row 2 as a view, so the view starts at offset 6
// and its storage still holds rows 3 onwards.
let donor = Tensor::new(
&[
[90f32, 91., 92.],
[93., 94., 95.],
[96., 97., 98.],
[99., 100., 101.],
],
&device,
)?;
let row = donor.narrow(0, 2, 1)?.unsqueeze(0)?; // [1, 1, 3]
let out = base.slice_scatter(&row, 1, 2)?;
println!("{device:?}: {:?}", out.flatten_all()?.to_vec1::<f32>()?);
}
Ok(())
}Only position 2 was asked for, so both devices should print
[0, 1, 2, 3, 4, 5, 96, 97, 98, 9, 10, 11]Actual:
Cpu : [0, 1, 2, 3, 4, 5, 96, 97, 98, 9, 10, 11] correct
Cuda(CudaDevice(DeviceId(1))): [0, 1, 2, 3, 4, 5, 96, 97, 98, 99, 100, 101] position 3 clobberedPosition 3 has been overwritten with the donor's row 3, which the caller never referenced. Making the source own its storage (Tensor::new, or any op that allocates by elem_count) hides the bug; .contiguous() does not, because a narrow view is already contiguous and keeps its parent's storage.
Tensor::cat over the same narrowed pieces was tested alongside and is correct on both backends, so this appears reachable through slice_scatter's use of the helper rather than through every caller of it.
Suggested fix
let to_copy = src_l.shape().elem_count();keeping the clamp against dst.len() - dst_offset if a defensive bound is still wanted. slice_src_and_dst is only reached from the src_l.is_contiguous() branch of copy_strided_src, where elem_count is exactly the number of elements the caller means to move.
How it surfaced
Downstream, an activation-patching intervention overwrites one sequence position of a [batch, seq_len, hidden] residual stream with a row taken from another forward pass. That donor row is naturally a narrow view into a captured activation. On CUDA the patch also overwrote every position after the patch site.
The symptom was not a crash but a plausible number: a causal-tracing table reporting 100% recovery at every layer for every token position but the last, including the final layer, where patching a non-final position cannot affect the logits at all. Unit tests passed on CPU and CUDA, because they built the source with Tensor::new; only a donor row taken as a view reproduces it.
Related
This is the mirror image of #3874, where the CPU backend read a view as though it were the whole storage and the GPU backends were correct. Here CUDA does it and the CPU is correct. It sits with the other recent view/offset fixes (#3735 / #3736, #3893 / #3894, #3853, #3836), which is the argument for fixing the shared helper rather than each call site.
Source: huggingface/candle