Make `malloc(0)` and `posix_memalign(0)` return a non-null pointer

[RalfJung]

Currently we just return a null pointer, but we're also allowed to return a unique non-zero address. That would have the benefit that we can detect leaks (not calling free) or double-free.

[tiif]

This is interesting. I've had this question on my mind for quite a while, how will malloc(0) cause a memory leak if the memory allocated is 0 byte?

Besides, the return value of malloc(0) seems to be implementation-defined^1, and may return null or a valid pointer. If miri only return a non-zero address, and the user assumed malloc(0) return null (hence they don't free it), would it be considered a false positive?

[RalfJung]

This is interesting. I've had this question on my mind for quite a while, how will malloc(0) cause a memory leak if the memory allocated is 0 byte?

There can be metadata associated with this allocation, managed by the allocator, that would be leaked.

Besides, the return value of malloc(0) seems to be implementation-defined1, and may return null or a valid pointer. If miri only return a non-zero address, and the user assumed malloc(0) return null (hence they don't free it), would it be considered a false positive?

Implementation-defined means that the implementation gets to decide. If the user assumes that malloc(0) returns null, then they are making a false assumption, since it is not necessarily true that the implementation will return null.

[tiif]

I tried to take a shot at this, and while trying to get the size of old pointer in realloc, I ran into an ICE when I calllet (alloc_id, ..) = this.ptr_try_get_alloc_id(old_ptr).unwrap(); and old_ptr is ptr::null_mut(). Is this ICE expected and is there any assumption stated somewhere that null pointer cannot be passed to ptr_try_get_alloc_id()?

This is how the error looks like, I can open an issue with more details if this shouldn't happen:

thread 'rustc' panicked at src/shims/alloc.rs:135:69:
called `Result::unwrap()` on an `Err` value: 0
stack backtrace:
   0:     0x724e53b8cb55 - std::backtrace_rs::backtrace::libunwind::trace::h3d4bd8ab81318aca
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/../../backtrace/src/backtrace/libunwind.rs:105:5
   1:     0x724e53b8cb55 - std::backtrace_rs::backtrace::trace_unsynchronized::h3e6f352fd1579ed5
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x724e53b8cb55 - std::sys_common::backtrace::_print_fmt::h82151674c7c9e71f
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/sys_common/backtrace.rs:68:5
   3:     0x724e53b8cb55 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h737cfa117a0a3242
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x724e53bdbd6b - core::fmt::rt::Argument::fmt::h4c281a4e3f751385
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/core/src/fmt/rt.rs:165:63
   5:     0x724e53bdbd6b - core::fmt::write::ha939687058c9aa62
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/core/src/fmt/mod.rs:1157:21
   6:     0x724e53b8189f - std::io::Write::write_fmt::h6a610d8cac497201
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/io/mod.rs:1835:15
   7:     0x724e53b8c92e - std::sys_common::backtrace::_print::h165265ac517d31b5
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x724e53b8c92e - std::sys_common::backtrace::print::h90990ef99e1bb2a4
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x724e53b8f299 - std::panicking::default_hook::{{closure}}::h435b440f89c2555f
  10:     0x724e53b8efdd - std::panicking::default_hook::hece8ed21eabbfe86
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/panicking.rs:298:9
  11:     0x724e56caef4c - std[f233acd9da5dd114]::panicking::update_hook::<alloc[7e49742545415465]::boxed::Box<rustc_driver_impl[ef4cf2d5a728d441]::install_ice_hook::{closure#0}>>::{closure#0}
  12:     0x724e53b8f996 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::heab237e8b1b6a434
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/alloc/src/boxed.rs:2036:9
  13:     0x724e53b8f996 - std::panicking::rust_panic_with_hook::ha996882231fc9b42
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/panicking.rs:799:13
  14:     0x724e53b8f744 - std::panicking::begin_panic_handler::{{closure}}::hbe466cafe1216d4e
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/panicking.rs:664:13
  15:     0x724e53b8d019 - std::sys_common::backtrace::__rust_end_short_backtrace::hf1d2290c461b5d73
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/sys_common/backtrace.rs:171:18
  16:     0x724e53b8f477 - rust_begin_unwind
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/panicking.rs:652:5
  17:     0x724e53bd8333 - core::panicking::panic_fmt::h65836f14859a292b
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/core/src/panicking.rs:72:14
  18:     0x724e53bd8976 - core::result::unwrap_failed::h2946aee4a0f37c1d
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/core/src/result.rs:1654:5
  19:     0x61bfedc7f740 - core::result::Result<T,E>::unwrap::h8a405ccf2473f099
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/core/src/result.rs:1077:23
  20:     0x61bfedc7f740 - miri::shims::alloc::EvalContextExt::realloc::h88557ef5391ba1d2
                               at /home/byt/Documents/miri/src/shims/alloc.rs:135:34
  21:     0x61bfedc88870 - miri::shims::foreign_items::EvalContextExtPriv::emulate_foreign_item_inner::h8e3dc503802ec737
                               at /home/byt/Documents/miri/src/shims/foreign_items.rs:448:27
  22:     0x61bfedc82fee - miri::shims::foreign_items::EvalContextExt::emulate_foreign_item::hd2264792f44ca81f
                               at /home/byt/Documents/miri/src/shims/foreign_items.rs:85:15
  23:     0x61bfedc24a6b - <miri::machine::MiriMachine as rustc_const_eval::interpret::machine::Machine>::find_mir_or_eval_fn::he073b1582490a8bc
                               at /home/byt/Documents/miri/src/machine.rs:960:20
  24:     0x61bfedc24a6b - rustc_const_eval::interpret::terminator::<impl rustc_const_eval::interpret::eval_context::InterpCx<M>>::eval_fn_call::ha2b854578a3f5476
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_const_eval/src/interpret/terminator.rs:578:46
  25:     0x61bfedc5ddbf - rustc_const_eval::interpret::terminator::<impl rustc_const_eval::interpret::eval_context::InterpCx<M>>::eval_terminator::hba334dc09862f71d
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_const_eval/src/interpret/terminator.rs:155:17
  26:     0x61bfedc5ddbf - rustc_const_eval::interpret::step::<impl rustc_const_eval::interpret::eval_context::InterpCx<M>>::terminator::h639b81d66f10b2c9
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_const_eval/src/interpret/step.rs:386:9
  27:     0x61bfedc5ddbf - rustc_const_eval::interpret::step::<impl rustc_const_eval::interpret::eval_context::InterpCx<M>>::step::h2b592d3f00082c27
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_const_eval/src/interpret/step.rs:51:9
  28:     0x61bfedc5ddbf - miri::concurrency::thread::EvalContextExt::run_threads::hb86c6885f8bf66e2
                               at /home/byt/Documents/miri/src/concurrency/thread.rs:1091:25
  29:     0x61bfedbb392e - miri::eval::eval_entry::{{closure}}::he88a88ce82ccab1c
                               at /home/byt/Documents/miri/src/eval.rs:444:49
  30:     0x61bfedbb392e - core::ops::function::FnOnce::call_once::hdd10e36d46e81399
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/core/src/ops/function.rs:250:5
  31:     0x61bfedbb392e - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h68c3644a5596ed4c
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/core/src/panic/unwind_safe.rs:272:9
  32:     0x61bfedbb392e - std::panicking::try::do_call::h77559fb42ca84572
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/panicking.rs:559:40
  33:     0x61bfedbb392e - std::panicking::try::h169daa5aab1c48d4
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/panicking.rs:523:19
  34:     0x61bfedbb392e - std::panic::catch_unwind::hd3d0ac27d0a92838
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/panic.rs:149:14
  35:     0x61bfedbb392e - miri::eval::eval_entry::h2a132248bf10826f
                               at /home/byt/Documents/miri/src/eval.rs:444:9
  36:     0x61bfedb38195 - <miri::MiriCompilerCalls as rustc_driver_impl::Callbacks>::after_analysis::{{closure}}::hd478a7dbce385ae2
                               at /home/byt/Documents/miri/src/bin/miri.rs:114:40
  37:     0x61bfedb38195 - rustc_middle::ty::context::GlobalCtxt::enter::{{closure}}::h8f53bfee5e6020bc
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_middle/src/ty/context.rs:771:37
  38:     0x61bfedb38195 - rustc_middle::ty::context::tls::enter_context::{{closure}}::hc966c30e58d09121
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_middle/src/ty/context/tls.rs:82:9
  39:     0x61bfedb38195 - std::thread::local::LocalKey<T>::try_with::h7efc6f94c1650c02
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/thread/local.rs:286:12
  40:     0x61bfedb38195 - std::thread::local::LocalKey<T>::with::hdcada936a74bb319
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/thread/local.rs:262:9
  41:     0x61bfedb38195 - rustc_middle::ty::context::tls::enter_context::h5c8bd00c8c036539
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_middle/src/ty/context/tls.rs:79:9
  42:     0x61bfedb38195 - rustc_middle::ty::context::GlobalCtxt::enter::hfed307f727e5b94c
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_middle/src/ty/context.rs:771:9
  43:     0x61bfedb30a09 - rustc_interface::queries::QueryResult<&rustc_middle::ty::context::GlobalCtxt>::enter::hca2c166b256af7a5
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/compiler/rustc_interface/src/queries.rs:70:9
  44:     0x61bfedb30a09 - <miri::MiriCompilerCalls as rustc_driver_impl::Callbacks>::after_analysis::h97ea44ba4d4030d4
                               at /home/byt/Documents/miri/src/bin/miri.rs:73:9
  45:     0x724e58c1de50 - rustc_interface[bdcbe09dbff9b323]::interface::run_compiler::<core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>, rustc_driver_impl[ef4cf2d5a728d441]::run_compiler::{closure#0}>::{closure#1}
  46:     0x724e58c0a409 - std[f233acd9da5dd114]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[bdcbe09dbff9b323]::util::run_in_thread_with_globals<rustc_interface[bdcbe09dbff9b323]::util::run_in_thread_pool_with_globals<rustc_interface[bdcbe09dbff9b323]::interface::run_compiler<core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>, rustc_driver_impl[ef4cf2d5a728d441]::run_compiler::{closure#0}>::{closure#1}, core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>>::{closure#0}, core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>>
  47:     0x724e58c0a1b8 - <<std[f233acd9da5dd114]::thread::Builder>::spawn_unchecked_<rustc_interface[bdcbe09dbff9b323]::util::run_in_thread_with_globals<rustc_interface[bdcbe09dbff9b323]::util::run_in_thread_pool_with_globals<rustc_interface[bdcbe09dbff9b323]::interface::run_compiler<core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>, rustc_driver_impl[ef4cf2d5a728d441]::run_compiler::{closure#0}>::{closure#1}, core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>>::{closure#0}, core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[322e9cc525b8dfd9]::result::Result<(), rustc_span[a23972874a281879]::ErrorGuaranteed>>::{closure#2} as core[322e9cc525b8dfd9]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  48:     0x724e53b997eb - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hbc52cdde05419ffd
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/alloc/src/boxed.rs:2022:9
  49:     0x724e53b997eb - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hd4f524b02355b43c
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/alloc/src/boxed.rs:2022:9
  50:     0x724e53b997eb - std::sys::pal::unix::thread::Thread::new::thread_start::habee50eacd885bc3
                               at /rustc/d568423a7a4ddb4b49323d96078a22f94df55fbd/library/std/src/sys/pal/unix/thread.rs:108:17
  51:     0x724e53694ac3 - start_thread
                               at ./nptl/pthread_create.c:442:8
  52:     0x724e53726850 - __GI___clone3
                               at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
  53:                0x0 - <unknown>

error: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/miri/issues/new

note: please make sure that you have updated to the latest nightly

note: please attach the file at `/home/byt/Documents/miri/rustc-ice-2024-05-07T05_08_40-172605.txt` to your bug report

query stack during panic:
end of query stack

Miri caused an ICE during evaluation. Here's the interpreter backtrace at the time of the panic:
note: the place in the program where the ICE was triggered
   --> ./tests/pass-dep/libc/libc-mem.rs:114:18
    |
114 |         let p2 = libc::realloc(ptr::null_mut(), 0);
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: BACKTRACE:
    = note: inside `test_malloc` at ./tests/pass-dep/libc/libc-mem.rs:114:18: 114:51
note: inside `main`
   --> ./tests/pass-dep/libc/libc-mem.rs:243:5
    |
243 |     test_malloc();
    |     ^^^^^^^^^^^^^
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:155:18: 155:21
    = note: inside closure at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:159:18: 159:82
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ops/function.rs:284:13: 284:31
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:559:40: 559:43
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:523:19: 523:88
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panic.rs:149:14: 149:33
    = note: inside closure at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:141:48: 141:73
    = note: inside `std::panicking::r#try::do_call::<{closure@std::rt::lang_start_internal::{closure#2}}, isize>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:559:40: 559:43
    = note: inside `std::panicking::r#try::<isize, {closure@std::rt::lang_start_internal::{closure#2}}>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:523:19: 523:88
    = note: inside `std::panic::catch_unwind::<{closure@std::rt::lang_start_internal::{closure#2}}, isize>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panic.rs:149:14: 149:33
    = note: inside `std::rt::lang_start_internal` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:141:20: 141:98
    = note: inside `std::rt::lang_start::<()>` at /home/byt/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:158:17: 163:6

error: test failed, to rerun pass `--test ui`

Caused by:
  process didn't exit successfully: `/home/byt/Documents/miri/target/debug/deps/ui-1decb19f575f371f --miri-run-dep-mode ./tests/pass-dep/libc/libc-mem.rs --edition=2021 --sysroot /home/byt/.cache/miri` (exit status: 1)
Error: command exited with non-zero code `cargo +miri --quiet test --manifest-path /home/byt/Documents/miri/Cargo.toml --test ui -- --miri-run-dep-mode ./tests/pass-dep/libc/libc-mem.rs --edition=2021 --sysroot /home/byt/.cache/miri`: 1

[RalfJung]

The null pointer has no AllocId, so yeah this is expected.

Why are you changing realloc at all? It doesn't seem to me like it has to change.

[tiif]

Why are you changing realloc at all? It doesn't seem to me like it has to change.

Yea, I don't think anything needs to be change in realloc. I just had a memory leak that I thought it was coming from the realloc implementation.

[tiif]

I wonder if we should do the same for posix_memalign? It is still returning a null pointer currently.

If size is 0, then the value placed in *memptr is either NULL or a unique pointer value. https://man7.org/linux/man-pages/man3/posix_memalign.3.html

[RalfJung]

Yeah, posix_memalign shod also return non-null on size 0. (But that can be a separate PR.)

One of the key parts of the PR will be two tests to ensure we detect double free and memory leaks of malloc(0).

[RalfJung]

https://github.com/rust-lang/miri/pull/3580 implements this for malloc; posix_memalign could get the same treatment as discussed above.