Miscompilation of rustc-produced IR since 01859da84bad95fd51d6a03b08b60c660e642a4f

[glandium]

Compiling Firefox with cross language LTO + PGO (note: I haven't tried without PGO) produces a crashy Firefox on all platforms. I bisected this down to 01859da, and reverting that commit from current trunk fixes the issue.

Reproducer (sorry, it's massive, I haven't reduced it yet because I'm also in the middle of tracking down separate problems with non-trunk clang): https://drive.google.com/file/d/12s6DsTnMWIPuMikVRYTMPklyj-OeXH6f/view?usp=sharing
Instructions (run on x86-64 Linux):

• Unpack testcase.tar.xz
• cd testcase
• run the command in the build file
• run the command in the crash file (requires an X11 server ; without an X11, there's a different, unrelated crash)

In case this helps, this is a pernosco session with the crash: https://pernos.co/debug/Twdl8_V2QanMUEn2lEG0qg/index.html

Cc: @pcwalton, @nikic

[pcwalton]

I'd like to make sure that this is a miscompilation (not saying it isn't!) and not UB that's now causing crashes. In particular if you write to immutable data behind an & reference and don't use UnsafeCell, that patch will probably cause your code to crash. (It could crash before, but it's especially likely to crash now.)

[nikic]

Does this issue still exist?

[glandium]

Does this issue still exist?

It does.

[xry111]

Thunderbird suffers the same issue (with only LTO and no PGO). Disabling LTO hides the issue.

[nekopsykose]

the specific issue seems to fix itself if one disables the following optimisation:

diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index dc728c1cb..d476965b7 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -704,12 +704,12 @@ ModRefInfo BasicAAResult::getModRefInfoMask(const MemoryLocation &Loc,
     //
     // An argument that is marked readonly and noalias is known to be
     // invariant while that function is executing.
-    if (const Argument *Arg = dyn_cast<Argument>(V)) {
-      if (Arg->hasNoAliasAttr() && Arg->onlyReadsMemory()) {
-        Result |= ModRefInfo::Ref;
-        continue;
-      }
-    }
+    // if (const Argument *Arg = dyn_cast<Argument>(V)) {
+    //   if (Arg->hasNoAliasAttr() && Arg->onlyReadsMemory()) {
+    //     Result |= ModRefInfo::Ref;
+    //     continue;
+    //   }
+    // }
 
     // A global constant can't be mutated.
     if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(V)) {

from the above changes only. sounds indeed like something broken on firefox side

[glandium]

Thank you for pinpointing the specific part that was causing problems. Along with rust compiler beta releases using LLVM 16 causing similar problems in a more reduced way, this allowed to pinpoint where the problem actually lies, and yes, it's something broken in Firefox unsafe code, but I'd argue because of the changes in 01859da it shouldn't allow the pattern that causes problems in the first place. I'll file a bug against the rust compiler.

Thanks again.

[thesamesam]

Cross-linking to help out folks:

• FF bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1831242
• Rust bug: mutable_transmutes lint should catch transmutes from a type without interior mutability to one with rust-lang/rust#111229