-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Bootstrap build is failing on Mac OS X with "rust: task ... ran out of stack" #6061
Comments
…he generic `gen`." This reverts commit 4a24f10 as part of Felix's attempt to resolve Issue rust-lang#6061.
This reverts commit 9860fe1 as part of Felix's attempt to resolve Issue rust-lang#6061.
A theory from #rust IRC channel:
|
Does (Also, sorry for breaking everything :( ) |
And, does giving type hints, i.e. |
I'm checking on this. |
@pcwalton's recent ffi changes could have significantly changed the way stack accounting works. You could try running with RUST_MAX_STACK=20000000 (20MB, the default is 8MB) and see if that gives you enough overhead. |
More specifically, C stacks were never accounted for before when deciding whether there was enough stack. I assume that now when we use the fast_ffi stacks, that is attributing an additional 2MB of stack to the task. |
Would (I'm still not quite sure what it does, but it's helped with stack troubles before.) |
@catamorphism (or @brson, or anyone else who can reproduce this) I just opened #6073, which reduces the number of C calls that the rand module does (they are then only necessary for generating the initial seed), which may fix this if it that is the problem. I can't test it so I'd appreciate if someone could throw some spare CPU cycles at it. |
Maybe I am doing it wrong, or maybe our stack exhaustion handling code needs some hacking. Looking through the other suggestions. |
@huonw commit 6d50d55 does not fix the issue for me. That is, after fetching that repository, rebasing that commit to my incoming (commit 1d53bab), cleaning my build tree (via:
@brson Increasing the stack size by various reasonable+unreasonable amounts does not fix the problem:
(I still haven't investigating what it is about the commits that causes the problem. I plan to do that now; I just wanted to make sure I addressed these comments first.) |
See #6061. Reverting these commits fixes it; later we'll isolate the problem.
FWIW I opened pull request #6081 to fix some longstanding issues about recursion limits. It doesn't sound like it will fix this problem though. |
@huonw A new discovery: rather than reverting the entirety of the rand-related commits, this much smaller change: pnkfelix/rust@b9aaa79 on my own repo (branch: investigate-issue-6061) also appears to fix the stack exhaustion problem. That smaller change was largely a shot in the dark on my part (it just happened to be the first in a series of planned changes to narrow down the reversion to something we can actually analyze). It removes a trait impl of the form: // Allow direct chaining with `task_rng`
impl<R: Rng> Rng for @R {
fn next(&self) -> u32 { (*self).next() }
} and then puts in some supplementary explicit derefs that I'm guessing were meant to be main unnecessary by the direct chaining, but became necessary after removing the above I'll admit, I find the whole matter bizarre; @pcwalton asserted yesterday that we should be auto-deref'ing on these sorts of method invocations anyway. Maybe that's a hint to what is going on here, some sort of nasty interaction between the above |
I freely admit that the auto-derefs I removed do not look like they have anything to do with the Anyway, at this point I have further narrowed down the change that brings bootstrapping back to a working state to the following:
Curiouser and curiouser. I am still looking. |
Okay I finally had the bright idea to make a debug-enabled build and set a breakpoint at the point in the C++ code
|
So at this point I'm fairly confident that the stack exhaustion problem that we are observing here is because the trait from the comment above is erroneous: the way its code is written, it makes one think that it is doing one level of dereference (a single unrolling), but in fact it is compiling into a self-recursive call, at least on my host. But I think I understand why @huonw put in that
and, after fixing that:
My hypothesis is that the latter failures are problems in our auto-deref insertion logic, at least at the level of the static analysis before code generation. The question is: Why is it that the resulting code works at all on some (many!) targets, but compiles into the infinite loop on others? Very very strange. I'm also not sure about whether it makes sense to be able to impl a trait that has trait Fooable { fn foo(&self) -> int; }
impl Fooable for int { fn foo(&self) -> int { 3 } }
impl Fooable for @str { fn foo(&self) -> int { 4 } } Would *self denote an int in the first case and a str in the second case (which seems like magic)? I only wrote the above example off the top of my head; I'll need to double-check that I even got the details right later. But my point is, a bug like the Rng issue above seems like it might be arising in part due to confusion in scenarios like the above. |
Hi all, I think I have a fix: diff --git a/src/libcore/rand.rs b/src/libcore/rand.rs
index cdf6a5b..5294439 100644
--- a/src/libcore/rand.rs
+++ b/src/libcore/rand.rs
@@ -690,7 +690,7 @@ pub fn task_rng() -> @IsaacRng {
// Allow direct chaining with `task_rng`
impl<R: Rng> Rng for @R {
- fn next(&self) -> u32 { (*self).next() }
+ fn next(&self) -> u32 { (**self).next() }
}
/** The problem is that the first |
@Blei your hypothesis is plausible. But it does not explain why the infinite loop is only arising on some targets. I think there is still a latent platform-dependent bug there, perhaps in trait method resolution or perhaps in the auto-deref generation. |
@Blei, (maybe) you could/should open a pull request with that fix, so that this get solved. The platform dependency can get investigated/solved progressively. FWIW, the following gives a trait Foo {
fn x(&self);
}
impl Foo for int {
fn x(&self) {}
}
impl<F:Foo> Foo for @F {
fn x(&self) {
(*self).x()
}
}
fn main() {
(@1i).x()
} |
An interesting detail I just discovered while trying to understand why the infinite recursion is not exhibited across all architectures: when I do a bootstrap build, the default configure settings for Mac OS X (namely Clarification: the above experiment was applied to the commit preceding Blei's fix (SHA: cdd342b); the whole point is to find out why the code before the fix was not behaving uniformly across the board (either breaking everywhere or working everywhere). |
I think I was mistaken in earlier comment when I claimed that the relevant bit was the choice between x86_64 and i686 -- there were a number of other settings on my original
I am still investigating. This particular issue has been closed, but there is a more insidious bug hiding here I believe, and if I can replicate it readily, I will open a fresh issue for it. |
While I was bisecting through this commit range I saw this problem on linux too with |
Lint: filter(Option::is_some).map(Option::unwrap) Fixes rust-lang#6061 *Please write a short comment explaining your change (or "none" for internal only changes)* changelog: * add new lint for filter(Option::is_some).map(Option::unwrap) First Rust PR, so I'm sure I've violated some idioms. Happy to change anything. I'm getting one test failure locally -- a stderr diff for `compile_test`. I'm having a hard time seeing how I could be causing it, so I'm tentatively opening this in the hopes that it's an artifact of my local setup against `rustc`. Hoping it can at least still be reviewed in the meantime. I'm gathering that since this is a method lint, and `.filter(...).map(...)` is already checked, the means of implementation needs to be a little different, so I didn't exactly follow the setup boilerplate. My way of checking for method calls seems a little too direct (ie, "is the second element of the expression literally the path for `Option::is_some`?"), but it seems like that's how some other lints work, so I went with it. I'm assuming we're not concerned about, eg, closures that just end up equivalent to `Option::is_some` by eta reduction.
Fresh clone of repo.
commit 1d53bab
This may be a duplicate of another issue, for example perhaps #6049. I'm honestly not sure.
At this point I have bisected the problem down to:
4a24f10 is the first bad commit
commit 4a24f10
Author: Huon Wilson [email protected]
Date: Wed Apr 24 22:29:19 2013 +1000
(But it is possible that Huon's code is just exposing a latent bug in rustc, rather than actually having something fundamentally wrong with it itself.) I plan to investigate backing out Huon's change to see if that makes the problem go away, just so I can actually bootstrap a compiler and do more investigation.
The text was updated successfully, but these errors were encountered: