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

[WIP] Change Travis CI job order. #1

Closed
wants to merge 1 commit into from
Closed

Conversation

kennytm
Copy link
Owner

@kennytm kennytm commented Jul 15, 2017

  1. Move the ALLOW_PR image to the top, so users' PRs will start testing
    immediately.
  2. Move the slow macOS images near the top, so they share more time with
    the rest of the faster Linux builds, which should shorten total test
    time.
  3. Merged the install section of both Linux and macOS.
  4. Do not install anything if $SKIP_BUILD, thus further reducing chance of
    spurious failure in the PR-CI stage (avoid the red cross appearing even
    if CI passed).

1. Move the ALLOW_PR image to the top, so users' PRs will start testing
   immediately.
2. Move the slow macOS images near the top, so they share more time with
   the rest of the faster Linux builds, which should shorten total test
   time.
3. Merged the `install` section of both Linux and macOS.
4. Do not install anything if $SKIP_BUILD, thus further reducing chance of
   spurious failure in the PR-CI stage (avoid the red cross appearing even
   if CI passed).
@kennytm kennytm closed this Jul 15, 2017
kennytm pushed a commit that referenced this pull request Oct 8, 2017
…crichton

Allow atomic operations up to 32 bits

The ARMv5te platform does not have instruction-level support for atomics, however the kernel provides [user space helpers] which can be used to perform atomic operations. When linked with `libgcc`, the atomic symbols needed by Rust will be provided, rather than CPU level intrinsics.

[user space helpers]: https://www.kernel.org/doc/Documentation/arm/kernel_user_helpers.txt

32-bit versions of these kernel level helpers were introduced in Linux Kernel 2.6.12, and 64-bit version of these kernel level helpers were introduced in Linux Kernel 3.1. I have selected 32 bit versions as std currently only requires Linux version 2.6.18 and above as far as I am aware.

As this target is specifically linux and gnueabi, it is reasonable to assume the Linux Kernel and libc will be available for the target. There is a large performance penalty, as we are not using CPU level intrinsics, however this penalty is likely preferable to not having the target at all.

I have used this change in a custom target (along with xargo) to build std, as well as a number of higher level crates.

## Additional information

For reference, here is what a a code snippet decompiles to:

```rust
use std::sync::atomic::{AtomicIsize, Ordering};

#[no_mangle]
pub extern fn foo(a: &AtomicIsize) -> isize {

    a.fetch_add(1, Ordering::SeqCst)
}
```

```
Disassembly of section .text.foo:

00000000 <foo>:
   0:	e92d4800 	push	{fp, lr}
   4:	e3a01001 	mov	r1, #1
   8:	ebfffffe 	bl	0 <__sync_fetch_and_add_4>
   c:	e8bd8800 	pop	{fp, pc}
```

Which in turn is provided by `libgcc.a`, which has code which looks like this:

```
Disassembly of section .text:

00000000 <__sync_fetch_and_add_4>:
       0:	e92d40f8 	push	{r3, r4, r5, r6, r7, lr}
       4:	e1a05000 	mov	r5, r0
       8:	e1a07001 	mov	r7, r1
       c:	e59f6028 	ldr	r6, [pc, rust-lang#40]	; 3c <__sync_fetch_and_add_4+0x3c>
      10:	e5954000 	ldr	r4, [r5]
      14:	e1a02005 	mov	r2, r5
      18:	e1a00004 	mov	r0, r4
      1c:	e0841007 	add	r1, r4, r7
      20:	e1a0e00f 	mov	lr, pc
      24:	e12fff16 	bx	r6
      28:	e3500000 	cmp	r0, #0
      2c:	1afffff7 	bne	10 <__sync_fetch_and_add_4+0x10>
      30:	e1a00004 	mov	r0, r4
      34:	e8bd40f8 	pop	{r3, r4, r5, r6, r7, lr}
      38:	e12fff1e 	bx	lr
      3c:	ffff0fc0 	.word	0xffff0fc0
```

Where you can see the reference to `0xffff0fc0`, which is provided by the [user space helpers].
kennytm pushed a commit that referenced this pull request Dec 25, 2017
…r=michaelwoerister

Set the dwarf linkage_name to the mangled name

ref rust-lang#46453

