From a2f938ac52d8d1b3ee1cf94f18705e95f28ca0b8 Mon Sep 17 00:00:00 2001 From: oli Date: Thu, 19 Nov 2020 11:05:15 +0000 Subject: [PATCH 01/10] Document that heap allocations are not guaranteed to happen, even if explicitly performed in the code --- library/core/src/alloc/global.rs | 13 +++++++++++++ library/core/src/alloc/mod.rs | 12 ++++++++++++ 2 files changed, 25 insertions(+) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index c198797e650f6..65406125a3dbc 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -53,6 +53,19 @@ use crate::ptr; /// * `Layout` queries and calculations in general must be correct. Callers of /// this trait are allowed to rely on the contracts defined on each method, /// and implementors must ensure such contracts remain true. +/// +/// * You may not rely on allocations actually happening, even if there are explicit +/// heap allocations in the source. The optimizer may detect allocation/deallocation +/// pairs that it can instead move to stack allocations/deallocations and thus never +/// invoke the allocator here. +/// More concretely, the following code example is unsound, irrespective of whether your +/// custom allocator allows counting how many allocations have happened. +/// +/// ```rust,ignore +/// drop(Box::new(42)); +/// let number_of_heap_allocs = /* call private allocator API */; +/// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } +/// ``` #[stable(feature = "global_alloc", since = "1.28.0")] pub unsafe trait GlobalAlloc { /// Allocate memory as described by the given `layout`. diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index c61c19cc7d1d1..47d5ad9ad5609 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -94,6 +94,18 @@ pub unsafe trait AllocRef { /// The returned block may have a larger size than specified by `layout.size()`, and may or may /// not have its contents initialized. /// + /// Note that you may not rely on this method actually getting called, even if there are calls + /// to it in the source. The optimizer may detect allocation/deallocation pairs that it can + /// instead move to stack allocations/deallocations and thus never invoke the allocator here. + /// More concretely, the following code example is unsound, irrespective of whether your + /// custom allocator allows counting how many allocations have happened. + /// + /// ```rust,ignore + /// Global::dealloc(Global::alloc(some_layout)); + /// let number_of_heap_allocs = /* call private allocator API */; + /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } + /// ``` + /// /// # Errors /// /// Returning `Err` indicates that either memory is exhausted or `layout` does not meet From 173892c7765365ca47f920771ee7f743e2c4dfaa Mon Sep 17 00:00:00 2001 From: oli Date: Thu, 19 Nov 2020 11:09:10 +0000 Subject: [PATCH 02/10] Note that there are other optimizations than the one showcased --- library/core/src/alloc/global.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index 65406125a3dbc..e6fe1e89c1071 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -66,6 +66,12 @@ use crate::ptr; /// let number_of_heap_allocs = /* call private allocator API */; /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } /// ``` +/// +/// Note that allocation/deallocation pairs being moved to the stack is not the only +/// optimization that can be applied. You may generally not rely on heap allocations +/// happening, if they can be removed without changing program behaviour. +/// Whether allocations happen or not is not part of the program behaviour, even if it +/// could be detected via an allocator that tracks allocations. #[stable(feature = "global_alloc", since = "1.28.0")] pub unsafe trait GlobalAlloc { /// Allocate memory as described by the given `layout`. From 3d1f6769066978946d381ca77e82e48b652bad65 Mon Sep 17 00:00:00 2001 From: oli Date: Thu, 19 Nov 2020 11:41:47 +0000 Subject: [PATCH 03/10] Fix tidy --- library/core/src/alloc/global.rs | 2 +- library/core/src/alloc/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index e6fe1e89c1071..4922028eee2f2 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -61,7 +61,7 @@ use crate::ptr; /// More concretely, the following code example is unsound, irrespective of whether your /// custom allocator allows counting how many allocations have happened. /// -/// ```rust,ignore +/// ```text /// drop(Box::new(42)); /// let number_of_heap_allocs = /* call private allocator API */; /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index 47d5ad9ad5609..d174842bcbf0f 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -100,7 +100,7 @@ pub unsafe trait AllocRef { /// More concretely, the following code example is unsound, irrespective of whether your /// custom allocator allows counting how many allocations have happened. /// - /// ```rust,ignore + /// ```text /// Global::dealloc(Global::alloc(some_layout)); /// let number_of_heap_allocs = /* call private allocator API */; /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } From dfca61a4c22d3cd1bc6b7a493de6f6763223b71f Mon Sep 17 00:00:00 2001 From: oli Date: Thu, 19 Nov 2020 14:28:28 +0000 Subject: [PATCH 04/10] Elaborate on side effects --- library/core/src/alloc/global.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index 4922028eee2f2..89d85bf291e05 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -71,7 +71,8 @@ use crate::ptr; /// optimization that can be applied. You may generally not rely on heap allocations /// happening, if they can be removed without changing program behaviour. /// Whether allocations happen or not is not part of the program behaviour, even if it -/// could be detected via an allocator that tracks allocations. +/// could be detected via an allocator that tracks allocations by printing or otherwise +/// having side effects. #[stable(feature = "global_alloc", since = "1.28.0")] pub unsafe trait GlobalAlloc { /// Allocate memory as described by the given `layout`. From 58d62b83711b5f1401509b97a1340495f9907134 Mon Sep 17 00:00:00 2001 From: oli Date: Thu, 19 Nov 2020 14:57:30 +0000 Subject: [PATCH 05/10] Inform tidy about the reason for the ignored rust code --- library/core/src/alloc/global.rs | 2 +- library/core/src/alloc/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index 89d85bf291e05..1ecf6abc7546f 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -61,7 +61,7 @@ use crate::ptr; /// More concretely, the following code example is unsound, irrespective of whether your /// custom allocator allows counting how many allocations have happened. /// -/// ```text +/// ```rust,ignore (unsound and has placeholders) /// drop(Box::new(42)); /// let number_of_heap_allocs = /* call private allocator API */; /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index d174842bcbf0f..1616d8fab212c 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -100,7 +100,7 @@ pub unsafe trait AllocRef { /// More concretely, the following code example is unsound, irrespective of whether your /// custom allocator allows counting how many allocations have happened. /// - /// ```text + /// ```rust,ignore (unsound and has placeholders) /// Global::dealloc(Global::alloc(some_layout)); /// let number_of_heap_allocs = /* call private allocator API */; /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } From fda4c8d5c188da56d8f25b562a4a25d422179a87 Mon Sep 17 00:00:00 2001 From: oli Date: Fri, 4 Dec 2020 17:28:22 +0000 Subject: [PATCH 06/10] Update documentation to review comments --- library/core/src/alloc/global.rs | 15 +++++++++------ library/core/src/alloc/mod.rs | 14 ++++++++++++-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index 1ecf6abc7546f..6549ea1a12411 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -55,9 +55,12 @@ use crate::ptr; /// and implementors must ensure such contracts remain true. /// /// * You may not rely on allocations actually happening, even if there are explicit -/// heap allocations in the source. The optimizer may detect allocation/deallocation -/// pairs that it can instead move to stack allocations/deallocations and thus never -/// invoke the allocator here. +/// heap allocations in the source. +/// The optimizer may detect unused allocations that it can either +/// eliminate entirely or move to the stack and thus never invoke the allocator here. The +/// optimizer may further assume that allocation is infallible, so code that used to fail due +/// to allocator failures may now suddenly work because the optimizer worked around the +/// need for an allocation. /// More concretely, the following code example is unsound, irrespective of whether your /// custom allocator allows counting how many allocations have happened. /// @@ -67,10 +70,10 @@ use crate::ptr; /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } /// ``` /// -/// Note that allocation/deallocation pairs being moved to the stack is not the only +/// Note that the optimizations mentioned above are not the only /// optimization that can be applied. You may generally not rely on heap allocations -/// happening, if they can be removed without changing program behaviour. -/// Whether allocations happen or not is not part of the program behaviour, even if it +/// happening if they can be removed without changing program behavior. +/// Whether allocations happen or not is not part of the program behavior, even if it /// could be detected via an allocator that tracks allocations by printing or otherwise /// having side effects. #[stable(feature = "global_alloc", since = "1.28.0")] diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index 1616d8fab212c..fc89046bc427b 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -95,8 +95,11 @@ pub unsafe trait AllocRef { /// not have its contents initialized. /// /// Note that you may not rely on this method actually getting called, even if there are calls - /// to it in the source. The optimizer may detect allocation/deallocation pairs that it can - /// instead move to stack allocations/deallocations and thus never invoke the allocator here. + /// to it in the source. The optimizer may detect unused allocations that it can either + /// eliminate entirely or move to the stack and thus never invoke the allocator here. The + /// optimizer may further assume that allocation is infallible, so code that used to fail due + /// to allocator failures may now suddenly work because the optimizer worked around the + /// need for an allocation. /// More concretely, the following code example is unsound, irrespective of whether your /// custom allocator allows counting how many allocations have happened. /// @@ -106,6 +109,13 @@ pub unsafe trait AllocRef { /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } /// ``` /// + /// Note that the optimizations mentioned above are not the only + /// optimization that can be applied. You may generally not rely on heap allocations + /// happening if they can be removed without changing program behavior. + /// Whether allocations happen or not is not part of the program behavior, even if it + /// could be detected via an allocator that tracks allocations by printing or otherwise + /// having side effects. + /// /// # Errors /// /// Returning `Err` indicates that either memory is exhausted or `layout` does not meet From 48c8ff59ecc01060532b43c2fb27f9bc7a3d8fcd Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Sat, 26 Dec 2020 18:05:55 +0100 Subject: [PATCH 07/10] Update library/core/src/alloc/global.rs Co-authored-by: Ralf Jung --- library/core/src/alloc/global.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index 6549ea1a12411..c1bd896bdbcae 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -57,7 +57,7 @@ use crate::ptr; /// * You may not rely on allocations actually happening, even if there are explicit /// heap allocations in the source. /// The optimizer may detect unused allocations that it can either -/// eliminate entirely or move to the stack and thus never invoke the allocator here. The +/// eliminate entirely or move to the stack and thus never invoke the allocator. The /// optimizer may further assume that allocation is infallible, so code that used to fail due /// to allocator failures may now suddenly work because the optimizer worked around the /// need for an allocation. From 714feab05993c35e971a307191ee662932df1ee5 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Sat, 26 Dec 2020 18:06:04 +0100 Subject: [PATCH 08/10] Update library/core/src/alloc/mod.rs Co-authored-by: Ralf Jung --- library/core/src/alloc/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index fc89046bc427b..4aa166e6aa15b 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -96,7 +96,7 @@ pub unsafe trait AllocRef { /// /// Note that you may not rely on this method actually getting called, even if there are calls /// to it in the source. The optimizer may detect unused allocations that it can either - /// eliminate entirely or move to the stack and thus never invoke the allocator here. The + /// eliminate entirely or move to the stack and thus never invoke the allocator. The /// optimizer may further assume that allocation is infallible, so code that used to fail due /// to allocator failures may now suddenly work because the optimizer worked around the /// need for an allocation. From fba17e3f8d00acf67f1fe86590c4e9c0671dd2bc Mon Sep 17 00:00:00 2001 From: oli Date: Sat, 26 Dec 2020 17:14:49 +0000 Subject: [PATCH 09/10] Adjust markdown text to be more like the rendered text --- library/core/src/alloc/global.rs | 8 +++----- library/core/src/alloc/mod.rs | 5 ++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/library/core/src/alloc/global.rs b/library/core/src/alloc/global.rs index c1bd896bdbcae..6ec0f0b5ffc5b 100644 --- a/library/core/src/alloc/global.rs +++ b/library/core/src/alloc/global.rs @@ -55,14 +55,12 @@ use crate::ptr; /// and implementors must ensure such contracts remain true. /// /// * You may not rely on allocations actually happening, even if there are explicit -/// heap allocations in the source. -/// The optimizer may detect unused allocations that it can either +/// heap allocations in the source. The optimizer may detect unused allocations that it can either /// eliminate entirely or move to the stack and thus never invoke the allocator. The /// optimizer may further assume that allocation is infallible, so code that used to fail due /// to allocator failures may now suddenly work because the optimizer worked around the -/// need for an allocation. -/// More concretely, the following code example is unsound, irrespective of whether your -/// custom allocator allows counting how many allocations have happened. +/// need for an allocation. More concretely, the following code example is unsound, irrespective +/// of whether your custom allocator allows counting how many allocations have happened. /// /// ```rust,ignore (unsound and has placeholders) /// drop(Box::new(42)); diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index 4aa166e6aa15b..9c6e57381834b 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -99,9 +99,8 @@ pub unsafe trait AllocRef { /// eliminate entirely or move to the stack and thus never invoke the allocator. The /// optimizer may further assume that allocation is infallible, so code that used to fail due /// to allocator failures may now suddenly work because the optimizer worked around the - /// need for an allocation. - /// More concretely, the following code example is unsound, irrespective of whether your - /// custom allocator allows counting how many allocations have happened. + /// need for an allocation. More concretely, the following code example is unsound, irrespective + /// of whether your custom allocator allows counting how many allocations have happened. /// /// ```rust,ignore (unsound and has placeholders) /// Global::dealloc(Global::alloc(some_layout)); From efcd8a96c469bf7b9eb3bb302217d7b9fa749968 Mon Sep 17 00:00:00 2001 From: oli Date: Sat, 26 Dec 2020 17:16:50 +0000 Subject: [PATCH 10/10] DIrect invocations of `AllocRef::alloc` cannot get optimized away --- library/core/src/alloc/mod.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/library/core/src/alloc/mod.rs b/library/core/src/alloc/mod.rs index 9c6e57381834b..c61c19cc7d1d1 100644 --- a/library/core/src/alloc/mod.rs +++ b/library/core/src/alloc/mod.rs @@ -94,27 +94,6 @@ pub unsafe trait AllocRef { /// The returned block may have a larger size than specified by `layout.size()`, and may or may /// not have its contents initialized. /// - /// Note that you may not rely on this method actually getting called, even if there are calls - /// to it in the source. The optimizer may detect unused allocations that it can either - /// eliminate entirely or move to the stack and thus never invoke the allocator. The - /// optimizer may further assume that allocation is infallible, so code that used to fail due - /// to allocator failures may now suddenly work because the optimizer worked around the - /// need for an allocation. More concretely, the following code example is unsound, irrespective - /// of whether your custom allocator allows counting how many allocations have happened. - /// - /// ```rust,ignore (unsound and has placeholders) - /// Global::dealloc(Global::alloc(some_layout)); - /// let number_of_heap_allocs = /* call private allocator API */; - /// unsafe { std::intrinsics::assume(number_of_heap_allocs > 0); } - /// ``` - /// - /// Note that the optimizations mentioned above are not the only - /// optimization that can be applied. You may generally not rely on heap allocations - /// happening if they can be removed without changing program behavior. - /// Whether allocations happen or not is not part of the program behavior, even if it - /// could be detected via an allocator that tracks allocations by printing or otherwise - /// having side effects. - /// /// # Errors /// /// Returning `Err` indicates that either memory is exhausted or `layout` does not meet