Deref coercion `&Box<[T]>` -> `&[T]` can be translated incorrectly #30478

petrochenkov · 2015-12-19T13:45:47Z

I wasn't able to come up with a minimal example yet, but this was the reason of segfault in #30095

Here's a patch fixing that segfault on 32 bit Linux - petrochenkov@b225678
I.e. segfaults disappear when the deref coercion is not used.

Update: Segfault reproduces on https://github.com/rust-lang/rust/tree/master if impl<T> Deref for P<[T]> is modified to use coercion (fn deref(&self) -> &[T] { &self.ptr }).

The text was updated successfully, but these errors were encountered:

eddyb · 2015-12-19T16:24:55Z

cc @dotdash

dotdash · 2015-12-23T17:31:53Z

@petrochenkov I can't reproduce the segfault with that change on a current master. Do you know how far back I need to go?

petrochenkov · 2015-12-23T18:10:43Z

@dotdash
It still happens on the current master (4ce1daf).
32-bit Kubuntu, ./configure without parameters.

I believe it also happens with some debug turned on ./configure --enable-debug --enable-optimize --disable-debug-assertions, but I must recheck. Update: yes, it does.

dotdash · 2015-12-23T22:43:24Z

@petrochenkov Ah, great, fails for me on a 32bit build as well.

petrochenkov · 2015-12-27T15:37:31Z

Relevant fragments of valgrind report:

