modularml / mojo

The Mojo Programming Language
https://docs.modular.com/mojo/manual/
Other
23.27k stars 2.59k forks source link

[BUG] memset segmentation fault from using wrong DTypePointer type #2420

Open dreycenfoiles opened 6 months ago

dreycenfoiles commented 6 months ago

Bug description

I found that under certain conditions, using memset can cause segfaults. I don't think these edges cases are address are addressed in the docs. I get the following stack trace:

Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  mojo                               0x00005cc1afdafc87
1  mojo                               0x00005cc1afdadade
2  mojo                               0x00005cc1afdb031f
3  libc.so.6                          0x0000750081845320
4  mojo                               0x00005cc1b11ae474
5  mojo                               0x00005cc1b11ae3dc
6  mojo                               0x00005cc1b11ae31f
7  mojo                               0x00005cc1b11b9718
8  mojo                               0x00005cc1b21d05b8
9  libKGENCompilerRTShared.so.19.0git 0x000075007243b05c KGEN_CompilerRT_AlignedAlloc + 124
10 libKGENCompilerRTShared.so.19.0git 0x000075002c0011fa KGEN_CompilerRT_AlignedAlloc + 18446744072530715162
mojo crashed!
Please file a bug report.
Segmentation fault (core dumped)

Steps to reproduce

fn main():

alias dtype = DType.float64 # Won't segfault if DType.uint8

var n = 5
var vv = List[Scalar[dtype]](n)

var data = DTypePointer[dtype].alloc(n)        

memset(data, 5, n)

vv[1] = data[0] # If you index vv[0] or leave this line out, it won't segfault

print(data) # Won't segfault if you index individual members

System information

- OS: Ubuntu 24.04
- Mojo: mojo 2024.4.1618 (37a3e965)
- Modular: 0.7.2
soraros commented 6 months ago

I don't think it has much to do with DTypePointer. vv is a list of length 1, vv[1] is UB.

jiex-liu commented 6 months ago

Question about different List constructor: Should be change the constructor that takes in the size instead of the capacity? Or should we have a new constructor that takes in the size? With only the capacity user cannot access elements in the list.

soraros commented 1 month ago

We now have capacity as a keyword only argument, this no longer applies.