Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove common usage pattern from AllocRef #69026

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

TimDiekmann
Copy link
Member

This removes the common usage patterns from AllocRef:

  • alloc_one
  • dealloc_one
  • alloc_array
  • realloc_array
  • dealloc_array

Actually, they add nothing to AllocRef except a convenience wrapper around Layout and other methods in this trait but have a major flaw: The documentation of AllocRefs notes, that

some higher-level allocation methods (alloc_one, alloc_array) are well-defined on zero-sized types and can optionally support them: it is left up to the implementor whether to return Err, or to return Ok with some pointer.

With the current API, GlobalAlloc does not have those methods, so they cannot be overridden for liballoc::Global, which means that even if the global allocator would support zero-sized allocations, alloc_one, alloc_array, and realloc_array for liballoc::Global will error, while calling alloc with a zeroed-size Layout could succeed. Even worse: allocating with alloc and deallocating with dealloc_{one,array} could end up with not calling dealloc at all!

For the full discussion please see rust-lang/wg-allocators#18

r? @Amanieu

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 10, 2020
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-02-10T16:05:30.6632003Z ========================== Starting Command Output ===========================
2020-02-10T16:05:30.6633548Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/3b66a497-b04f-496b-af51-4e3bde2fda90.sh
2020-02-10T16:05:30.6633582Z 
2020-02-10T16:05:30.6636119Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-10T16:05:30.6642208Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69026/merge to s
2020-02-10T16:05:30.6644380Z Task         : Get sources
2020-02-10T16:05:30.6644419Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-10T16:05:30.6644458Z Version      : 1.0.0
2020-02-10T16:05:30.6644494Z Author       : Microsoft
---
2020-02-10T16:05:35.2873998Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-10T16:05:36.1398528Z ##[command]git config gc.auto 0
2020-02-10T16:05:36.1401221Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-10T16:05:36.1403176Z ##[command]git config --get-all http.proxy
2020-02-10T16:05:36.1409588Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/69026/merge:refs/remotes/pull/69026/merge
---
2020-02-10T17:01:14.0487222Z .................................................................................................... 1700/9625
2020-02-10T17:01:18.7723188Z .................................................................................................... 1800/9625
2020-02-10T17:01:30.7991843Z .............................i...................................................................... 1900/9625
2020-02-10T17:01:37.7753458Z .................................................................................................... 2000/9625
2020-02-10T17:01:51.5053430Z ...................iiiii............................................................................ 2100/9625
2020-02-10T17:02:01.0138286Z .................................................................................................... 2300/9625
2020-02-10T17:02:03.3851411Z .................................................................................................... 2400/9625
2020-02-10T17:02:08.2450180Z .................................................................................................... 2500/9625
2020-02-10T17:02:28.5551559Z .................................................................................................... 2600/9625
---
2020-02-10T17:05:03.1068492Z ......................................................................i...............i............. 4900/9625
2020-02-10T17:05:10.7887546Z .................................................................................................... 5000/9625
2020-02-10T17:05:18.4709186Z .................................................................................................... 5100/9625
2020-02-10T17:05:23.0998897Z ............i....................................................................................... 5200/9625
2020-02-10T17:05:33.7535099Z ......................................................................................ii.ii........i 5300/9625
2020-02-10T17:05:41.3716200Z ........................i........................................................................... 5500/9625
2020-02-10T17:05:49.2799703Z .................................................................................................... 5600/9625
2020-02-10T17:05:57.2278826Z ..........................................................................i......................... 5700/9625
2020-02-10T17:06:04.4438677Z .................................................................................................... 5800/9625
2020-02-10T17:06:04.4438677Z .................................................................................................... 5800/9625
2020-02-10T17:06:10.6403262Z .................................................................................................... 5900/9625
2020-02-10T17:06:20.6025900Z ..................................................................ii...i..ii...........i............ 6000/9625
2020-02-10T17:06:41.4128719Z .................................................................................................... 6200/9625
2020-02-10T17:06:45.9287052Z .................................................................................................... 6300/9625
2020-02-10T17:06:45.9287052Z .................................................................................................... 6300/9625
2020-02-10T17:06:50.0035943Z ..............................................................................................i..ii. 6400/9625
2020-02-10T17:07:11.6372855Z .................................................................................................... 6600/9625
2020-02-10T17:07:21.5905368Z .................................................................................i.................. 6700/9625
2020-02-10T17:07:23.8431576Z .................................................................................................... 6800/9625
2020-02-10T17:07:26.2210953Z ........................................................................................i........... 6900/9625
---
2020-02-10T17:09:02.8570467Z .................................................................................................... 7600/9625
2020-02-10T17:09:06.8182575Z .................................................................................................... 7700/9625
2020-02-10T17:09:12.1155256Z .................................................................................................... 7800/9625
2020-02-10T17:09:20.1053387Z .................................................................................................... 7900/9625
2020-02-10T17:09:28.7545524Z ...................................................................iiiiiii.i........................ 8000/9625
2020-02-10T17:09:43.6918262Z .......i......i..................................................................................... 8200/9625
2020-02-10T17:09:49.0420579Z .................................................................................................... 8300/9625
2020-02-10T17:10:02.7068415Z .................................................................................................... 8400/9625
2020-02-10T17:10:11.9054555Z .................................................................................................... 8500/9625
---
2020-02-10T17:12:06.1701719Z ---- [ui] ui/allocator-alloc-one.rs stdout ----
2020-02-10T17:12:06.1701760Z 
2020-02-10T17:12:06.1702227Z error: test compilation failed although it shouldn't!
2020-02-10T17:12:06.1702292Z status: exit code: 1
2020-02-10T17:12:06.1703100Z command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/allocator-alloc-one.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-alloc-one/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-alloc-one/auxiliary"
2020-02-10T17:12:06.1703565Z ------------------------------------------
2020-02-10T17:12:06.1703603Z 
2020-02-10T17:12:06.1703852Z ------------------------------------------
2020-02-10T17:12:06.1703902Z stderr:
---
2020-02-10T17:12:06.1718451Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:348:22
2020-02-10T17:12:06.1718533Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2020-02-10T17:12:06.1729148Z 
2020-02-10T17:12:06.1729228Z 
2020-02-10T17:12:06.1731255Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-7/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "7.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2020-02-10T17:12:06.1731608Z 
2020-02-10T17:12:06.1731641Z 
2020-02-10T17:12:06.1738411Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2020-02-10T17:12:06.1738513Z Build completed unsuccessfully in 0:59:48
2020-02-10T17:12:06.1738513Z Build completed unsuccessfully in 0:59:48
2020-02-10T17:12:06.1792613Z == clock drift check ==
2020-02-10T17:12:06.1826005Z   local time: Mon Feb 10 17:12:06 UTC 2020
2020-02-10T17:12:06.2617913Z   network time: Mon, 10 Feb 2020 17:12:06 GMT
2020-02-10T17:12:06.2622853Z == end clock drift check ==
2020-02-10T17:12:06.8201458Z 
2020-02-10T17:12:06.8294385Z ##[error]Bash exited with code '1'.
2020-02-10T17:12:06.8320208Z ##[section]Finishing: Run build
2020-02-10T17:12:06.8366359Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69026/merge to s
2020-02-10T17:12:06.8368120Z Task         : Get sources
2020-02-10T17:12:06.8368173Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-10T17:12:06.8368241Z Version      : 1.0.0
2020-02-10T17:12:06.8368288Z Author       : Microsoft
2020-02-10T17:12:06.8368288Z Author       : Microsoft
2020-02-10T17:12:06.8368340Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-02-10T17:12:06.8368412Z ==============================================================================
2020-02-10T17:12:07.2717671Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-02-10T17:12:07.2770406Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/69026/merge to s
2020-02-10T17:12:07.2879564Z Cleaning up task key
2020-02-10T17:12:07.2880390Z Start cleaning up orphan processes.
2020-02-10T17:12:07.2980769Z Terminate orphan process: pid (3963) (python)
2020-02-10T17:12:07.3527540Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Amanieu
Copy link
Member