==8423== Conditional jump or move depends on uninitialised value(s)
==8423==    at 0x5B7352B: Box$LT$$u5b$rustc_front..hir..TyParam$u5d$$GT$::drop.75375::hd3aca454c9de642c (in /home/we/rust/i686-unknown-linux-gnu/stage2/lib/librustc-ca1c970e.so)
==8423==    by 0x5CB0C2A: rebuild_ty_params (error_reporting.rs:1156)
==8423==    by 0x5CB0C2A: middle::infer::error_reporting::Rebuilder$LT$$u27$a$C$$u20$$u27$tcx$GT$::rebuild::hf3a9c08db66d929dZ4v (error_reporting.rs:1046)
==8423==    by 0x5CADF72: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::give_suggestion::h0e0cc7b89f139e9fpZv (error_reporting.rs:970)
==8423==    by 0x5CA5437: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_processed_errors::haab1aef5dc61e100zYv (error_reporting.rs:911)
==8423==    by 0x5C7C66B: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_region_errors::hb7a3d90d445d6d92B7u (error_reporting.rs:315)
==8423==    by 0x5C7BF66: middle::infer::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$::resolve_regions_and_report_errors::h0b6c938cc42e8c7bs6D (mod.rs:1146)
==8423==    by 0x58DE95B: check::compare_method::compare_impl_method::hcd62d152136c9f4dRmm (compare_method.rs:365)
==8423==    by 0x58BB296: check_impl_items_against_trait (mod.rs:909)
==8423==    by 0x58BB296: check::check_item_type::h3ed5e7601540256ajTo (mod.rs:674)
==8423==    by 0x58B8FE8: check::CheckItemTypesVisitor$LT$$u27$a$C$$u20$$u27$tcx$GT$.Visitor$LT$$u27$tcx$GT$::visit_item::hd68843fd129f0e32Fwo (mod.rs:362)
==8423==    by 0x58B5D80: visit_all_items<rustc_typeck::check::CheckItemTypesVisitor> (hir.rs:441)
==8423==    by 0x58B5D80: fnfn (mod.rs:396)
==8423==    by 0x58B5D80: abort_if_new_errors<closure> (mod.rs:127)
==8423==    by 0x58B5D80: check::check_item_types::h1619773917681f71Nyo (mod.rs:393)
==8423==    by 0x58AB48F: fnfn (lib.rs:351)
==8423==    by 0x58AB48F: time<(),closure> (common.rs:39)
==8423==    by 0x58AB48F: check_crate::h4b677714f869c15cfWC (lib.rs:350)
==8423==    by 0x4897BEC: driver::phase_3_run_analysis_passes::_$LT$closure$GT$::closure.26362 (driver.rs:753)
==8423==  Uninitialised value was created by a stack allocation
==8423==    at 0x5CAEC56: middle::infer::error_reporting::Rebuilder$LT$$u27$a$C$$u20$$u27$tcx$GT$::rebuild::hf3a9c08db66d929dZ4v (error_reporting.rs:1024)
==8423== 
==8423== Conditional jump or move depends on uninitialised value(s)
==8423==    at 0x5B73536: Box$LT$$u5b$rustc_front..hir..TyParam$u5d$$GT$::drop.75375::hd3aca454c9de642c (in /home/we/rust/i686-unknown-linux-gnu/stage2/lib/librustc-ca1c970e.so)
==8423==    by 0x5CB0C2A: rebuild_ty_params (error_reporting.rs:1156)
==8423==    by 0x5CB0C2A: middle::infer::error_reporting::Rebuilder$LT$$u27$a$C$$u20$$u27$tcx$GT$::rebuild::hf3a9c08db66d929dZ4v (error_reporting.rs:1046)
==8423==    by 0x5CADF72: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::give_suggestion::h0e0cc7b89f139e9fpZv (error_reporting.rs:970)
==8423==    by 0x5CA5437: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_processed_errors::haab1aef5dc61e100zYv (error_reporting.rs:911)
==8423==    by 0x5C7C66B: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_region_errors::hb7a3d90d445d6d92B7u (error_reporting.rs:315)
==8423==    by 0x5C7BF66: middle::infer::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$::resolve_regions_and_report_errors::h0b6c938cc42e8c7bs6D (mod.rs:1146)
==8423==    by 0x58DE95B: check::compare_method::compare_impl_method::hcd62d152136c9f4dRmm (compare_method.rs:365)
==8423==    by 0x58BB296: check_impl_items_against_trait (mod.rs:909)
==8423==    by 0x58BB296: check::check_item_type::h3ed5e7601540256ajTo (mod.rs:674)
==8423==    by 0x58B8FE8: check::CheckItemTypesVisitor$LT$$u27$a$C$$u20$$u27$tcx$GT$.Visitor$LT$$u27$tcx$GT$::visit_item::hd68843fd129f0e32Fwo (mod.rs:362)
==8423==    by 0x58B5D80: visit_all_items<rustc_typeck::check::CheckItemTypesVisitor> (hir.rs:441)
==8423==    by 0x58B5D80: fnfn (mod.rs:396)
==8423==    by 0x58B5D80: abort_if_new_errors<closure> (mod.rs:127)
==8423==    by 0x58B5D80: check::check_item_types::h1619773917681f71Nyo (mod.rs:393)
==8423==    by 0x58AB48F: fnfn (lib.rs:351)
==8423==    by 0x58AB48F: time<(),closure> (common.rs:39)
==8423==    by 0x58AB48F: check_crate::h4b677714f869c15cfWC (lib.rs:350)
==8423==    by 0x4897BEC: driver::phase_3_run_analysis_passes::_$LT$closure$GT$::closure.26362 (driver.rs:753)
==8423==  Uninitialised value was created by a stack allocation
==8423==    at 0x5CAEC56: middle::infer::error_reporting::Rebuilder$LT$$u27$a$C$$u20$$u27$tcx$GT$::rebuild::hf3a9c08db66d929dZ4v (error_reporting.rs:1024)
==8423== 
==8423== Use of uninitialised value of size 4
==8423==    at 0x5B73550: Box$LT$$u5b$rustc_front..hir..TyParam$u5d$$GT$::drop.75375::hd3aca454c9de642c (in /home/we/rust/i686-unknown-linux-gnu/stage2/lib/librustc-ca1c970e.so)
==8423==    by 0x5CB0C2A: rebuild_ty_params (error_reporting.rs:1156)
==8423==    by 0x5CB0C2A: middle::infer::error_reporting::Rebuilder$LT$$u27$a$C$$u20$$u27$tcx$GT$::rebuild::hf3a9c08db66d929dZ4v (error_reporting.rs:1046)
==8423==    by 0x5CADF72: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::give_suggestion::h0e0cc7b89f139e9fpZv (error_reporting.rs:970)
==8423==    by 0x5CA5437: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_processed_errors::haab1aef5dc61e100zYv (error_reporting.rs:911)
==8423==    by 0x5C7C66B: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_region_errors::hb7a3d90d445d6d92B7u (error_reporting.rs:315)
==8423==    by 0x5C7BF66: middle::infer::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$::resolve_regions_and_report_errors::h0b6c938cc42e8c7bs6D (mod.rs:1146)
==8423==    by 0x58DE95B: check::compare_method::compare_impl_method::hcd62d152136c9f4dRmm (compare_method.rs:365)
==8423==    by 0x58BB296: check_impl_items_against_trait (mod.rs:909)
==8423==    by 0x58BB296: check::check_item_type::h3ed5e7601540256ajTo (mod.rs:674)
==8423==    by 0x58B8FE8: check::CheckItemTypesVisitor$LT$$u27$a$C$$u20$$u27$tcx$GT$.Visitor$LT$$u27$tcx$GT$::visit_item::hd68843fd129f0e32Fwo (mod.rs:362)
==8423==    by 0x58B5D80: visit_all_items<rustc_typeck::check::CheckItemTypesVisitor> (hir.rs:441)
==8423==    by 0x58B5D80: fnfn (mod.rs:396)
==8423==    by 0x58B5D80: abort_if_new_errors<closure> (mod.rs:127)
==8423==    by 0x58B5D80: check::check_item_types::h1619773917681f71Nyo (mod.rs:393)
==8423==    by 0x58AB48F: fnfn (lib.rs:351)
==8423==    by 0x58AB48F: time<(),closure> (common.rs:39)
==8423==    by 0x58AB48F: check_crate::h4b677714f869c15cfWC (lib.rs:350)
==8423==    by 0x4897BEC: driver::phase_3_run_analysis_passes::_$LT$closure$GT$::closure.26362 (driver.rs:753)
==8423==  Uninitialised value was created by a stack allocation
==8423==    at 0x5CAEC56: middle::infer::error_reporting::Rebuilder$LT$$u27$a$C$$u20$$u27$tcx$GT$::rebuild::hf3a9c08db66d929dZ4v (error_reporting.rs:1024)
==8423== 
==8423== Invalid read of size 4
==8423==    at 0x5B73550: Box$LT$$u5b$rustc_front..hir..TyParam$u5d$$GT$::drop.75375::hd3aca454c9de642c (in /home/we/rust/i686-unknown-linux-gnu/stage2/lib/librustc-ca1c970e.so)
==8423==    by 0x5CB0C2A: rebuild_ty_params (error_reporting.rs:1156)
==8423==    by 0x5CB0C2A: middle::infer::error_reporting::Rebuilder$LT$$u27$a$C$$u20$$u27$tcx$GT$::rebuild::hf3a9c08db66d929dZ4v (error_reporting.rs:1046)
==8423==    by 0x5CADF72: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::give_suggestion::h0e0cc7b89f139e9fpZv (error_reporting.rs:970)
==8423==    by 0x5CA5437: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_processed_errors::haab1aef5dc61e100zYv (error_reporting.rs:911)
==8423==    by 0x5C7C66B: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_region_errors::hb7a3d90d445d6d92B7u (error_reporting.rs:315)
==8423==    by 0x5C7BF66: middle::infer::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$::resolve_regions_and_report_errors::h0b6c938cc42e8c7bs6D (mod.rs:1146)
==8423==    by 0x58DE95B: check::compare_method::compare_impl_method::hcd62d152136c9f4dRmm (compare_method.rs:365)
==8423==    by 0x58BB296: check_impl_items_against_trait (mod.rs:909)
==8423==    by 0x58BB296: check::check_item_type::h3ed5e7601540256ajTo (mod.rs:674)
==8423==    by 0x58B8FE8: check::CheckItemTypesVisitor$LT$$u27$a$C$$u20$$u27$tcx$GT$.Visitor$LT$$u27$tcx$GT$::visit_item::hd68843fd129f0e32Fwo (mod.rs:362)
==8423==    by 0x58B5D80: visit_all_items<rustc_typeck::check::CheckItemTypesVisitor> (hir.rs:441)
==8423==    by 0x58B5D80: fnfn (mod.rs:396)
==8423==    by 0x58B5D80: abort_if_new_errors<closure> (mod.rs:127)
==8423==    by 0x58B5D80: check::check_item_types::h1619773917681f71Nyo (mod.rs:393)
==8423==    by 0x58AB48F: fnfn (lib.rs:351)
==8423==    by 0x58AB48F: time<(),closure> (common.rs:39)
==8423==    by 0x58AB48F: check_crate::h4b677714f869c15cfWC (lib.rs:350)
==8423==    by 0x4897BEC: driver::phase_3_run_analysis_passes::_$LT$closure$GT$::closure.26362 (driver.rs:753)
==8423==  Address 0x67 is not stack'd, malloc'd or (recently) free'd
==8423== 
==8423== 
==8423== Process terminating with default action of signal 11 (SIGSEGV)
==8423==  General Protection Fault
==8423==    at 0x4A9F64B: sys::stack_overflow::imp::signal_handler::he4ed13eea9571052pbx (stack_overflow.rs:108)
==8423==    by 0x4E0669F: ??? (in /lib/i386-linux-gnu/libc-2.19.so)
==8423==    by 0x5CADF72: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::give_suggestion::h0e0cc7b89f139e9fpZv (error_reporting.rs:970)
==8423==    by 0x5CA5437: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_processed_errors::haab1aef5dc61e100zYv (error_reporting.rs:911)
==8423==    by 0x5C7C66B: middle::infer::error_reporting::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$.ErrorReporting$LT$$u27$tcx$GT$::report_region_errors::hb7a3d90d445d6d92B7u (error_reporting.rs:315)
==8423==    by 0x5C7BF66: middle::infer::InferCtxt$LT$$u27$a$C$$u20$$u27$tcx$GT$::resolve_regions_and_report_errors::h0b6c938cc42e8c7bs6D (mod.rs:1146)
==8423==    by 0x58DE95B: check::compare_method::compare_impl_method::hcd62d152136c9f4dRmm (compare_method.rs:365)
==8423==    by 0x58BB296: check_impl_items_against_trait (mod.rs:909)
==8423==    by 0x58BB296: check::check_item_type::h3ed5e7601540256ajTo (mod.rs:674)
==8423==    by 0x58B8FE8: check::CheckItemTypesVisitor$LT$$u27$a$C$$u20$$u27$tcx$GT$.Visitor$LT$$u27$tcx$GT$::visit_item::hd68843fd129f0e32Fwo (mod.rs:362)
==8423==    by 0x58B5D80: visit_all_items<rustc_typeck::check::CheckItemTypesVisitor> (hir.rs:441)
==8423==    by 0x58B5D80: fnfn (mod.rs:396)
==8423==    by 0x58B5D80: abort_if_new_errors<closure> (mod.rs:127)
==8423==    by 0x58B5D80: check::check_item_types::h1619773917681f71Nyo (mod.rs:393)
==8423==    by 0x58AB48F: fnfn (lib.rs:351)
==8423==    by 0x58AB48F: time<(),closure> (common.rs:39)
==8423==    by 0x58AB48F: check_crate::h4b677714f869c15cfWC (lib.rs:350)
==8423==    by 0x4897BEC: driver::phase_3_run_analysis_passes::_$LT$closure$GT$::closure.26362 (driver.rs:753)

