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

AArch64 Linux CI failure #77615

Closed
pietroalbini opened this issue Oct 6, 2020 · 9 comments
Closed

AArch64 Linux CI failure #77615

pietroalbini opened this issue Oct 6, 2020 · 9 comments
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state

Comments

@pietroalbini
Copy link
Member

aarch-gnu looks like it's been failing for a while (and no one noticed because it's auto-fallible).

--- expected_show_coverage.generics.txt	2020-10-06 03:36:29.936045671 +0000
+++ /checkout/obj/build/aarch64-unknown-linux-gnu/test/run-make-fulldeps/coverage-reports-deadcode/coverage-reports-deadcode/actual_show_coverage.generics.txt	2020-10-06 05:21:51.420743607 +0000
@@ -29,12 +29,12 @@
    18|      2|        println!("BOOM times {}!!!", self.strength);
    19|      2|    }
   ------------------
-  | <generics::Firework<i32> as core::ops::drop::Drop>::drop:
+  | <generics::Firework<i32> as core[a7a74cee373f048]::ops::drop::Drop>::drop:
   |   17|      1|    fn drop(&mut self) {
   |   18|      1|        println!("BOOM times {}!!!", self.strength);
   |   19|      1|    }
   ------------------
-  | <generics::Firework<f64> as core::ops::drop::Drop>::drop:
+  | <generics::Firework<f64> as core[a7a74cee373f048]::ops::drop::Drop>::drop:
   |   17|      1|    fn drop(&mut self) {
   |   18|      1|        println!("BOOM times {}!!!", self.strength);
   |   19|      1|    }

------------------------------------------
stderr:
------------------------------------------
Error: 1
Error: 1
diff failed, and not suppressed without `// ignore-llvm-cov-show-diffs` in ../coverage/generics.rs
make: *** [../coverage-reports-base/Makefile:45: generics] Error 1

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

failures:
    [run-make] run-make-fulldeps/coverage-reports-base
    [run-make] run-make-fulldeps/coverage-reports-deadcode

Originally posted by @jyn514 in #76356 (comment)

@pietroalbini
Copy link
Member Author

@rustbot ping arm

@rustbot rustbot added the O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state label Oct 6, 2020
@rustbot
Copy link
Collaborator

rustbot commented Oct 6, 2020

Hey ARM Group! This bug has been identified as a good "ARM candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @JamieCunliffe @joaopaulocarreiro @raw-bin @Stammark @vigoux

@pietroalbini pietroalbini added the C-bug Category: This is a bug. label Oct 6, 2020
@jyn514 jyn514 added the A-testsuite Area: The testsuite used to check the correctness of rustc label Oct 6, 2020
@richkadel
Copy link
Contributor

This change is due to a recent change in rust-demangler (in #77080), which uses a regex to strip off "crate disambiguators" from demangled names.

The regex assumes the pattern has 16 hex characters enclosed in square brackets.

[a7a74cee373f048]
 123456789012345

I'm not sure why AArch64 shows only 15 characters.

Could this be a leading zero issue? In which case, the regex needs to be less rigid.

Or is there a bug related to generating the character string on AArch64?

We could bandaid this, but I don't want to close it out with a Regex change that might cover up some other problem.

@alexcrichton - This uses your rustc-demangle library. What do you think?

Also, I was wondering if you think it's worth adding an option to the library to generate demangled names without the crate disambiguators, avoiding the need for the regex replacement.

@alexcrichton
Copy link
Member

Hm I'm not entirely sure! It seems like the most likely situation here is that the hash is different depending on the target platform (x86_64 vs aarch64) and perhaps the hash is just wrong enough that the regex isn't stripping it? (like the leading zero idea you had). I am not intimately familiar with the symbol mangling here though myself so there could be a bug in rustc-demangle as well, I'm just not sure.

@richkadel
Copy link
Contributor

It looks like it's definitely possible to have disambiguators shorter than 16 characters, because the format code doesn't enforce a specific width:

https://github.com/alexcrichton/rustc-demangle/blob/c4e3ab004edda9fbe5e2d8149d2b62c10668d10b/src/v0.rs#L681

That means that, for now, this is a bug in the rust-demangler tool, regardless of the target platform/architecture.

The fact that this happens on AArch64 might just be a random coincidence.

I'll submit a patch to make the regex more forgiving.

@alexcrichton - Do you maintain the rustc-demangler library, and if so, what do you think about also including an option in the library to omit these disambiguators?

Note that I had to strip them because they aren't cross-platform stable, and for some use cases (like the source code coverage tooling) it's better to leave them off.

(FYI, @wesleywiser @tmandry )

@richkadel
Copy link
Contributor

It seems like the most likely situation here is that the hash is different depending on the target platform (x86_64 vs aarch64)

BTW, yes, the hash changes depending on some factor, not simply the CPU type though.

I first ran into the issue here:

https://github.com/rust-lang-ci/rust/runs/1197008506

The demangled crate disambiguator hashes were different between my linux workstation (where I generated the original blessed test output) linux, vs. one of the Ubuntu platforms vs. MSVC. They are all x86_64-based, but all had different values.

@alexcrichton
Copy link
Member

Seems reasonable to me to have an option for that!

@richkadel
Copy link
Contributor

Thanks. No hurry, assuming #77627 lands, this issue here should be resolved.

Also, when I was changing the Regex in the fix, I realized the pattern [<digits>]:: also does come up for closures and possibly (according to a doc in the tree) also for anonymous types. When I've seen this for closures, the values were small integers like 0, 1, etc. (possibly for different type parameter variations). I think these 0-based indexes should be retained.

I'm not sure that the demangler library distinguishes them, so if you do add the option, you may want to consider checking the value, maybe like I did with the regex (though since the library is starting with a number, you could simply check if the value is below a threshold).

It sounds a big hacky, but I'm not sure what else to suggest.

richkadel added a commit to richkadel/rust that referenced this issue Oct 8, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 9, 2020
rust-demangler tool strips crate disambiguators with < 16 digits

Addresses Issue rust-lang#77615.
@richkadel
Copy link
Contributor

The fix was merged, and these tests now pass again:

https://github.com/rust-lang-ci/rust/runs/1229835355

test [run-make] run-make-fulldeps/coverage-reports-base ... ok
test [run-make] run-make-fulldeps/issue-22131 ... ok
test [run-make] run-make-fulldeps/issue-26006 ... ok
test [run-make] run-make-fulldeps/issue-33329 ... ok
test [run-make] run-make-fulldeps/issue-25581 ... ok
test [run-make] run-make-fulldeps/issue-28766 ... ok
test [run-make] run-make-fulldeps/issue-24445 ... ok
test [run-make] run-make-fulldeps/issue-35164 ... ok
test [run-make] run-make-fulldeps/issue-28595 ... ok
test [run-make] run-make-fulldeps/issue-36710 ... ok
test [run-make] run-make-fulldeps/issue-37839 ... ok
test [run-make] run-make-fulldeps/issue-51671 ... ok
test [run-make] run-make-fulldeps/issue-40535 ... ok
test [run-make] run-make-fulldeps/issue-53964 ... ok
test [run-make] run-make-fulldeps/issue-37893 ... ok
test [run-make] run-make-fulldeps/issue-46239 ... ok
test [run-make] run-make-fulldeps/issue-7349 ... ok
test [run-make] run-make-fulldeps/issues-41478-43796 ... ok
test [run-make] run-make-fulldeps/coverage-reports-deadcode ... ok

@jyn514 jyn514 closed this as completed Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state
Projects
None yet
Development

No branches or pull requests

5 participants