yanqi27 / core_analyzer

A power tool to debug memory-related issues
376 stars 72 forks source link

move get libc version to the common file #59

Closed Celthi closed 2 years ago

Celthi commented 2 years ago

In this PR, the heap_pt_malloc.cpp is added to the compiling process. It is aiming to contain all the common functions related to different pt malloc version. Now it only has the get_libc_version. By adding this file into the compiling, we can progressively consolidate all the common functions into one place. The consolidating procedure is

  1. Add the common function common_function into the heap_pt_malloc.cpp under namespace pt
  2. Replace the call site with pt::common_function
  3. Remove all the common_function

And when all the common functions are moved into heap_ptmalloc.cpp, we can consider removing the namespace pt

Celthi commented 2 years ago

Hi @yanqi27 This is the PR using the approach I discussed in https://github.com/yanqi27/core_analyzer/pull/58#issuecomment-1166311009

Celthi commented 2 years ago

We should have the following layers:

  1. Core analyzer layer like search, ref command. It only talks to an abstract heap interface or heap data structure(heap, block, reference, etc)
  2. Debugger layer like the gdb provides the debugger API. (such as read memory from an address, etc)
  3. For each kind of heap manager, there is a heap manager layer for all the heap manager versions. The heap manager layer talks to common data structures among different heap manager versions.
  4. The underhood of a specific heap.

This PR tries to address layer 3 by extracting all the common functions of pt malloc heap.

Celthi commented 2 years ago

Can we keep the src/heap_ptmalloc.cpp file for older gdb versions, 7.11.1/8.1, until they are totally phased out? We may name the new common function file something like heap_ptmalloc_common.cpp.

gdb 7.11/8.1 is not using src/heap_ptmalloc.cpp anymore: https://github.com/yanqi27/core_analyzer/blob/master/gdbplus/gdb-7.11.1/gdb/heap_ptmalloc.c https://github.com/yanqi27/core_analyzer/blob/master/gdbplus/gdb-8.1/gdb/heap_ptmalloc.c

Yes. heap_ptmalloc_common.cpp is a better name.

yanqi27 commented 2 years ago

yeah, I forgot that. In which case, I am ok to do it either way, the old name or the xx_common.