dotdash · 2015-12-27T15:55:37Z

This is caused by a bad call to a lifetime.end intrinsic marking memory as dead even though it's still alive. It's caused by auto_ref returning an Rvalue datum for the fat pointer even though that datum doesn't actually own the fat pointer. In other places, e.g. trans_field we return an Lvalue for similar operations for exactly that reason. I guess using an Rvalue is a kind of micro-optimization to avoid the extra alloca for the pointer, and for regular pointers there's no problem because they are passed by value. I think I'll go ahead and fix this by returning an Lvalue or Rvalue datum depending on the sizedness of the type. MIR trans should not expose this problem due to explicit handling of fat pointers.

`auto_ref()` currently returns an Rvalue datum for the ref'd value, which is fine for thin pointers, but for fat pointers this means that once the pointer is moved out of that datum, its memory will be marked as dead. And because there is not necessarily an intermediate temporary involved we can end up marking memory as dead that is actually still used. As I don't want to break the micro-optimization for thin pointers by always returning an Lvalue datum, I decided to only do so for fat pointers. Fix rust-lang#30478

`auto_ref()` currently returns an Rvalue datum for the ref'd value, which is fine for thin pointers, but for fat pointers this means that once the pointer is moved out of that datum, its memory will be marked as dead. And because there is not necessarily an intermediate temporary involved we can end up marking memory as dead that is actually still used. As I don't want to break the micro-optimization for thin pointers by always returning an Lvalue datum, I decided to only do so for fat pointers. Fix #30478

petrochenkov · 2015-12-28T22:21:05Z

@dotdash
This still segfaults on current master (27a1834)

dotdash · 2015-12-29T13:29:48Z

@petrochenkov probably in stage0? That's built with the snapshot compiler which doesn't have the fix yet. I tried it out by building stage 0 with the fix but without the change to libsyntax, and once it got past libsyntax, I made the change to it so stage1 and stage2 were built with it, and both succeeded.

dotdash · 2015-12-29T13:31:09Z

@petrochenkov well, the segfault happens in stage1 building libcore, but I guess you get what I meant.

petrochenkov · 2015-12-29T13:46:02Z

@dotdash
That's right. I just looked at the make check results and reported without much thinking, sorry.

petrochenkov mentioned this issue Dec 20, 2015

Improve OwnedSlice and use it in HIR (take 2) #30470

Merged

dotdash mentioned this issue Dec 27, 2015

Fix auto_ref() for fat pointers #30579

Merged

bors closed this as completed in #30579 Dec 28, 2015

petrochenkov mentioned this issue Feb 10, 2016

Implement CommandExt::{exec, before_exec} #31409

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deref coercion `&Box<[T]>` -> `&[T]` can be translated incorrectly #30478

Deref coercion `&Box<[T]>` -> `&[T]` can be translated incorrectly #30478

