src: Fix bugs and refactor NativeSymbolDebuggingContext::GetLoadedLibraries

[Whitecx]

tldr:

GetLoadedLibraries is used in process.report.GetReport to generate a list of loaded libraries. This PR fixes the following:

• Random failures when retrieving module file paths
• Retrieve only up to half the max file path length intended
• Allocating unused memory for module handles
• Memory leak from converting file path wchar* strings to utf8 std::strings

--

Refactor:

1. GetModuleFileNameExW can fail randomly: Microsoft's Documentation recommends using GetModuleFileNameW over GetModuleFileNameExW when a process is trying to get a list of it's own dlls. GetModuleFileNameExW can fail unexpectedly for some calls.

Sources from Docs + Raymond Chen

• "To retrieve the name of a module in the current process, use the GetModuleFileName function" -- WinApi Docs: Remarks

• Raymond Chen explains how GetModuleFileNameExW can fail randomly, in a way that GetModuleFileNameW avoids -- Article

Bug Fixes:

1. Unused Buffer Memory Allocated: The constructor for modules allocates memory by multiplying the constructor arg size_t n by the sizeof it's type. In this case, modules would be allocated with size_1 * sizeof(HMODULE) bytes. But EnumProcessModules sets size_1 to the number of bytes that need to be allocated for the module buffer. Using size_1/sizeof(HMODULE) gives the constructor what it's asking for i.e the number of HMODULES to allocate space for.

Exapnd to Step through MallocedBuffer constructor behavior

src/util.h MallocedBuffer

  explicit MallocedBuffer(size_t size) : data(Malloc<T>(size)), size(size) {}

src/util-inl.h UncheckedMalloc

template <typename T>
inline T* UncheckedMalloc(size_t n) {
  return UncheckedRealloc<T>(nullptr, n);
}

src/util-inl.h UncheckedRealloc

template <typename T>
T* UncheckedRealloc(T* pointer, size_t n) {
  size_t full_size = MultiplyWithOverflowCheck(sizeof(T), n); // <- Expects n to be number of T elements
/* other code not shown... */

  void* allocated = realloc(pointer, full_size);

/* other code not shown... */

  return static_cast<T*>(allocated);
}

src/util-inl.h MultiplyWithOverflowCheck

template <typename T>
inline T MultiplyWithOverflowCheck(T a, T b) {
  auto ret = a * b;
  if (a != 0)
    CHECK_EQ(b, ret / a);

  return ret;
}

2. Unnecessary Module path truncation: GetModuleFileName is called using array_size(module_name) / sizeof(WCHAR) for the nSize param. nSize is supposed to be the number of characters in the wchar array supplied. module_name is already a WCHAR array, so nSize should just be array_size(module_name). The current code causes GetModuleFileName to truncate after reaching half the capacity of the buffer (i.e half of MAX_PATH)

3. Memory Leak: When converting the file path written by GetModuleFileName from a wchar array to a char array with UTF8 encoding, a char array str is added to the heap using new. Next, it's added to list, a std::vector<std::string> using emplace_back. emplace_back will pass str intro the std::string constructor. This constructor is defined to copy values from a null terminated char * array into the std::string. I assume str was created on the heap so that it could be dynamically allocated based on the number of bytes returned by WideCharToMultiByte. Once it's added to list, the memory is no longer needed, but is never deleted from the heap.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.25%. Comparing base (668a0b8) to head (80624e0).
Report is 11 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #57738   +/-   ##
=======================================
  Coverage   90.24%   90.25%           
=======================================
  Files         630      630           
  Lines      184949   184989   +40     
  Branches    36207    36218   +11     
=======================================
+ Hits       166902   166954   +52     
+ Misses      11002    10997    -5     
+ Partials     7045     7038    -7     

Files with missing lines	Coverage Δ
src/debug_utils.cc	60.90% <ø> (ø)

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.