@michaelwoerister or anyone else who knows, i'm not sure if this is the correct instance to pass here (or how to get the correct one precisely): https://github.com//m4b/rust/blob/5a94a48678ec0a20ea6a63a783e63546bf9459b1/src/librustc_trans/debuginfo/namespace.rs#L36

So don't merge this yet, I'd like to learn about correct instance first; however, I think this already fixes a bunch of weirdness i'm seeing debugging from time to time, not to mention backtraces in gdb via `bt` are now ~readable~ meaningful 🎉

E.g.:

new:
```
(gdb) bt
#0  <inline::Foo as core::convert::From<()>>::from () at /home/m4b/tmp/bad_debug/inline.rs:11
#1  0x000055555555a35d in inline::deadbeef () at /home/m4b/tmp/bad_debug/inline.rs:16
rust-lang#2  0x000055555555a380 in inline::main () at /home/m4b/tmp/bad_debug/inline.rs:20
```

old:
```
(gdb) bt
#0  inline::{{impl}}::from () at /home/m4b/tmp/bad_debug/inline.rs:11
#1  0x000055555555b0ed in inline::deadbeef () at /home/m4b/tmp/bad_debug/inline.rs:16
rust-lang#2  0x000055555555b120 in inline::main () at /home/m4b/tmp/bad_debug/inline.rs:20
```
kennytm pushed a commit that referenced this pull request Apr 11, 2018
Building for x86_64-unknown-linux-musl currently results in an executable lacking debug information for musl libc itself. If you request a backtrace in GDB while control flow is within musl – including sycalls made by musl – the result looks like:

#0  0x0000000000434b46 in __cp_end ()
#1  0x0000000000432dbd in __syscall_cp_c ()
rust-lang#2  0x0000000000000000 in ?? ()

i.e. not very helpful. Adding --enable-debug resolves this, and --enable-optimize re-enables optimisations which default to off given the previous flag.
kennytm added a commit that referenced this pull request Apr 11, 2018
Add --enable-debug flag to musl CI build script

Building for x86_64-unknown-linux-musl currently results in an executable lacking debug information for musl libc itself. If you request a backtrace in GDB while control flow is within musl – including sycalls made by musl – the result looks like:

```
#0  0x0000000000434b46 in __cp_end ()
#1  0x0000000000432dbd in __syscall_cp_c ()
rust-lang#2  0x0000000000000000 in ?? ()
```

i.e. not very helpful. Adding --enable-debug resolves this, and --enable-optimize re-enables optimisations which default to off given the previous flag.
kennytm pushed a commit that referenced this pull request Apr 20, 2018
kennytm pushed a commit that referenced this pull request May 19, 2018
There is a hot path through `opt_normalize_projection_type`:
- `try_start` does a cache lookup (#1).
- The result is a `NormalizedTy`.
- There are no unresolved type vars, so we call `complete`.
- `complete` does *another* cache lookup (rust-lang#2), then calls
  `SnapshotMap::insert`.
- `insert` does *another* cache lookup (rust-lang#3), inserting the same value
  that's already in the cache.

This patch optimizes this hot path by introducing `complete_normalized`,
for use when the value is known in advance to be a `NormalizedTy`. It
always avoids lookup rust-lang#2. Furthermore, if the `NormalizedTy`'s
obligations are empty (the common case), we know that lookup rust-lang#3 would be
a no-op, so we avoid it, while inserting a Noop into the `SnapshotMap`'s
undo log.
kennytm pushed a commit that referenced this pull request Jun 2, 2018
When encountering an unexisting method for a given trait where an
associated function has the same name, suggest using the appropriate
syntax, instead of using `help` text.

When only one candidate is found, do not call it "candidate #1", just
call it "the candidate".
kennytm pushed a commit that referenced this pull request Jun 2, 2018
Tweak output on E0599 for assoc fn used as method

 - Use suggestion instead of `help` when possible
 - Add primary span label
 - Remove incorrect `help` suggestion using incorrect syntax
 - Do not refer to only one possible candidate as `candidate #1`, refer to it as `the candidate`
kennytm pushed a commit that referenced this pull request Jun 28, 2018
Suggestion for 'static impl Trait return

When encountering a named or anonymous sup requirement (for example,
`&'a self`) and a `'static` impl Trait return type, suggest adding the
`'_` lifetime constraing to the return type.

Fix rust-lang#43719, rust-lang#51282.

```
error: cannot infer an appropriate lifetime
  --> $DIR/static-return-lifetime-infered.rs:17:16
   |
LL |     fn iter_values_anon(&self) -> impl Iterator<Item=u32> {
   |                                   ----------------------- this return type evaluates to the `'static` lifetime...
LL |         self.x.iter().map(|a| a.0)
   |         ------ ^^^^
   |         |
   |         ...but this borrow...
   |
note: ...can't outlive the anonymous lifetime #1 defined on the method body at 16:5
  --> $DIR/static-return-lifetime-infered.rs:16:5
   |
LL | /     fn iter_values_anon(&self) -> impl Iterator<Item=u32> {
LL | |         self.x.iter().map(|a| a.0)
LL | |     }
   | |_____^
help: you can add a constraint to the return type to make it last less than `'static` and match the anonymous lifetime #1 defined on the method body at 16:5
   |
LL |     fn iter_values_anon(&self) -> impl Iterator<Item=u32> + '_ {
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
kennytm pushed a commit that referenced this pull request Aug 4, 2018
…-box, r=eddyb

[NLL] Dangly paths for box

Special-case `Box` in `rustc_mir::borrow_check`.

Since we know dropping a box will not access any `&mut` or `&` references, it is safe to model its destructor as only touching the contents *owned* by the box.

----

There are three main things going on here:

1. The first main thing, this PR is fixing a bug in NLL where `rustc` previously would issue a diagnostic error in a case like this:
```rust
fn foo(x: Box<&mut i32>) -> &mut i32 { &mut **x }
```

such code was accepted by the AST-borrowck in the past, but NLL was rejecting it with the following message ([playground](https://play.rust-lang.org/?gist=13c5560f73bfb16d6dab3ceaad44c0f8&version=nightly&mode=release&edition=2015))
```
error[E0597]: `**x` does not live long enough
 --> src/main.rs:3:40
  |
3 | fn foo(x: Box<&mut i32>) -> &mut i32 { &mut **x }
  |                                        ^^^^^^^^ - `**x` dropped here while still borrowed
  |                                        |
  |                                        borrowed value does not live long enough
  |
note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 3:1...
 --> src/main.rs:3:1
  |
3 | fn foo(x: Box<&mut i32>) -> &mut i32 { &mut **x }
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error
```

2. The second main thing: The reason such code was previously rejected was because NLL (MIR-borrowck) incorporates a fix for issue rust-lang#31567, where it models a destructor's execution as potentially accessing any borrows held by the thing being destructed. The tests with `Scribble` model this, showing that the compiler now catches such unsoundness.

However, that fix for issue rust-lang#31567 is too strong, in that NLL (MIR-borrowck) includes `Box` as one of the types with a destructor that potentially accesses any borrows held by the box. This thus was the cause of the main remaining discrepancy between AST-borrowck and MIR-borrowck, as documented in issue rust-lang#45696, specifically in [the last example of this comment](rust-lang#45696 (comment)), which I have adapted into the `fn foo` shown above.

We did close issue rust-lang#45696 back in December of 2017, but AFAICT that example was not fixed by PR rust-lang#46268. (And we did not include a test, etc etc.)

This PR fixes that case, by trying to model the so-called `DerefPure` semantics of `Box<T>` when we traverse the type of the input to `visit_terminator_drop`.

3. The third main thing is that during a review of the first draft of this PR, @matthewjasper pointed out that the new traversal of `Box<T>` could cause the compiler to infinite loop. I have adjusted the PR to avoid this (by tracking what types we have previously seen), and added a much needed test of this somewhat odd scenario. (Its an odd scenario because the particular case only arises for things like `struct A(Box<A>);`, something which cannot be constructed in practice.)

Fix rust-lang#45696.
kennytm pushed a commit that referenced this pull request Sep 27, 2018
kennytm pushed a commit that referenced this pull request Oct 30, 2018
sync fork with upstream (master)
kennytm pushed a commit that referenced this pull request Feb 5, 2019
kennytm pushed a commit that referenced this pull request May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant