[SCEV] Ensure SCEV does not replace aliases with their aliasees

[leonardchan]

Summary

Passes in general shouldn't replace an alias with the aliasee (see https://reviews.llvm.org/D66606). This can lead to situations where a linkonce_odr symbol (which could be interposable if lowered to weak linkage) can be replaced with a local aliasee which won't be interposable. SVEC does this when the function is invoked by FunctionPass Manager -> Loop Pass Manager -> Induction Variable Users in the codegen pipeline. This was found in hwasan instrumented code where a linonce_odr alias was replaced with its private aliasee.

This fixes the bug descriped at https://github.com/llvm/llvm-project/issues/60668.

[leonardchan]

Lemme know if the clang regression test isn't necessary or should be in its own commit.

[leonardchan]

*ping*

[pcc]

Passes shouldn't be replacing aliases with aliasees in general, see D66606. The right fix is to fix SCEV to not do that.

[leonardchan]

In D144035#4143030, @pcc wrote:

Passes shouldn't be replacing aliases with aliasees in general, see D66606. The right fix is to fix SCEV to not do that.

Updated

[MaskRay]

LG

clang/test/CodeGenCXX/pr60668.cpp

2

We try avoiding .cc to assembly test. Drop this file.

[MaskRay]

Full context at https://github.com/llvm/llvm-project/issues/60668.

Best to leave a simplified description in the summary so that a reader doesn't have to go to the external link and summarize the discussions themselves.
And give a link to https://reviews.llvm.org/D66606

This SCEV function is invoked by FunctionPass Manager -> Loop Pass Manager -> Induction Variable Users in the codegen pipeline.

[leonardchan]

In D144035#4145662, @MaskRay wrote:

Full context at https://github.com/llvm/llvm-project/issues/60668.

Best to leave a simplified description in the summary so that a reader doesn't have to go to the external link and summarize the discussions themselves.
And give a link to https://reviews.llvm.org/D66606

This SCEV function is invoked by FunctionPass Manager -> Loop Pass Manager -> Induction Variable Users in the codegen pipeline.

Updated

[MaskRay]

[llvm]

[SCEV] is a more common tag for ScalarEvolution patches.

[leonardchan]

@pcc does this lgty?

[pcc]

LGTM

[leonardchan]

@reames let's continue the discussion here. What specific bits with the optimizer does this break? I'm not too familiar with SVEC. Prior to this, one thing I tried was instead just doing Ops.push_back(GA) and getSCEV(GA) but those seemed to result in infinite recursions last I checked probably because there's another pass attempting to resolve aliases.

clang/test/CodeGenCXX/pr60668.cpp

+// RUN: %clang --target=aarch64-unknown-linux-gnu -O1 -fvisibility=hidden \
+// RUN:   -fsanitize=hwaddress -fsanitize=undefined -std=c++17 -fno-exceptions \
+// RUN:   -fno-rtti -c %s -S -o - | FileCheck %s
+
+struct AndroidSizeClassConfig {
+  static constexpr unsigned Classes[] = {
+      0x00020, 0x00030, 0x00040, 0x00050, 0x00060, 0x00070, 0x00090, 0x000b0,
+      0x000c0, 0x000e0, 0x00120, 0x00160, 0x001c0, 0x00250, 0x00320, 0x00450,
+      0x00670, 0x00830, 0x00a10, 0x00c30, 0x01010, 0x01210, 0x01bd0, 0x02210,
+      0x02d90, 0x03790, 0x04010, 0x04810, 0x05a10, 0x07310, 0x08210, 0x10010,
+  };
+};
+
+static const unsigned NumClasses =
+    sizeof(AndroidSizeClassConfig::Classes) / sizeof(AndroidSizeClassConfig::Classes[0]);
+
+void func(unsigned);
+
+void printMap() {
+  for (unsigned I = 0; I < NumClasses; I++) {
+    func(AndroidSizeClassConfig::Classes[I]);
+  }
+}
+
+// CHECK-LABEL: _Z8printMapv
+// CHECK:        adrp    x{{.*}}, :pg_hi21_nc:_ZN22AndroidSizeClassConfig7ClassesE
+// CHECK:        movk    x{{.*}}, #:prel_g3:_ZN22AndroidSizeClassConfig7ClassesE+4294967296
+// CHECK:        add     x{{.*}}, x19, :lo12:_ZN22AndroidSizeClassConfig7ClassesE

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

for (uint64_t DescriptorPos = 0; DescriptorPos < SizeInBytes;
     appendToCompilerUsed(M, Descriptor);
   }
 
   Constant *Aliasee = ConstantExpr::getIntToPtr(
       ConstantExpr::getAdd(
           ConstantExpr::getPtrToInt(NewGV, Int64Ty),
           ConstantInt::get(Int64Ty, uint64_t(Tag) << PointerTagShift)),
       GV->getType());
+
+  // Ensure the global doesn't have *ODRLinkage. Those linkages should only be
+  // used if all other definitions of the global are "equivalent" to this one.
+  // Since this alias could be defined in other TU's, it's very unlikely they'll
+  // all have the same tag.
+  //
+  // This fixes PR60668.
+  GlobalValue::LinkageTypes Linkage;
+  if (GV->hasLinkOnceODRLinkage()) {
+    Linkage = GlobalValue::LinkOnceAnyLinkage;
+  } else if (GV->hasWeakODRLinkage()) {
+    Linkage = GlobalValue::WeakAnyLinkage;
+  } else {
+    Linkage = GV->getLinkage();
+  }
+
   auto *Alias = GlobalAlias::create(GV->getValueType(), GV->getAddressSpace(),
-                                    GV->getLinkage(), "", Aliasee, &M);
+                                    Linkage, "", Aliasee, &M);
   Alias->setVisibility(GV->getVisibility());
   Alias->takeName(GV);
   GV->replaceAllUsesWith(Alias);
   GV->eraseFromParent();
 }
 
 void HWAddressSanitizer::instrumentGlobals() {
   std::vector<GlobalVariable *> Globals;

llvm/test/Instrumentation/HWAddressSanitizer/odr.ll

+; RUN: opt < %s -passes=hwasan -S | FileCheck %s
+
+; CHECK: @linkonce_odr_obj = linkonce hidden alias
+@linkonce_odr_obj = linkonce_odr hidden constant i32 1
+
+; CHECK: weak_odr_obj = weak hidden alias
+@weak_odr_obj = weak_odr hidden constant i32 1