[InstrProfiling] Use nobits for __llvm_prf_cnts with binutils >=2.28

[phosek]

Summary

binutils <2.28 have a bug which causes start_/stop_ symbols have
incorrect address for nobits section which causes counters to come up
as empty in raw profiles.

[phosek]

Updated test and extra comment.

[kazu]

This patch fixes the problem I was having while building an instrumented binary.

[davidxl]

lgtm

[MaskRay]

Do you have a link for the bug? Might to good mentioning in the description.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

445

Nit: end a complete sentence with a period.

llvm/test/Instrumentation/InstrProfiling/X86/nobits.ll

2

Nit: you can drop -unknown-linux-gnu because this tests generic ELF behavior.

[Conanap]

If this patch could be pushed soon that'd be great; we need it to unblock PowerPC buildbots.

[phosek]

In D97336#2585434, @Conanap wrote:

If this patch could be pushed soon that'd be great; we need it to unblock PowerPC buildbots.

I've reverted the original change D97110, are you still seeing the issue?

[Conanap]

In D97336#2585442, @phosek wrote:

In D97336#2585434, @Conanap wrote:

If this patch could be pushed soon that'd be great; we need it to unblock PowerPC buildbots.

I've reverted the original change D97110, are you still seeing the issue?

Apologies it has indeed gone away. Thanks!

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

return TargetLoweringObjectFile::
       getTTypeReference(MCSymbolRefExpr::create(SSym, getContext()),
                         Encoding & ~DW_EH_PE_indirect, Streamer);
   }
 
   return TargetLoweringObjectFile::getTTypeGlobalReference(GV, Encoding, TM,
                                                            MMI, Streamer);
 }
 
-static SectionKind getELFKindForNamedSection(StringRef Name, SectionKind K) {
+static SectionKind getELFKindForNamedSection(MCContext &Ctx, StringRef Name,
+                                             SectionKind K) {
   // N.B.: The defaults used in here are not the same ones used in MC.
   // We follow gcc, MC follows gas. For example, given ".section .eh_frame",
   // both gas and MC will produce a section with no flags. Given
   // section(".eh_frame") gcc will produce:
   //
   //   .section   .eh_frame,"a",@progbits
 
   if (Name == getInstrProfSectionName(IPSK_covmap, Triple::ELF,
                                       /*AddSegmentInfo=*/false) ||
       Name == getInstrProfSectionName(IPSK_covfun, Triple::ELF,
                                       /*AddSegmentInfo=*/false) ||
       Name == ".llvmbc" || Name == ".llvmcmd")
     return SectionKind::getMetadata();
 
+  // GNU binutils ld.bfd <2.28 has a bug which causes __start_/__stop_ symbols
+  // for SHT_NOBITS sections to have incorrect address
   if (Name == getInstrProfSectionName(IPSK_cnts, Triple::ELF,
-                                      /*AddSegmentInfo=*/false))
+                                      /*AddSegmentInfo=*/false) &&
+      Ctx.getAsmInfo()->binutilsIsAtLeast(2, 28))
     return SectionKind::getBSS();
 
   if (Name.empty() || Name[0] != '.') return K;
 
   // Default implementation based on some magic section names.
   if (Name == ".bss" ||
       Name.startswith(".bss.") ||
       Name.startswith(".gnu.linkonce.b.") ||
if (GV && GV->hasImplicitSection()) {
     }
   }
   const Function *F = dyn_cast<Function>(GO);
   if (F && F->hasFnAttribute("implicit-section-name")) {
     SectionName = F->getFnAttribute("implicit-section-name").getValueAsString();
   }
 
   // Infer section flags from the section name if we can.
-  Kind = getELFKindForNamedSection(SectionName, Kind);
+  Kind = getELFKindForNamedSection(getContext(), SectionName, Kind);
 
   StringRef Group = "";
   bool IsComdat = false;
   unsigned Flags = getELFSectionFlags(Kind);
   if (const Comdat *C = getELFComdat(GO)) {
     Group = C->getName();
     IsComdat = C->getSelectionKind() == Comdat::Any;
     Flags |= ELF::SHF_GROUP;

llvm/test/Instrumentation/InstrProfiling/X86/nobits.ll

 ;; Ensure that SHT_NOBITS section type is set for __llvm_prf_cnts in ELF.
-; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -binutils-version=2.27 | FileCheck %s --check-prefix=OLD
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -binutils-version=2.28 | FileCheck %s --check-prefix=NEW
 
 @__profc_foo = hidden global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", align 8
 
 define void @foo() {
   %pgocount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0), align 4
   %1 = add i64 %pgocount, 1
   store i64 %1, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0), align 4
   ret void
 }
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
 
-; CHECK: .section __llvm_prf_cnts,"aw",@nobits
+; OLD: .section __llvm_prf_cnts,"aw",@progbits
+; NEW: .section __llvm_prf_cnts,"aw",@nobits