[AsmPrinter] GlobalAlias with non-zero size and public linkage ends up with wrong size, PR38794

[daphnediane]

Summary

Fix PR38794

The size of global aliases were not showing up correctly in object files. The updated logic checks if the alias is at an offset from the global object or has a size and that size is smaller than the global object. This expands the logic in rL244752.

[john.brawn]

The reason for the default of not emitting a size (which ends up having the effect of the alias having the same size as the aliasee) is for compatibility with gcc where if you have

int var1[10] = {1};
extern int var2 __attribute__((alias("var1")));

you end up with var2 being a symbol with the same size as var1, even though it's a smaller type.

So I think checking for the size being smaller is incorrect, or rather it would mean we would be inconsistent with gcc which we generally want to avoid unless there's a good reason, but the change to emit the size when the alias has an offset (i.e. the change in how we decide BaseObject) looks OK. It does make me wonder though if perhaps it would be better to explicitly distinguish at the IR level between alias as "this name is another name for that other thing", which we want for the alias variable attribute and so don't want to emit a size; and "this name refers to a subpart of that other thing", which we want for aliases generated by GlobalMerge and so do want to emit a size.

[daphnediane]

In D51637#1227528, @john.brawn wrote:

The reason for the default of not emitting a size (which ends up having the effect of the alias having the same size as the aliasee) is for compatibility with gcc where if you have

int var1[10] = {1};
extern int var2 __attribute__((alias("var1")));

you end up with var2 being a symbol with the same size as var1, even though it's a smaller type.

So I think checking for the size being smaller is incorrect, or rather it would mean we would be inconsistent with gcc which we generally want to avoid unless there's a good reason, but the change to emit the size when the alias has an offset (i.e. the change in how we decide BaseObject) looks OK. It does make me wonder though if perhaps it would be better to explicitly distinguish at the IR level between alias as "this name is another name for that other thing", which we want for the alias variable attribute and so don't want to emit a size; and "this name refers to a subpart of that other thing", which we want for aliases generated by GlobalMerge and so do want to emit a size.

Note I'm using LLVM IR for a different language not C/C++ and need to be able to generate partial symbols of different sizes. The programming language I'm porting has a concept of combined segments where some parts of the segments are public and that the layout between them is well defined within a single module but only visible by symbol name to external modules.

[daphnediane]

In D51637#1227528, @john.brawn wrote:

It does make me wonder though if perhaps it would be better to explicitly distinguish at the IR level between alias as "this name is another name for that other thing", which we want for the alias variable attribute and so don't want to emit a size; and "this name refers to a subpart of that other thing", which we want for aliases generated by GlobalMerge and so do want to emit a size.

Was considering trying this using attributes, but it looks like while GlobalValues contain an AttributeSet, GlobalAliases do not. I wonder if the best fix would just allow any sized global alias to emit sizes and have clang change how it makes such aliases so that the external symbol is unsized possibly with a command line flag to allow the sizes to match. It's interesting that the gcc documentation only has alias listed as supporting functions and not variables. https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes and https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html. Other attributes that are common have references back to function attributes in the variable attribute section.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::emitGlobalIndirectSymbol(Module &M,
   if (isa<GlobalAlias>(&GIS) && MAI->hasAltEntry() && isa<MCBinaryExpr>(Expr))
     OutStreamer->EmitSymbolAttribute(Name, MCSA_AltEntry);
 
   // Emit the directives as assignments aka .set:
   OutStreamer->EmitAssignment(Name, Expr);
 
   if (auto *GA = dyn_cast<GlobalAlias>(&GIS)) {
     // If the aliasee does not correspond to a symbol in the output, i.e. the
-    // alias is not of an object or the aliased object is private, then set the
+    // alias is not of an object, the aliased object is private, the aliasee is
+    // offset from the object or is smaller then the base object, then set the
     // size of the alias symbol from the type of the alias. We don't do this in
     // other situations as the alias and aliasee having differing types but same
     // size may be intentional.
-    const GlobalObject *BaseObject = GA->getBaseObject();
-    if (MAI->hasDotTypeDotSizeDirective() && GA->getValueType()->isSized() &&
-        (!BaseObject || BaseObject->hasPrivateLinkage())) {
+    const GlobalObject *BaseObject =
+        dyn_cast<GlobalObject>(GA->stripPointerCasts());
+    if (MAI->hasDotTypeDotSizeDirective() && GA->getValueType()->isSized()) {
       const DataLayout &DL = M.getDataLayout();
       uint64_t Size = DL.getTypeAllocSize(GA->getValueType());
-      OutStreamer->emitELFSize(Name, MCConstantExpr::create(Size, OutContext));
+      if (!BaseObject || BaseObject->hasPrivateLinkage() ||
+          !BaseObject->getValueType()->isSized() ||
+          Size < DL.getTypeAllocSize(BaseObject->getValueType())) {
+        OutStreamer->emitELFSize(Name,
+                                 MCConstantExpr::create(Size, OutContext));
+      }
     }
   }
 }
 
 bool AsmPrinter::doFinalization(Module &M) {
   // Set the MachineFunction to nullptr so that we can catch attempted
   // accesses to MF specific features at the module level and so that
   // we can conditionalize accesses based on whether or not it is nullptr.

llvm/test/CodeGen/X86/global-alias.ll

+; RUN: llc < %s -mtriple=i386-unknown-linux-gnu | FileCheck %s
+
+%global.type = type <{ [4 x i32] }>
+@global = linkonce_odr global %global.type zeroinitializer, align 4
+@my_alias = alias [2 x i32], bitcast (i32* getelementptr inbounds (i32, i32* bitcast (%global.type* @global to i32*), i32 2) to [2 x i32]*)
+@my_alias2 = alias [4 x i32], bitcast (i32* getelementptr inbounds (i32, i32* bitcast (%global.type* @global to i32*), i32 2) to [4 x i32]*)
+@my_alias3 = alias %global.type, %global.type* @global
+
+;CHECK: .weak global
+;CHECK: .size global, 16
+;CHECK: .set my_alias, global+8
+;CHECK: .size my_alias, 8
+;CHECK: .set my_alias2, global+8
+;CHECK: .size my_alias2, 16
+;CHECK: .set my_alias3, global
+;CHECK-NOT: .size my_alias3