modularml / mojo

The Mojo Programming Language
https://docs.modular.com/mojo
Other
22.41k stars 2.55k forks source link

[BUG] Copying null `String`/`List` introduces bugs #2834

Open helehex opened 2 months ago

helehex commented 2 months ago

Bug description

This has to do with how the default initializer creates a null pointer, but initializers with capacity=0 will still call alloc(0), and List.__copyinit__() always calls the one with capacity.

This could be solved on multiple different levels:

Steps to reproduce

var s1 = String()
var s2 = s1
print(len(s1)) # prints 0
print(len(s2)) # prints -1

System information

Host Information
  ================

  Target Triple: x86_64-unknown-linux
  CPU: skylake
  CPU Features: adx, aes, avx, avx2, bmi, bmi2, clflushopt, cmov, crc32, cx16, cx8, f16c, fma, fsgsbase, fxsr, invpcid, lzcnt, mmx, movbe, pclmul, popcnt, prfchw, rdrnd, rdseed, sahf, sgx, sse, sse2, sse3, sse4.1, sse4.2, ssse3, x87, xsave, xsavec, xsaveopt, xsaves

mojo 2024.5.2605 (9c328b12)

modular 0.8.0 (39a426b5)
gabrieldemarmiesse commented 1 month ago

I think we can put this bug on hold until small buffer optimisation and small string optimisation land, then we'll have more options to fix the bugs and some of them might not even exist anymore

JoeLoser commented 1 month ago

Agreed. I'd punt on this for a little bit until the SSO/SBO work lands. We should totally figure out our longer-term null-terminated string strategy and FFI once that dust settles.