[SHT_LLVM_BB_ADDR_MAP] Fixes two bugs in decoding of PGOAnalyses in BBAddrMap. (#77139)

We had specified that `readBBAddrMap` will always keep PGOAnalyses and
BBAddrMaps the same length on success.

https://github.com/llvm/llvm-project/blob/365fbbfbcfefb8766f7716109b9c3767b58e6058/llvm/include/llvm/Object/ELFObjectFile.h#L116-L117

It turns out that this is not currently the case when no analyses exist
in a function. No test had caught it.

We also should not append PGOBBEntries when there is no BBFreq or
BrProb.

This patch adds:
* tests that PGOAnalyses and BBAddrMaps are same length even when no
analyses are enabled
* fixes decode so that PGOAnalyses and BBAddrMaps are same length
* updates test to not emit unnecessary PGOBBEntries
* fixes decode to not emit PGOBBEntries when unnecessary

Author: red1bluelost

llvm/include/llvm/Object/ELFTypes.h

@@ -885,6 +885,9 @@ struct PGOAnalysisMap {
     bool BBFreq : 1;
     bool BrProb : 1;
 
+    // True if at least one feature is enabled
+    bool anyEnabled() const { return FuncEntryCount || BBFreq || BrProb; }
+
     // Encodes to minimum bit width representation.
     uint8_t encode() const {
       return (static_cast<uint8_t>(FuncEntryCount) << 0) |

llvm/lib/Object/ELF.cpp

@@ -774,16 +774,17 @@ decodeBBAddrMapImpl(const ELFFile<ELFT> &EF,
     }
     FunctionEntries.emplace_back(Address, std::move(BBEntries));
 
-    if (FeatEnable.FuncEntryCount || FeatEnable.BBFreq || FeatEnable.BrProb) {
+    if (PGOAnalyses || FeatEnable.anyEnabled()) {
       // Function entry count
       uint64_t FuncEntryCount =
           FeatEnable.FuncEntryCount
               ? readULEB128As<uint64_t>(Data, Cur, ULEBSizeErr)
               : 0;
 
       std::vector<PGOAnalysisMap::PGOBBEntry> PGOBBEntries;
-      for (uint32_t BlockIndex = 0; !MetadataDecodeErr && !ULEBSizeErr && Cur &&
-                                    (BlockIndex < NumBlocks);
+      for (uint32_t BlockIndex = 0;
+           (FeatEnable.BBFreq || FeatEnable.BrProb) && !MetadataDecodeErr &&
+           !ULEBSizeErr && Cur && (BlockIndex < NumBlocks);
            ++BlockIndex) {
         // Block frequency
         uint64_t BBF = FeatEnable.BBFreq

llvm/unittests/Object/ELFObjectFileTest.cpp

@@ -1017,10 +1017,23 @@ TEST(ELFObjectFileTest, ReadPGOAnalysisMap) {
               BrProb:      0xffffffff
           - BBFreq:         1000
             Successors:    []
-)");
+  - Name: .llvm_bb_addr_map_5
+    Type: SHT_LLVM_BB_ADDR_MAP
+  # Link: 0 (by default, can be overriden)
+    Entries:
+      - Version: 2
+        Address: 0x55555
+        Feature: 0x0
+        BBEntries:
+          - ID:            2
+            AddressOffset: 0x0
+            Size:          0x2
+            Metadata:      0x4
+    PGOAnalyses: [{}]
+ )");
 
   BBAddrMap E1(0x11111, {{1, 0x0, 0x1, {false, true, false, false, false}}});
-  PGOAnalysisMap P1 = {892, {{}}, {true, false, false}};
+  PGOAnalysisMap P1 = {892, {}, {true, false, false}};
   BBAddrMap E2(0x22222, {{2, 0x0, 0x2, {false, false, true, false, false}}});
   PGOAnalysisMap P2 = {{}, {{BlockFrequency(343), {}}}, {false, true, false}};
   BBAddrMap E3(0x33333, {{0, 0x0, 0x3, {false, true, true, false, false}},
@@ -1049,16 +1062,18 @@ TEST(ELFObjectFileTest, ReadPGOAnalysisMap) {
        {BlockFrequency(18), {{3, BranchProbability::getRaw(0xffff'ffff)}}},
        {BlockFrequency(1000), {}}},
       {true, true, true}};
+  BBAddrMap E5(0x55555, {{2, 0x0, 0x2, {false, false, true, false, false}}});
+  PGOAnalysisMap P5 = {{}, {}, {false, false, false}};
 
-  std::vector<BBAddrMap> Section0BBAddrMaps = {E4};
+  std::vector<BBAddrMap> Section0BBAddrMaps = {E4, E5};
   std::vector<BBAddrMap> Section1BBAddrMaps = {E3};
   std::vector<BBAddrMap> Section2BBAddrMaps = {E1, E2};
-  std::vector<BBAddrMap> AllBBAddrMaps = {E1, E2, E3, E4};
+  std::vector<BBAddrMap> AllBBAddrMaps = {E1, E2, E3, E4, E5};
 
-  std::vector<PGOAnalysisMap> Section0PGOAnalysisMaps = {P4};
+  std::vector<PGOAnalysisMap> Section0PGOAnalysisMaps = {P4, P5};
   std::vector<PGOAnalysisMap> Section1PGOAnalysisMaps = {P3};
   std::vector<PGOAnalysisMap> Section2PGOAnalysisMaps = {P1, P2};
-  std::vector<PGOAnalysisMap> AllPGOAnalysisMaps = {P1, P2, P3, P4};
+  std::vector<PGOAnalysisMap> AllPGOAnalysisMaps = {P1, P2, P3, P4, P5};
 
   auto DoCheckSucceeds =
       [&](StringRef YamlString, std::optional<unsigned> TextSectionIndex,
@@ -1081,6 +1096,10 @@ TEST(ELFObjectFileTest, ReadPGOAnalysisMap) {
         if (ExpectedPGO) {
           EXPECT_EQ(BBAddrMaps->size(), PGOAnalyses.size());
           EXPECT_EQ(PGOAnalyses, *ExpectedPGO);
+          for (auto &&[BB, PGO] : llvm::zip(*BBAddrMaps, PGOAnalyses)) {
+            if (PGO.FeatEnable.BBFreq || PGO.FeatEnable.BrProb)
+              EXPECT_EQ(BB.getBBEntries().size(), PGO.BBEntries.size());
+          }
         }
       };
 
@@ -1132,9 +1151,9 @@ TEST(ELFObjectFileTest, ReadPGOAnalysisMap) {
     Link: 10
 )";
 
-  DoCheckFails(InvalidLinkedYamlString, /*TextSectionIndex=*/4,
+  DoCheckFails(InvalidLinkedYamlString, /*TextSectionIndex=*/5,
                "unable to get the linked-to section for "
-               "SHT_LLVM_BB_ADDR_MAP section with index 4: invalid section "
+               "SHT_LLVM_BB_ADDR_MAP section with index 5: invalid section "
                "index: 10");
   // Linked sections are not checked when we don't target a specific text
   // section.
@@ -1150,7 +1169,7 @@ TEST(ELFObjectFileTest, ReadPGOAnalysisMap) {
 )";
 
   DoCheckFails(TruncatedYamlString, /*TextSectionIndex=*/std::nullopt,
-               "unable to read SHT_LLVM_BB_ADDR_MAP section with index 4: "
+               "unable to read SHT_LLVM_BB_ADDR_MAP section with index 5: "
                "unable to decode LEB128 at offset 0x0000000a: malformed "
                "uleb128, extends past end");
   // Check that we can read the other section's bb-address-maps which are