Amanieu commented Feb 11, 2020

cc @rust-lang/libs

@bors r+

@bors
Copy link
Contributor

bors commented Feb 11, 2020

📌 Commit 25de80a has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Feb 12, 2020
Remove common usage pattern from `AllocRef`

This removes the common usage patterns from `AllocRef`:
- `alloc_one`
- `dealloc_one`
- `alloc_array`
- `realloc_array`
- `dealloc_array`

Actually, they add nothing to `AllocRef` except a [convenience wrapper around `Layout` and other methods in this trait](https://doc.rust-lang.org/1.41.0/src/core/alloc.rs.html#1076-1240) but have a major flaw: The documentation of `AllocRefs` notes, that

> some higher-level allocation methods (`alloc_one`, `alloc_array`) are well-defined on zero-sized types and can optionally support them: it is left up to the implementor whether to return `Err`, or to return `Ok` with some pointer.

With the current API, `GlobalAlloc` does not have those methods, so they cannot be overridden for `liballoc::Global`, which means that even if the global allocator would support zero-sized allocations, `alloc_one`, `alloc_array`, and `realloc_array` for `liballoc::Global` will error, while calling `alloc` with a zeroed-size `Layout` could succeed. Even worse: allocating with `alloc` and deallocating with `dealloc_{one,array}` could end up with not calling `dealloc` at all!

For the full discussion please see rust-lang/wg-allocators#18

r? @Amanieu
bors added a commit that referenced this pull request Feb 12, 2020
Rollup of 11 pull requests

Successful merges:

 - #67695 (Added dyn and true keyword docs)
 - #68487 ([experiment] Support linking from a .rlink file)
 - #68554 (Split lang_items to crates `rustc_hir` and `rustc_passes`.)
 - #68937 (Test failure of unchecked arithmetic intrinsics in const eval)
 - #68947 (Python script PEP8 style guide space formatting and minor Python source cleanup)
 - #68999 (remove dependency on itertools)
 - #69026 (Remove common usage pattern from `AllocRef`)
 - #69027 (Add missing `_zeroed` varants to `AllocRef`)
 - #69058 (Preparation for allocator aware `Box`)
 - #69070 (Add self to .mailmap)
 - #69077 (Fix outdated doc comment.)

Failed merges:

r? @ghost
@bors bors merged commit 25de80a into rust-lang:master Feb 12, 2020
@TimDiekmann TimDiekmann deleted the common-usage branch August 4, 2020 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants