[llvm] [SHT_LLVM_BB_ADDR_MAP] Add assertion and clarify docstring (PR #77374)

[Aiden Grossman via llvm-commits]

https://github.com/boomanaiden154 updated https://github.com/llvm/llvm-project/pull/77374

>From 1d4b0fbe031c4b303df9d4bfbec8ac128cb47fbc Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Mon, 8 Jan 2024 13:03:20 -0800
Subject: [PATCH 1/3] [SHT_LLVM_BB_ADDR_MAP] Add assertion and clarify
 docstring

This patch adds an assertion to readBBAddrMapImpl to confirm that
PGOAnalyses and BBAddrMaps are of the same size when PGO information is
requested (part of the API contract). This patch also updates the
docstring for readBBAddrMap to better clarify what is guaranteed.
---
 llvm/include/llvm/Object/ELFObjectFile.h | 11 ++++++-----
 llvm/lib/Object/ELFObjectFile.cpp        |  4 ++++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h
index 86f42654f8af8d..5182c6cf62c2c5 100644
--- a/llvm/include/llvm/Object/ELFObjectFile.h
+++ b/llvm/include/llvm/Object/ELFObjectFile.h
@@ -110,11 +110,12 @@ class ELFObjectFileBase : public ObjectFile {
   Expected<std::vector<VersionEntry>> readDynsymVersions() const;
 
   /// Returns a vector of all BB address maps in the object file. When
-  // `TextSectionIndex` is specified, only returns the BB address maps
-  // corresponding to the section with that index. When `PGOAnalyses`is
-  // specified, the vector is cleared then filled with extra PGO data.
-  // `PGOAnalyses` will always be the same length as the return value on
-  // success, otherwise it is empty.
+  /// `TextSectionIndex` is specified, only returns the BB address maps
+  /// corresponding to the section with that index. When `PGOAnalyses`is
+  /// specified, the vector is cleared then filled with extra PGO data.
+  /// When PGO information is requested (`PGOAnalyses` is not nullptr),
+  /// `PGOAnalyses` will always be the same length as the return value,
+  /// assuming no error occurs. Upon failure, `PGOAnalyses` will be empty.
   Expected<std::vector<BBAddrMap>>
   readBBAddrMap(std::optional<unsigned> TextSectionIndex = std::nullopt,
                 std::vector<PGOAnalysisMap> *PGOAnalyses = nullptr) const;
diff --git a/llvm/lib/Object/ELFObjectFile.cpp b/llvm/lib/Object/ELFObjectFile.cpp
index 95c4f9f8545db2..f43fdf8be58541 100644
--- a/llvm/lib/Object/ELFObjectFile.cpp
+++ b/llvm/lib/Object/ELFObjectFile.cpp
@@ -830,6 +830,10 @@ Expected<std::vector<BBAddrMap>> static readBBAddrMapImpl(
     std::move(BBAddrMapOrErr->begin(), BBAddrMapOrErr->end(),
               std::back_inserter(BBAddrMaps));
   }
+  if (PGOAnalyses)
+    assert(PGOAnalyses->size() == BBAddrMaps.size() &&
+           "The same number of BBAddrMaps and PGOAnalysisMaps should be "
+           "returned when PGO information is requested");
   return BBAddrMaps;
 }
 

>From d55db65979ec5717a55bde58aaee9602b589d0e8 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Wed, 17 Jan 2024 01:30:47 -0800
Subject: [PATCH 2/3] Note that PGOAnalyses will be cleared explicitly

---
 llvm/include/llvm/Object/ELFObjectFile.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h
index 5182c6cf62c2c5..f09b7ac5c8b1fe 100644
--- a/llvm/include/llvm/Object/ELFObjectFile.h
+++ b/llvm/include/llvm/Object/ELFObjectFile.h
@@ -115,7 +115,7 @@ class ELFObjectFileBase : public ObjectFile {
   /// specified, the vector is cleared then filled with extra PGO data.
   /// When PGO information is requested (`PGOAnalyses` is not nullptr),
   /// `PGOAnalyses` will always be the same length as the return value,
-  /// assuming no error occurs. Upon failure, `PGOAnalyses` will be empty.
+  /// assuming no error occurs. Upon failure, `PGOAnalyses` will be emptied.
   Expected<std::vector<BBAddrMap>>
   readBBAddrMap(std::optional<unsigned> TextSectionIndex = std::nullopt,
                 std::vector<PGOAnalysisMap> *PGOAnalyses = nullptr) const;

>From 35d75f321208400c933ca80f03ac865983c75cb0 Mon Sep 17 00:00:00 2001
From: Aiden Grossman <agrossman154 at yahoo.com>
Date: Fri, 19 Jan 2024 00:25:56 -0800
Subject: [PATCH 3/3] Reword sentences to improve clarity

---
 llvm/include/llvm/Object/ELFObjectFile.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/Object/ELFObjectFile.h b/llvm/include/llvm/Object/ELFObjectFile.h
index f09b7ac5c8b1fe..7124df50b561db 100644
--- a/llvm/include/llvm/Object/ELFObjectFile.h
+++ b/llvm/include/llvm/Object/ELFObjectFile.h
@@ -112,10 +112,10 @@ class ELFObjectFileBase : public ObjectFile {
   /// Returns a vector of all BB address maps in the object file. When
   /// `TextSectionIndex` is specified, only returns the BB address maps
   /// corresponding to the section with that index. When `PGOAnalyses`is
-  /// specified, the vector is cleared then filled with extra PGO data.
-  /// When PGO information is requested (`PGOAnalyses` is not nullptr),
-  /// `PGOAnalyses` will always be the same length as the return value,
-  /// assuming no error occurs. Upon failure, `PGOAnalyses` will be emptied.
+  /// specified (PGOAnalyses is not nullptr), the vector is cleared then filled
+  /// with extra PGO data. `PGOAnalyses` will always be the same length as the
+  /// return value when it is requested assuming no error occurs. Upon failure,
+  /// `PGOAnalyses` will be emptied.
   Expected<std::vector<BBAddrMap>>
   readBBAddrMap(std::optional<unsigned> TextSectionIndex = std::nullopt,
                 std::vector<PGOAnalysisMap> *PGOAnalyses = nullptr) const;