Cache negative symbol lookups

[leculver]

Fixes issue #675, but also just improves the amount of symbol lookups we do.

[Copilot]

Pull request overview

This PR addresses issue #675 by implementing negative caching for failed symbol lookups in SOS, preventing repeated network round-trips to symbol servers when symbols are unavailable. The implementation adds a global cache that tracks module PE addresses for which symbol loading has failed, and clears the cache when the symbol path changes.

Changes:

• Implemented negative caching using std::set<ULONG64> to track failed symbol lookups by module PE address
• Added cache invalidation when symbol path changes (in SetSymbolServer and InitializeSymbolStoreFromSymPath)
• Added unit tests for OpenSymbolFile to verify graceful handling of invalid data

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/SOS/Strike/symbols.h	Added declaration for ClearSymbolLookupCache() function
src/SOS/Strike/symbols.cpp	Implemented negative cache with g_failedSymbolLookups set, cache check logic, and partial cache population for failed portable PDB lookups
src/SOS/Strike/managedcommands.cpp	Added cache clearing call in SetSymbolServer command
src/SOS/Strike/dbgengservices.cpp	Added cache clearing call in InitializeSymbolStoreFromSymPath
src/tests/Microsoft.Diagnostics.DebugServices.UnitTests/SymbolServiceTests.cs	Added tests for OpenSymbolFile handling of invalid PE and PDB streams

Comments suppressed due to low confidence (1)

src/SOS/Strike/symbols.cpp:342

• The failure of LoadSymbolsForWindowsPDB is not cached. When this Windows PDB lookup fails and the code continues to try LoadSymbolsForPortablePDB, the module address should be added to g_failedSymbolLookups to prevent repeated attempts. Otherwise, Windows PDB lookup failures will still repeatedly hit symbol servers.

Consider adding the module address to the cache when LoadSymbolsForWindowsPDB fails, similar to how it's done for LoadSymbolsForPortablePDB failures at line 379.

            hr = LoadSymbolsForWindowsPDB(pMD, moduleBase, pModuleName, FALSE);
            if (SUCCEEDED(hr))
            {
                return hr;
            }
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[leculver]

@max-charlamb Thanks! Can you re-approve? I had to rebase on main as it got out of date.

[max-charlamb]

@max-charlamb Thanks! Can you re-approve? I had to rebase on main as it got out of date.

Yep. Thanks for this change. Should be a very positive improvement.