petrochenkov commented Dec 19, 2015

eddyb commented Dec 19, 2015

dotdash commented Dec 23, 2015

petrochenkov commented Dec 23, 2015

dotdash commented Dec 23, 2015

petrochenkov commented Dec 27, 2015

dotdash commented Dec 27, 2015

petrochenkov commented Dec 28, 2015

dotdash commented Dec 29, 2015

dotdash commented Dec 29, 2015

petrochenkov commented Dec 29, 2015

Deref coercion &Box<[T]> -> &[T] can be translated incorrectly #30478

Deref coercion &Box<[T]> -> &[T] can be translated incorrectly #30478

Comments

petrochenkov commented Dec 19, 2015

eddyb commented Dec 19, 2015

dotdash commented Dec 23, 2015

petrochenkov commented Dec 23, 2015

dotdash commented Dec 23, 2015

petrochenkov commented Dec 27, 2015

dotdash commented Dec 27, 2015

petrochenkov commented Dec 28, 2015

dotdash commented Dec 29, 2015

dotdash commented Dec 29, 2015

petrochenkov commented Dec 29, 2015

Deref coercion `&Box<[T]>` -> `&[T]` can be translated incorrectly #30478

Deref coercion `&Box<[T]>` -> `&[T]` can be translated incorrectly #30478