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

make check-stage1 is broken for run-make/bootstrap-from-c-with-green #13732

Closed
pnkfelix opened this issue Apr 24, 2014 · 7 comments · Fixed by #14000
Closed

make check-stage1 is broken for run-make/bootstrap-from-c-with-green #13732

pnkfelix opened this issue Apr 24, 2014 · 7 comments · Fixed by #14000

Comments

@pnkfelix
Copy link
Member

There is something funky going on when I currently try to use make check-stage1 on a clean checkout of rust (commit e01e78f)

https://gist.github.com/pnkfelix/11260219

Summary of how the story in that gist ends:



% rm -rf x86_64-apple-darwin/stage1/lib/stamp.green
% rm -rf x86_64-apple-darwin/stage1/lib/stamp.rustuv
% rm -rf x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/libboot-c9bec0e5-0.1.dylib
% remake --trace check-stage1
...
##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
/Users/fklock/Dev/Mozilla/rust.git/objdir-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/main
##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

#0 all at /Users/fklock/Dev/Mozilla/rust.git/src/test/run-make/bootstrap-from-c-with-green/Makefile:3
remake[1]: Leaving directory `/Users/fklock/Dev/Mozilla/rust.git/src/test/run-make/bootstrap-from-c-with-green'
Command-line invocation:
"/Users/fklock/opt/remake/bin/remake -C /Users/fklock/Dev/Mozilla/rust.git/src/test/run-make/bootstrap-from-c-with-green/"

------ stderr ---------------------------------------------
dyld: Symbol not found: __ZN10event_loop20h1197db882d44291cDOf9v0.11.preE
Referenced from: /Users/fklock/Dev/Mozilla/rust.git/objdir-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/libboot-c9bec0e5-0.1.dylib
Expected in: /Users/fklock/Dev/Mozilla/rust.git/objdir-dbgopt/x86_64-apple-darwin/stage1/lib/librustuv-30cdf6fe-0.11-pre.dylib
in /Users/fklock/Dev/Mozilla/rust.git/objdir-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/libboot-c9bec0e5-0.1.dylib
Makefile:3: *** [all] Trace/BPT trap: 5

------ ---------------------------------------------

/Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:922: *** [x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green-1-T-x86_64-apple-darwin-H-x86_64-apple-darwin.ok] Error 2

#0 x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green-1-T-x86_64-apple-darwin-H-x86_64-apple-darwin.ok at /Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:922
#1 tmp/check-stage1-T-x86_64-apple-darwin-H-x86_64-apple-darwin-rmake.ok at /Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:923
#2 check-stage1-T-x86_64-apple-darwin-H-x86_64-apple-darwin-rmake-exec at /Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:923
#3 check-stage1-T-x86_64-apple-darwin-H-x86_64-apple-darwin-exec at /Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:332
#4 check-stage1-T-x86_64-apple-darwin-H-x86_64-apple-darwin at /Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:807
#5 check-stage1-H-x86_64-apple-darwin at /Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:847
#6 check-stage1 at /Users/fklock/Dev/Mozilla/rust.git/mk/tests.mk:828
Command-line invocation:
"/Users/fklock/opt/remake/bin/remake --trace check-stage1"
%

@alexcrichton says he often just runs make check-stage1-rpass, so that is sort of a workaround for the near term.

@pnkfelix
Copy link
Member Author

see also, further irc discussion/debugging between myself and alexcrichton here: https://botbot.me/mozilla/rust-internals/msg/13776568/

@pnkfelix
Copy link
Member Author

I resolved where the rpath into my $HOME/opt/rust-dbg/lib/rustlib/x86_64-apple-darwin/lib was coming from: the config.mk file in my build tree has this setting

CFG_LIBDIR           := /Users/fklock/opt/rust-dbg/lib

(I am still a bit confused because this particular build directory almost always doesn't use that particular --prefix) (update: no, it was correct, I shouldn't have been confused at all. I forgot I keep the non-optimized build in rust-dbg-nopt)

Anyway, so this is getting installed as the fallback rpath, as indicated here in a verbose log:

relative rpaths:
    @loader_path/../../../stage1/lib/rustlib/x86_64-apple-darwin/lib
    @loader_path/../../../stage1/lib/rustlib/x86_64-apple-darwin/lib
    @loader_path/../../../stage1/lib/rustlib/x86_64-apple-darwin/lib
    @loader_path/../../../stage1/lib/rustlib/x86_64-apple-darwin/lib
    @loader_path/../../../stage1/lib/rustlib/x86_64-apple-darwin/lib
fallback rpaths:
    /Users/fklock/opt/rust-dbg/lib/rustlib/x86_64-apple-darwin/lib

@pnkfelix
Copy link
Member Author

Looking at the code in librustc/back/rpath.rs, we hard-code a reference to the install prefix into rustc itself.

I don't have a problem with that, but we probably should have a compiler flag to either remove the usage of that fallback rpath, or override the path that rustc uses for the fallback. Then the Makefile could actually run rustc but have it referencing its own build tree for the fallback by using the overriding option.

@pnkfelix
Copy link
Member Author

Okay, so I hacked up a patch to add a --install-prefix option to rustc that overrides the embedded one in the librustc library.

After doing that and using it in the run-make/bootstrap-from-c-with-green/Makefile, I still hit the problem.

I did some more digging and determined that there is some funky effect being introduced by the setting of the DYLD_LIBRARY_PATH environment variable. (I added a call to env to the start of the Makefile rule to print out the environment before running the recipe, that helped a lot).

From the settings of the DYLD_LIBRARY_PATH and LD_LIBRARY_PATH environment variables, I determined that you need at least one of these environment variables to be set if you want this test to run at all; if both are unset, you get:

% /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/main
dyld: Library not loaded: @rpath/libboot-c9bec0e5-0.1.dylib
  Referenced from: /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/main
  Reason: image not found
Trace/BPT trap: 5
% 

So I transcribed the settings that the Makefiles are current establishing for the two environment variables at the time that this recipe is run (by the aforementioned env printout), and found the following distinct behaviors.

% DYLD_LIBRARY_PATH=/Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green::/Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/stage1/lib LD_LIBRARY_PATH=/Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green: /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/main
dyld: Symbol not found: __ZN10event_loop20h1197db882d44291cDOf9v0.11.preE
  Referenced from: /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/libboot-c9bec0e5-0.1.dylib
  Expected in: /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/stage1/lib/librustuv-30cdf6fe-0.11-pre.dylib
 in /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/libboot-c9bec0e5-0.1.dylib
Trace/BPT trap: 5
% LD_LIBRARY_PATH=/Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green: /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/test/run-make/bootstrap-from-c-with-green/main
hello
% 

It seems like the DYLD_LIBRARY_PATH is causing my problem here at this point. In particular the entry for /Users/fklock/Dev/Mozilla/rust.git/objdir-check1-dbgopt/x86_64-apple-darwin/stage1/lib is causing problems. Should that be pointing instead to x86_64-apple-darwin/stage1/lib/rustlib/x86_64-apple-darwin/lib ?

@pnkfelix
Copy link
Member Author

attempting to fix, work in progress, see PR #13753

@pnkfelix
Copy link
Member Author

pnkfelix commented May 6, 2014

(closed #13753 in favor of PR #14000)

@pnkfelix
Copy link
Member Author

pnkfelix commented May 7, 2014

part of #8058

bors added a commit that referenced this issue May 18, 2014
Fix #13732.

This is a revised, much less hacky form of PR #13753

The changes here:

 * add instrumentation to aid debugging of linkage errors,
 * fine tune some things in the Makefile where we are telling binaries to use a host-oriented path for finding dynamic libraries, when it should be feeding the binaries a target-oriented path for dynamic libraries.
 * pass along the current stage number to run-make tests, and
 * skip certain tests when running atop stage1.

Fix #13746 as well.
arcnmx pushed a commit to arcnmx/rust that referenced this issue Dec 17, 2022
…hievink

fix: add fallback case in generated `PartialEq` impl

Partially fixes rust-lang#13727.

When generating `PartialEq` implementations for enums, the original code can already generate the following fallback case:

```rs
_ => std::mem::discriminant(self) == std::mem::discriminant(other),
```

However, it has been suppressed in the following example for no good reason:

```rs
enum Either<T, U> {
    Left(T),
    Right(U),
}

