Skip to content

[NFC][SHT_LLVM_BB_ADDR_MAP] Define and use constructor and accessors for BBAddrMap fields. #72689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

rlavaee
Copy link
Contributor

@rlavaee rlavaee commented Nov 17, 2023

The fields are still kept as public for now since our tooling accesses them. Will change them to private visibility in a later patch.

…for BBAddrMap fields.

The fields are still kept as public for now since our tooling accesses them. Will change them
to private visibility in a later patch.
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2023

@llvm/pr-subscribers-llvm-binary-utilities

Author: Rahman Lavaee (rlavaee)

Changes

The fields are still kept as public for now since our tooling accesses them. Will change them to private visibility in a later patch.


Full diff: https://github.com/llvm/llvm-project/pull/72689.diff

4 Files Affected:

  • (modified) llvm/include/llvm/Object/ELFTypes.h (+12-2)
  • (modified) llvm/lib/Object/ELF.cpp (+1-1)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+2-2)
  • (modified) llvm/unittests/Object/ELFObjectFileTest.cpp (+4-4)
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index 45fc52288bdd4c0..4670abc867de63c 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -794,7 +794,6 @@ template <class ELFT> struct Elf_Mips_ABIFlags {
 
 // Struct representing the BBAddrMap for one function.
 struct BBAddrMap {
-  uint64_t Addr; // Function address
   // Struct representing the BBAddrMap information for one basic block.
   struct BBEntry {
     struct Metadata {
@@ -856,13 +855,24 @@ struct BBAddrMap {
     bool canFallThrough() const { return MD.CanFallThrough; }
     bool hasIndirectBranch() const { return MD.HasIndirectBranch; }
   };
-  std::vector<BBEntry> BBEntries; // Basic block entries for this function.
+
+  BBAddrMap(uint64_t Addr, std::vector<BBEntry> BBEntries)
+      : Addr(Addr), BBEntries(std::move(BBEntries)) {}
+
+  // Returns the address of the corresponding function.
+  uint64_t getFunctionAddress() const { return Addr; }
+
+  // Returns the basic block entries for this function.
+  const std::vector<BBEntry> &getBBEntries() const { return BBEntries; }
 
   // Equality operator for unit testing.
   bool operator==(const BBAddrMap &Other) const {
     return Addr == Other.Addr && std::equal(BBEntries.begin(), BBEntries.end(),
                                             Other.BBEntries.begin());
   }
+
+  uint64_t Addr;                  // Function address
+  std::vector<BBEntry> BBEntries; // Basic block entries for this function.
 };
 
 } // end namespace object.
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index 0d1862e573712f5..1d73a6ffa73f5f9 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -745,7 +745,7 @@ ELFFile<ELFT>::decodeBBAddrMap(const Elf_Shdr &Sec,
       }
       BBEntries.push_back({ID, Offset, Size, *MetadataOrErr});
     }
-    FunctionEntries.push_back({Address, std::move(BBEntries)});
+    FunctionEntries.emplace_back(Address, std::move(BBEntries));
   }
   // Either Cur is in the error state, or we have an error in ULEBSizeErr or
   // MetadataDecodeErr (but not both), but we join all errors here to be safe.
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index a828f3f5d761af4..2b7dee7bf46b953 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1275,8 +1275,8 @@ collectBBAddrMapLabels(const std::unordered_map<uint64_t, BBAddrMap> &AddrToBBAd
   auto Iter = AddrToBBAddrMap.find(StartAddress);
   if (Iter == AddrToBBAddrMap.end())
     return;
-  for (const BBAddrMap::BBEntry &BBEntry : Iter->second.BBEntries) {
-    uint64_t BBAddress = BBEntry.Offset + Iter->second.Addr;
+  for (const BBAddrMap::BBEntry &BBEntry : Iter->second.getBBEntries()) {
+    uint64_t BBAddress = BBEntry.Offset + Iter->second.getFunctionAddress();
     if (BBAddress >= EndAddress)
       continue;
     Labels[BBAddress].push_back(("BB" + Twine(BBEntry.ID)).str());
diff --git a/llvm/unittests/Object/ELFObjectFileTest.cpp b/llvm/unittests/Object/ELFObjectFileTest.cpp
index fe5ce2154dc7444..17402f39a5df834 100644
--- a/llvm/unittests/Object/ELFObjectFileTest.cpp
+++ b/llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -661,10 +661,10 @@ TEST(ELFObjectFileTest, ReadBBAddrMap) {
             Metadata:      0x18
 )");
 
-  BBAddrMap E1 = {0x11111, {{1, 0x0, 0x1, {false, true, false, false, false}}}};
-  BBAddrMap E2 = {0x22222, {{2, 0x0, 0x2, {false, false, true, false, false}}}};
-  BBAddrMap E3 = {0x33333, {{0, 0x0, 0x3, {false, true, true, false, false}}}};
-  BBAddrMap E4 = {0x44444, {{0, 0x0, 0x4, {false, false, false, true, true}}}};
+  BBAddrMap E1(0x11111, {{1, 0x0, 0x1, {false, true, false, false, false}}});
+  BBAddrMap E2(0x22222, {{2, 0x0, 0x2, {false, false, true, false, false}}});
+  BBAddrMap E3(0x33333, {{0, 0x0, 0x3, {false, true, true, false, false}}});
+  BBAddrMap E4(0x44444, {{0, 0x0, 0x4, {false, false, false, true, true}}});
 
   std::vector<BBAddrMap> Section0BBAddrMaps = {E4};
   std::vector<BBAddrMap> Section1BBAddrMaps = {E3};

@rlavaee rlavaee merged commit fab690d into llvm:main Nov 17, 2023
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
…for BBAddrMap fields. (llvm#72689)

The fields are still kept as public for now since our tooling accesses
them. Will change them to private visibility in a later patch.
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…for BBAddrMap fields. (llvm#72689)

The fields are still kept as public for now since our tooling accesses
them. Will change them to private visibility in a later patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants