[PATCH] D124560: [llvm-objdump] Symbolize branch targets and basic block addresses based on the bb-address-map when present in the object file.

[Rahman Lavaee via Phabricator via llvm-commits]

rahmanl added inline comments.


================
Comment at: llvm/unittests/Object/ELFObjectFileTest.cpp:608
+// Test for the ELFObjectFile::readBBAddrMap API.
+TEST(ELFObjectFileTest, InvalidReadBBAddrMap) {
+  StringRef CommonYamlString(R"(
----------------
jhenderson wrote:
> Do you have a test case where the TextSectionIndex is specified, but there are no matching sections?
> 
> I'd also expect to see checks that the retrieved entries match what was expected, but don't see any of those.
> Do you have a test case where the TextSectionIndex is specified, but there are no matching sections?

Would you please clarify?

I do have tests for when the requested `TextSectionIndex` doesn't match with `sh_link` of the bb-address-map sections. The call to `readBBAddrMap` would succeed and return an empty vector in such case -- indicating that no BB-address-map was found for that section. Do you suggest I return Error in this case? 

Or do you mean a test for when the requested `TextSectionIndex` is not a valid text section?

> I'd also expect to see checks that the retrieved entries match what was expected, but don't see any of those.

Sure. I added additional cases and also support for equality of `BBAddrMap`s.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124560/new/

https://reviews.llvm.org/D124560