impl<T, U> PartialEq for Either<T, U> {
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Self::Left(l0), Self::Left(r0)) => l0 == r0,
            (Self::Right(l0), Self::Right(r0)) => l0 == r0,
            // _ => std::mem::discriminant(self) == std::mem::discriminant(other),
            // ^ this completes the match arms!
        }
    }
}
```

This PR has removed that suppression logic.

~~Of course, the PR could have suppressed the fallback case generation for single-variant enums instead, but I believe that this case is quite rare and should be caught by `#[warn(unreachable_patterns)]` anyway.~~

After this fix, when the enum has >1 variants, the following fallback arm will be generated :

* `_ => false,` if we've already gone through every case where the variants of `self` and `other` match;
* The original one (as stated above) in other cases.

---

Note: The code example is still wrong after the fix due to incorrect trait bounds.
arcnmx pushed a commit to arcnmx/rust that referenced this issue Jan 9, 2023
…Veykril

fix: add generic `TypeBoundList` in generated derivable impl

Potentially fixes rust-lang#13727.

Continuing with the work in rust-lang#13732, this fix tries to add correct type bounds in the generated `impl` block:

```diff
  enum Either<T, U> {
      Left(T),
      Right(U),
  }

- impl<T, U> PartialEq for Either<T, U> {
+ impl<T: PartialEq, U: PartialEq> PartialEq for Either<T, U> {
      fn eq(&self, other: &Self) -> bool {
          match (self, other) {
              (Self::Left(l0), Self::Left(r0)) => l0 == r0,
              (Self::Right(l0), Self::Right(r0)) => l0 == r0,
              _ => false,
          }
      }
  }
```
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 a pull request may close this issue.

1 participant