Skip to content
  • Brad King's avatar
    PCH: Fix logic error that incorrectly clears sources during VS generation · fa7b041e
    Brad King authored
    Since commit 729d997f (Precompile Headers: Add REUSE_FROM signature,
    2019-08-30, v3.16.0-rc1~101^2), `GetPchFileObject` handles the case that
    it is called first for another target's `REUSE_FROM` by calling
    `AddSource` to make sure `GetObjectName` can produce the correct object
    name.  However, `AddSource` causes `ClearSourcesCache` to be called,
    which since commit a9f4f58f (cmGeneratorTarget: Clear AllConfigSources
    in ClearSourcesCache, 2020-05-15, v3.16.7~2^2) now correctly erases the
    `AllConfigSources` structure.  This is okay during `AddPchDependencies`,
    but there is another code path in which it is problematic.
    
    When the Visual Studio generator's `WriteAllSources` method is looping
    over the sources, the `cmake_pch.cxx` source is encountered first.  This
    causes `OutputSourceSpecificFlags` to call `GetPchCreateCompileOptions`,
    which calls `GetPchFile`, which under MSVC with `CMAKE_LINK_PCH` calls
    `GetPchFileObject`.  That leads to `ClearSourcesCache` erasing the
    structure over which `WriteAllSources` is iterating!
    
    This bug is caught by our `RunCMake.PrecompileHeaders` test when run
    with the VS generator as of the commit that exposed it by fixing
    `ClearSourcesCache`.  However, that change was backported to the CMake
    3.16 series after testing only with later versions versions that contain
    commit a55df204 (Multi-Ninja: Add precompile headers support,
    2020-01-10, v3.17.0-rc1~136^2).  By adding proper multi-config support
    for PCH, that commit taught `cmLocalGenerator::AddPchDependencies` to
    call `GetPchFile` with the real set of configurations instead of just
    the empty string.  This allows the `GetPchFile` cache of PCH sources to
    be populated up front so that the later calls to it in the
    `WriteAllSources` loop as described above do not actually call
    `GetPchFileObject` or `ClearSourcesCache`.  That hid the problem.
    
    Fix this by re-ordering calls to `AddPchDependencies` to handle
    `REUSE_FROM` targets only after the targets whose PCH they re-use.
    Remove the now-unnecessary call to `AddSource` from `GetPchFileObject`
    so that `ClearSourcesCache` is never called during `WriteAllSources`.
    Update the PchReuseFrom test case to cover an ordering of targets that
    causes generators to encounter a `REUSE_FROM` target before the target
    whose PCH it re-uses.
    
    Fixes: #20770
    fa7b041e