<table border="1" cellspacing="0" cellpadding="8">
    <tr>
        <th>Issue</th>
        <td>
            <a href=https://github.com/llvm/llvm-project/issues/109482>109482</a>
        </td>
    </tr>

    <tr>
        <th>Summary</th>
        <td>
            getProcessorGroups in Windows/Threading.inc does not appear to be reentrant/thread-safe
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            new issue
      </td>
    </tr>

    <tr>
      <th>Assignees</th>
      <td>
      </td>
    </tr>

    <tr>
      <th>Reporter</th>
      <td>
          tex3d
      </td>
    </tr>
</table>

<pre>
    In file `llvm/lib/Support/Windows/Threading.inc` in function `getProcessorGroups()`:

https://github.com/llvm/llvm-project/blob/2011cbcd84102236dd6d58e2079ac676a3403f25/llvm/lib/Support/Windows/Threading.inc#L243-L244

This function does not appear to be reentrant/thread-safe, due to the assignment to the static and return of an `ArrayRef` pointing at that.  If two threads were to call this function simultaneously, it appears possible that one could assign the static and return an `ArrayRef` pointing to that, while another comes in and overwrites the static, invalidating the memory pointed to by the `ArrayRef` used by the first thread while the caller could still be reading it.

I came across this when looking for why `ClandTests` would occasionally crash in a random clangd test on our automated Windows test runs.  At this time, I have no idea whether this issue is involved in that crash, or even if these tests are being run concurrently on the same process.

In case someone is curious how the tests could hit this if multi-threaded, here's the breakdown:

- Many Clangd tests will call `ClangdServer::optsForTest()`, which creates a `ClandServer::Options`, then overwrites several of the option values before returning.
- `ClandServer::Options` default constructor will call `clangd::getDefaultAsyncThreadsCount`.  See: https://github.com/llvm/llvm-project/blob/2011cbcd84102236dd6d58e2079ac676a3403f25/clang-tools-extra/clangd/ClangdServer.h#L105
- That will call `ThreadPoolStrategy::compute_thread_count()`, which calls `get_physical_cores()`.  See: https://github.com/llvm/llvm-project/blob/2672947633865c19c332c8e39f0d11e841e2480f/clang-tools-extra/clangd/TUScheduler.cpp#L1611 https://github.com/llvm/llvm-project/blob/2672947633865c19c332c8e39f0d11e841e2480f/llvm/lib/Support/Threading.cpp#L57
- In `llvm/lib/Support/Windows/Threading.inc`, `get_physical_cores()` calls `getProcessorGroups()` and uses the resulting groups.  See: https://github.com/llvm/llvm-project/blob/2672947633865c19c332c8e39f0d11e841e2480f/llvm/lib/Support/Windows/Threading.inc#L255-L261

Note that `get_physical_cores()` also initializes a static local, but it's just an `unsigned`, so that's unlikely to cause a real problem.  Though it would make sense to guard this static initialization as well, to avoid recomputing that value.
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJy8VlGP27gR_jXyy8CGRMmy_eCHbQIXC6TtobtFH4MRObJ4oUiBQ9rn_vqClHzrXLu5XlAEAYKFTM58830fZwaZ9dkSHYvtn4rtxxXGMDh_DPRLrVadU7fjs4VeG4KiLY25jIU4Gd0V4vQSp8n5UIjTP7VV7sqFOL0OnlBpe95oK4u2BG2hj1YG7WwKcKbwk3eSmJ3_s3dx4kLsC3Eo2rKon4ryY1Eu_w8hTJy-iVMhTmcdhthtpMvpFxTmMq4n734mmUB0xiVUoqwq2Um1b6pSiLpVqlXbPYlyd0DZ7lqsm7LuxfYhzv9Yjag_iaZefxJN8wj0ddD8VqNyxGBdAJwmQg_BQUfgiWzwaFP4kIOuGXsqxAdQkdKhMBDMUoxkw_0LBwxaAloFnkL0FlwPmJl88h5vf6c-kTw5bYO2Z8AAYcCwAXjuIVxTkJSM4Uo-p5FoDISvELMeowloyUU2twRJ39EzTI5Zd4ZyWHCWQLpo1AL1HYzfAJjrwpCyXIdkKrQuDORBupE4uSXFcRfyV68D8UOGjMxe0GiFc6yBYKTR-dscn1Rm-5Z_-A2AyKTuP_Xac1iYWVCkz4maDCTVx0EbMyuXDQA6bB5FfwaJIwFK75hnQq8DWTDOfUnHe-fhOtwSjA8GrXolDpyAXHN4JyWydhaNuYH0yEMuHTxa5UaQBu1ZQSBOnIOLHjAGN2KqcbHn_KuPljcAT2HGEPSYTfUMA14IrAOtCBO0THI-o5kjQfrDXpy5kEqps7wZSLruPNCFLOg-McOUczGgJ-goleejBemsjN6TDeaWUGalEinT_MC_5suCRCZgN1JykWaQ0WsXGQZ3zXfnHDP9g14K0j0kd-r1LBepBG8gT4XYzeboPOEX5a72N_1jDX9Be4MPb1QyXJOo-QksupzVC_kL-XS3fnJT4JPzSau3rjQ7VQ4gPWFyJP4q6uPdv03pMfFyIyQvPLiY6UIeTXq9CbPLh-GCJhJDR73ztDye1GzuBXw7DyjqMZqQhODgowzJc48Vzjaa750pfJzPP_HNyrmz8QcXbSjacgPwQlTUT_AD2m6GtQ7OGV7TL8Hj_ZsqxOlRlc2Qem5Vbu-EvCaXflXiXMdPzpmX4DHQ-TaXK904xUCfZ9t8lrnO_6IpGsPLXPo8DTfWEs1n6Ty9zaX_AzftThyaXVvX-3Yrq4OsayH3VB_6UlUV7ZuKRLMv-9_j5vUfL3IgFQ35jZymTE5bVT8C1zuj8m1ALni2u7tWz_a7Foakzbf1-Eq095aJPEYiLwPEE6cmYs9wzsd-qKTfsWVst-tPoq0e29lfXViG8O-wg4YdaKuDRqP_lfvVMqGNk2gSv10MoEPuoD9HDsu8jjYvgmoRge-TescQrdFfyNzmDSIypVFFaFKr7wyNG4DXwcXzkFaHecKN-IWAyXJeO84RvZo7-gLmV4SYWyGmFcVkdMEBXpxO28T8iudZj2Hul5uVOtbqUB9wRcdqJ9p9-rdbDcdqj-Wh3m5R7hsSXXeo99XusJWtwm3fbvuVPopSNOVBlKJqmma36fqWOtHU27brukYciqakEbXZJM02zp9XeVgeq_LQ7MXKYEeG86IshKXrPEoLIdLe7I_ZKV08c9GURnPgtzBBB0PH__RrGr3vGOGPbZKr6M3xD5s540-ZlwIvR_HvAAAA___3kwHN">