https://reviews.llvm.org/D144857 added the README for the tool.

@Orlando Thanks very much for your review.

These are the next 2 patches:

To all reviewers,

Updated patch to reflect changes from 08a-memory-management.

@dblaikie, @Orlando Thanks very much for your valuable feedback.

@Orlando Do you have any additional comments on the updated patch?

Update the patch to reflect a change based on @dblaikie feedback.

@dblaikie: Thanks for your valuable feedback.

Address feedback from @dblaikie.

@jmorse Thanks for your review.

In D143716#4121360, @h-vetinari wrote:

Tested this patch in https://github.com/conda-forge/llvmdev-feedstock/pull/198, can confirm it works. Thanks a lot!

@dblaikie Do you have any additional questions or concerns that I have not answered it? Thanks.

In D137933#4096494, @Orlando wrote:

That's all of my concerns addressed now, so this LGTM. Thanks @CarlosAlbertoEnciso for your detailed responses.

I won't hit accept though in case @dblaikie has further comments on this new approach.

Address the feedback from @Orlando:

LGTM

May be add some explanatory notes for the change to 'private'.

This patch uses the new SpecificBumpPtrAllocator functionality to manage the creation of logical elements.

I have explored several options for how to model the pointer ownership, and I think SpecificBumpPtrAllocator is probably the best, my reasoning is below.

Address @dblaikie concerns on the use of a bucket with smart pointers.

@dblaikie: I am sorry for my delay in addressing your feedback/concerns. I just came last week from a long annual leave (from Nov 19th).

To all reviewers,

To all reviewers,

In D138092#3931440, @dblaikie wrote:

Sounds good (this sort of cleanup is fine for post-commit review, FWIW)

Updated patch addressing @dblaikie review.

The main logical unit is the LVReader, which is the main access point from the user point of view. Its functions are:

• Load the binary file
• Parse the debug information
• Create the logical view which is a tree composed of logical elements (LVScopes, LVSymbols, LVTypes and LVLines, etc.). Depending on the command line options, additional information is added to the those logical elements.
• Print the logical view.
• Select logical elements using user criterias.
• Compare two `logical views'.

In D137933#3925820, @dblaikie wrote:

(some suggestions here might be too noisy to do all in this review - maybe a matter of splitting things up - the discussion about new data structures/changes in ownership might be one of those worth breaking down into smaller steps, not sure)

Perhaps some general discussion about the ownership/lifetime model of everything here would be useful to add to the patch description to get a general lay of the land?

@dblaikie A new patch has been uploaded that addresses the memory management issues: https://reviews.llvm.org/D137933 (10 Smart pointers).

@mgorny Can you update and close https://github.com/llvm/llvm-project/issues/58758 after your fix is landed. Thanks.

@mgorny submitted a correct patch.
https://reviews.llvm.org/D137400

LGTM.

In D137400#3907490, @mgorny wrote:

That said, I've only checked format strings in LVScope.cpp. I suspect other files may suffer from similar problems. If you replace llvm::format() with plain printf(), 32-bit GCC warns about type mismatches.

In D137242#3907482, @mgorny wrote:

D137400 is the proper fix.

In D137242#3902403, @mgorny wrote:

I suppose this makes sense as a stop-gap measure but the option should be fixed to work on all platforms.

In D137156#3902189, @jryans wrote:

In D137156#3901310, @CarlosAlbertoEnciso wrote:

Based on your suggestion, I can create a GitHub issue to include all the information and add that link to each of the entries in the reviews that refer to the memory handling.

I think it would be best to prioritise creating this issue quite soon (in the next few days), as it would be good to have on hand when discussing the current status of any other memory-related issues that arise with the tool.

Updated patch that includes:

The test case tools/llvm-debuginfo-analyzer/DWARF/06-dwarf-full-logical-view.test was disabled due to unexpected output:

In D137156#3899661, @vitalybuka wrote:

LGTM if this fixes the bug

Thanks very much for your review.

Still the same questing as before: Can this code avoid using directly new/delete and rely on unique_ptrs?

In D137156#3899106, @jryans wrote:

Thanks, this looks reasonable to me! 🙂

Were you able to reproduce the failure locally?

The test case tools/llvm-debuginfo-analyzer/DWARF/pr-incorrect-logical-instructions.test was disabled due to memory leak detected by the sanitizers.

After installing gcc-i686-linux-gnu and using

CC='/usr/bin/i686-linux-gnu-gcc -m32' CXX='/usr/bin/i686-linux-gnu-g++ -m32' cmake ../llvm -GNinja -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=MinSizeRel -DLLVM_TARGETS_TO_BUILD=X86 -DCLANG_ENABLE_ARCMT=OFF -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_CCACHE_BUILD=ON -DLLVM_ENABLE_PROJECTS="llvm" -DLLVM_BUILD_TESTS=OFF -DLLVM_HOST_TRIPLE=i686-pc-linux-gnu

I can reproduce the problem with

In D125783#3896360, @mgorny wrote:

As you are building on i386, can I have the CMake configuration?

Just to be clear, I'm building 32-bit multilib on amd64.
But I can also reproduce in my working copy that's configured as:

CC=i686-pc-linux-gnu-gcc CXX=i686-pc-linux-gnu-g++ cmake ../llvm -G Ninja -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=MinSizeRel -DLLVM_TARGETS_TO_BUILD=X86 -DCLANG_ENABLE_ARCMT=OFF -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_CCACHE_BUILD=ON -DLLVM_ENABLE_PROJECTS='llvm' -DLLVM_BUILD_TESTS=OFF -DLLVM_HOST_TRIPLE=i686-pc-linux-gnu

where i686-pc-linux-gnu-* wrappers simply call the respective GCC executables with -m32.

In D125783#3893003, @dblaikie wrote:

A bit concerned about two things that happened here (though, apologies as I haven't otherwise been following the review):

Very much appreciated your input and I fully understand your concerns about code reviews and large/complex patches.

In D125783#3894727, @mgorny wrote:

This is also broken on i386 (so perhaps on all 32-bit platforms?):

Input was:
           76:        189 (  0.00%) : [0x000000000b][001]              {CompileUnit} 'test.cpp' 
           77:        110 ( -0.00%) : [0x000000002a][002]      2         {Function} extern not_inlined 'foo' -> [0x0000000099]'int' 
           78:         27 (-26815615859885194199148049996411692254958731641184786755447122887443528060147093953603748596333806855380063716372972101707507765623893139892867298012168192.00%) : [0x0000000071][003]                  {Block}

Commit: https://reviews.llvm.org/rGcddea67663655591b3d650d81d6546abbdc50ae5