[LLVM] Segfault / "llvm::thinLTOInternalizeModule ... Assertion `GS != DefinedGlobals.end()' failed", again

[tmandry]

I got a segfault / LLVM assertion failure while compiling a target with ThinLTO enabled. This is the same failure as in #53912, albeit with a different root cause.

In my case, I was dealing with a global named switch.table._ZN77_$LT$omaha_client..protocol..request..Event$u20$as$u20$core..clone..Clone$GT$5clone17h829b64c9ab982ff5E.llvm.10390335839252477638.llvm.11308644296266801080.

Note the presence of two .llvm.NNNN extensions. This means that this was a local which was promoted to a global symbol twice. The implementation of getOriginalNameBeforePromote doesn't handle this case correctly; it strips off both extensions:

  /// Convenience method for creating a promoted global name
  /// for the given value name of a local, and its original module's ID.
  static std::string getGlobalNameForLocal(StringRef Name, ModuleHash ModHash) {
    SmallString<256> NewName(Name);
    NewName += ".llvm.";
    NewName += utostr((uint64_t(ModHash[0]) << 32) |
                      ModHash[1]); // Take the first 64 bits
    return NewName.str();
  }

  /// Helper to obtain the unpromoted name for a global value (or the original
  /// name if not promoted).
  static StringRef getOriginalNameBeforePromote(StringRef Name) {
    std::pair<StringRef, StringRef> Pair = Name.split(".llvm.");
    return Pair.first;
  }

The fix is actually trivial; just have to change split to rsplit so that it only removes the last .llvm.NNNN extension. Leaving this issue open to track the fix being cherry-picked.

[tmandry]

Filed a bug upstream.

[tmandry]

This might still be a rustc bug, since apparently a double-promotion is unexpected. Working on a minimal repro now.

[pnkfelix]

@tmandry is this something that we could/should be pulling in the ICE-breakers on?

[pnkfelix]

triage: P-high. Assigning to @tmandry and removing I-nominated label.

[tmandry]

Possibly, but I’ll need to put a usable reproducer somewhere. That tends to be more difficult when the target is Fuchsia.

I’ve had limited success doing all the minimizing myself. I’ll see if I can reproduce on other targets, and upload a reproducer if so.

[tmandry]

So, from the output of -Csave-temps, what appears to be happening is that we run the same bitcode through the ThinLTO optimization pipeline twice. Once when outputting an rlib, and once when linking to that rlib.

More specifically:

1. An rlib crate (omaha_client in this case) gets compiled with ThinLTO and optimizations enabled. We run the bitcode through the ThinLTO optimization pipeline.

• -Csave-temps results in a lot of files that look like libomaha_client.omaha_client.3a1fbbbh-cgu.15.rcgu.thin-lto-after-rename.bc

2. A binary crate foo is compiled which depends on the rlib. It is also compiled with ThinLTO and optimizations.

• -Csave-temps again results in a lot of files that look like foo.omaha_client.3a1fbbbh-cgu.15.rcgu.thin-lto-after-rename.bc (except after-internalize and everything after it are missing, because it crashes)
• the foo.omaha_client.3a1fbbbh-cgu.15.rcgu.thin-lto-input.bc file is the output of the ThinLTO pipeline from the previous step

The offending symbol is promoted to a global once in the first optimization run and again in the second, which accounts for the two .llvm.xxxxx suffixes on it. The crash occurs because it's not expected for a symbol to be promoted twice.

I can't think of a reason why we would run the same pipeline twice on the same bitcode. Is there one?