ShowGlobals tool to dump large/repeated Win32 globals

tools/win/ShowGlobals/ShowGlobals.cc

+// Copyright (c) 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This tool scans a PDB file and prints out information about 'interesting'
+// global variables. This includes duplicates and large globals. This is often
+// helpful inunderstanding code bloat or finding inefficient globals.
+//
+// Duplicate global variables often happen when constructs like this are placed
+// in a header file:
+//
+//     const double sqrt_two = sqrt(2.0);
+//
+// Many (although usually not all) of the translation units that include this
+// header file will get a copy of sqrt_two, possibly including an initializer.
+// Because 'const' implies 'static' there are no warnings or errors from the
+// linker. This duplication can happen with float/double, structs and classes,
+// and arrays - any non-integral type.
+//
+// Global variables are not necessarily a problem but it is useful to understand
+// them, and monitoring their changes can be instructive.
+
+#include <dia2.h>
+#include <stdio.h>
+
+#include <algorithm>
+#include <vector>
+
+// Helper function for comparing strings - returns a strcmp/wcscmp compatible
+// value.
+int StringCompare(const std::wstring& lhs, const std::wstring& rhs) {
+  return wcscmp(lhs.c_str(), rhs.c_str());
+}
+
+// Use this struct to record data about symbols for sorting and analysis.
+struct SymbolData {
+  SymbolData(DWORD size, DWORD section, const wchar_t* name)
+      : size(size), section(section), name(name) {}
+
+  DWORD size;
+  DWORD section;
+  std::wstring name;
+};
+
+// Comparison function for when sorting symbol data by name, in order to allow
+// looking for duplicate symbols. It uses the symbol size as a tiebreaker. This
+// is necessary because sometimes there are symbols with matching names but
+// different sizes in which case they aren't actually duplicates. These false
+// positives happen because namespaces are omitted from the symbol names that
+// DIA2 returns.
+bool NameCompare(const SymbolData& lhs, const SymbolData& rhs) {
+  int nameCompare = StringCompare(lhs.name, rhs.name);
+  if (nameCompare == 0)
+    return lhs.size < rhs.size;
+  return nameCompare < 0;
+}
+
+// Comparison function for when sorting symbols by size, in order to allow
+// finding the largest global variables. Use the symbol names as a tiebreaker
+// in order to get consistent ordering.
+bool SizeCompare(const SymbolData& lhs, const SymbolData& rhs) {
+  if (lhs.size == rhs.size) {
+    return StringCompare(lhs.name, rhs.name) < 0;
+  }

[stanisc, 2016/12/19 22:11:40]

Maybe skip braces to make this similar to NameCompare

[brucedawson, 2016/12/19 23:00:51]

Done.

+  return lhs.size < rhs.size;
+}
+
+// Use this struct to store data about repeated globals, for later sorting.
+struct RepeatData {
+  RepeatData(size_t repeat_count, DWORD bytes_wasted, const std::wstring& name)
+      : repeat_count(repeat_count), bytes_wasted(bytes_wasted), name(name) {}
+  bool operator<(const RepeatData& rhs) {
+    return bytes_wasted < rhs.bytes_wasted;
+  }
+
+  size_t repeat_count;
+  DWORD bytes_wasted;
+  std::wstring name;
+};
+
+bool DumpInterestingGlobals(IDiaSymbol* global, const wchar_t* filename) {
+  wprintf(L"#Dups\tDupSize\tSize\tSection\tSymbol name\tPDB name\n");
+
+  // How many bytes must be wasted on repeats before being listed.
+  const int kWastageThreshold = 100;
+  // How big must an individual symbol be before being listed.
+  const int kBigSizeThreshold = 500;
+
+  std::vector<SymbolData> symbols;
+  std::vector<RepeatData> repeats;
+
+  IDiaEnumSymbols* enum_symbols;
+  HRESULT result =
+      global->findChildren(SymTagData, NULL, nsNone, &enum_symbols);
+  if (FAILED(result)) {
+    wprintf(L"ERROR - DumpInterestingGlobals() returned no symbols.\n");
+    return false;
+  }
+
+  IDiaSymbol* symbol;
+  ULONG celt = 0;
+  while (SUCCEEDED(enum_symbols->Next(1, &symbol, &celt)) && (celt == 1)) {
+    // If we get get_length on symbol it works for functions but not for
+    // data. For some reason for data we have to call get_type() to get
+    // another IDiaSymbol object which we can query for length.
+    IDiaSymbol* type_symbol;

[chengx, 2016/12/19 21:48:02]

I think type_symbol needs to be freed to avoid memory leak?

[brucedawson, 2016/12/19 23:00:51]

Yep. The original sample code used manual calls to Release. I've updated my code
to use CComPtr and CComBSTR, although paradoxically the DIA2 APIs handle this
poorly.

+    symbol->get_type(&type_symbol);
+
+    ULONGLONG size = 0;
+    type_symbol->get_length(&size);
+
+    // Use -1 and -2 as canary values to indicate various failures.
+    DWORD section = (DWORD)-1;
+    if (symbol->get_addressSection(&section) != S_OK)
+      section = -2;

[stanisc, 2016/12/19 22:11:40]

Should this need (DWORD) cast as well?

[brucedawson, 2016/12/19 23:00:51]

Done. And changed to static_cast.

+
+    BSTR name;
+    if (symbol->get_name(&name) == S_OK) {
+      symbols.push_back(SymbolData((DWORD)size, section, name));

[chengx, 2016/12/19 21:48:02]

Not very familiar with BSTR. But no conversion needed between BSTR and
std::wstring which is the type of name in SymbolData?

[brucedawson, 2016/12/19 23:00:51]

BSTR is basically a null-terminated string with weird memory management
semantics, so std::wstring sees it as wchar_t*.

+      ::SysFreeString(name);
+    }
+
+    symbol->Release();
+  }
+
+  // Sort the symbols by name/size so that we can print a report about duplicate
+  // variables.
+  std::sort(symbols.begin(), symbols.end(), NameCompare);
+  for (auto p = symbols.begin(); p != symbols.end(); /**/) {
+    auto pScan = p;
+    // Scan the data looking for symbols that have the same name
+    // and size.
+    while (pScan != symbols.end() && p->size == pScan->size &&
+           StringCompare(p->name, pScan->name) == 0)
+      ++pScan;
+
+    // Calculate how many times the symbol name/size appears in this PDB.
+    size_t repeat_count = pScan - p;
+    if (repeat_count > 1) {
+      // Change the count from how many instances of this variable there are to
+      // how many *excess* instances there are.
+      --repeat_count;
+      DWORD bytes_wasted = repeat_count * p->size;
+      if (bytes_wasted > kWastageThreshold) {
+        repeats.push_back(RepeatData(repeat_count, bytes_wasted, p->name));
+      }
+    }
+
+    p = pScan;
+  }
+
+  // Print a summary of duplicated variables, sorted to put the worst offenders
+  // first.
+  std::sort(repeats.begin(), repeats.end());
+  std::reverse(repeats.begin(), repeats.end());

[chengx, 2016/12/19 21:48:02]

Another way is to define a comparison function using the operator ("<")
overloading you defined for RepeatData, and use it in std::sort(). In this way,
std::reverse is no longer needed. 

[stanisc, 2016/12/19 22:11:40]

nit: You could avoid reversing by changing the comparison operator.

[brucedawson, 2016/12/19 23:00:51]

I had that originally for RepeatData and decided I didn't like having an
operator< that was, technically, backwards.

+  for (auto& repeat : repeats) {

[chengx, 2016/12/19 21:48:02]

Is "auto const &" better?  

[brucedawson, 2016/12/19 23:00:51]

Yes. const all the things.

+    // The empty field contain a zero so that Excel/sheets will more easily
+    // create the pivot tables that I want.
+    wprintf(L"%d\t%u\t0\t0\t%s\t%s\n", repeat.repeat_count, repeat.bytes_wasted,

[stanisc, 2016/12/19 22:11:40]

nit: I think this might not align with the header. It depends on whether symbol
names take between 8 and 16 characters.
"#Dups\tDupSize\tSize\tSection\tSymbol name\tPDB name\n"

[brucedawson, 2016/12/19 23:00:51]

Yeah, PDB name doesn't align. But it generally doesn't matter.

I did tweak the numeric alignment slightly.

+            repeat.name.c_str(), filename);
+  }
+  wprintf(L"\n");
+
+  // Print a summary of the largest global variables
+  std::sort(symbols.begin(), symbols.end(), SizeCompare);
+  std::reverse(symbols.begin(), symbols.end());

[chengx, 2016/12/19 21:48:02]

Same as line 155, reverse operation can be avoided. 

[stanisc, 2016/12/19 22:11:40]

The same as above.

+  for (auto p = symbols.begin(); p != symbols.end(); ++p) {

[stanisc, 2016/12/19 22:11:40]

Could use for (auto& p : symbols) cycle

[brucedawson, 2016/12/19 23:00:51]

Done.

+    if (p->size < kBigSizeThreshold)
+      break;
+    // The empty fields contain a zero so that the columns line up which can
+    // be important when pasting the data into a spreadsheet.
+    wprintf(L"0\t0\t%6d\t%d\t%s\t%s\n", p->size, p->section, p->name.c_str(),
+            filename);
+  }
+
+  return true;
+}
+
+bool Initialize(const wchar_t* filename,
+                IDiaDataSource** source,
+                IDiaSession** session,
+                IDiaSymbol** global) {
+  HRESULT hr = CoInitialize(NULL);
+  if (FAILED(hr)) {
+    wprintf(L"CoInitialize failed - %08X.", hr);
+    return false;
+  }
+
+  // Initialize DIA2
+  hr = CoCreateInstance(__uuidof(DiaSource), NULL, CLSCTX_INPROC_SERVER,
+                        __uuidof(IDiaDataSource), (void**)source);
+  if (FAILED(hr)) {
+    wprintf(L"Failed to initialized DIA2 - %08X.\n", hr);
+    return false;
+  }
+
+  // Open the PDB
+  hr = (*source)->loadDataFromPdb(filename);
+  if (FAILED(hr)) {
+    wprintf(L"LoadDataFromPdb failed - %08X.\n", hr);
+    return false;
+  }
+
+  hr = (*source)->openSession(session);
+  if (FAILED(hr)) {
+    wprintf(L"OpenSession failed - %08X.\n", hr);
+    return false;
+  }
+
+  // Retrieve a reference to the global scope
+  hr = (*session)->get_globalScope(global);
+  if (hr != S_OK) {
+    wprintf(L"Get_globalScope failed - %08X.\n", hr);
+    return false;
+  }
+
+  return true;
+}
+
+void Cleanup(IDiaSession* session, IDiaSymbol* global) {
+  if (global) {
+    global->Release();
+  }
+
+  if (session) {
+    session->Release();
+  }
+
+  CoUninitialize();
+}
+
+int wmain(int argc, wchar_t* argv[]) {
+  if (argc < 2) {
+    wprintf(L"Usage: ShowGlobals file.pdb");
+    return -1;
+  }
+
+  const wchar_t* filename = argv[1];
+
+  IDiaDataSource* source = nullptr;
+  IDiaSession* session = nullptr;
+  IDiaSymbol* global = nullptr;
+  if (!(Initialize(filename, &source, &session, &global)))
+    return -1;
+
+  DumpInterestingGlobals(global, filename);
+
+  Cleanup(session, global);

[stanisc, 2016/12/19 22:11:40]

What about releasing the source?

[brucedawson, 2016/12/19 23:00:51]

Sample code didn't, but I do now.

+}