[clang] [clang][ThreadSafety] Check trylock function success and return types (PR #95290)

Dan McArdle via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 21 13:33:08 PDT 2024


dmcardle wrote:

So... I appear to have accidentally fixed a bug. Looks like enum success values were not being evaluated, and defaulted to false. As a result, the analysis could fail to detect unguarded access [[demo on godbolt.org](https://godbolt.org/#z:OYLghAFBqd5QCxAYwPYBMCmBRdBLAF1QCcAaPECAMzwBtMA7AQwFtMQByARg9KtQYEAysib0QXACx8BBAKoBnTAAUAHpwAMvAFYTStJg1DIApACYAQuYukl9ZATwDKjdAGFUtAK4sGIAMwA7KSuADJ4DJgAcj4ARpjEIABsAKykAA6oCoRODB7evgHBmdmOAuGRMSzxiam2mPZlDEIETMQE%2BT5%2BQfWNuS1tBBXRcQnJaQqt7Z2FPZODw1U14wCUtqhexMjsHAD0uwDUALJMyMRZB0QHjEyx9JeYkxHAB6hUB7GoBAgHAG5teEMBAUr3e30wlwQxEwTHQBwUTComAIAE8DoYxCjsgoAHQmDQAQXxRLM/jw7zkQmwAH03ATlASLABJUJMgAqAE1ieZ/FgaJEDqEAPJuADSjNC2AO0plsrl8pl1OpTAIBGIeFiXgImCVUFE6VudEIKIg5jMLC1mFUZpWtu5pL5EQhBKEVIASmzqdgABpuUKUpkANRpwrFEBxEZW8qVKrVGstuogTAUSna1P1htoxogSsDBOpBLdAHEhErbSt7bzMPzna7sB7qUIABKF7AAEWpodF4cjCulMdV6s12sTydTBGpCgQbUw6HTTANsSNqJz1LzBeLpep5crjoFPr9AeDnZFoupADE5FE3GymUKoj2cVHFcrB/GR9SoKcAI5ePDQ%2BdF2XE1c3zQsSzLO1CR5PcIWbVsOy7C8rxvO8HwjJ8FQHONhx1T8k2QX9/x1KcZznDMlyzFdQI3CDtygkkqxrA4ryQy9r1ve9H2fPt%2B1fHCE3w6F6GTHVgEYBI8GQQDM2zGjwK3HdoIdasnQOA9/SEIMaTZN0OTYlDOPQ3tsKHQSoDVFFlUIv8AIo4DV3XBTIIrZSmLU%2BC3XbaldP009kI4tDuNlUz3zwiziCsn9bJI6doXIhdZOotcwM3Fzd1U/dfU07STzFUsvIARTkJkvLbYK%2BNjMyPygaEiOhBQZMouSUtoxSGJgzK4JbMq8tFArsGK0r2wqkL%2BOq8KIDqmLGtI%2BKmoc%2BS0vo1zGIaJQMuYrsJSlXi9rGqqwsTWhUGQABrW56A6lTmJdd1PQ0o8Q1PUaX0O3DRxTBIJytZBvGyX4dRO87HNSuilMY2CDju%2BtPU87yu1euVQo%2B/Cx2%2Byc4tnalgbO0G2vStyocerTjwMwKuIwniDhR8yIF%2B/68EBnHTrO6kqC8BgHFyfHnJWzaPJ6hH/PY1DKd7aNxqO/C5ux3H2c57mBF55aIc65jWJFwygqp/aaal1GoE5%2BWOa5poVfB673Kyw9SZ0vTybF4zMNpmr6dUP6vABnVLJNxXzaWy3VvVwWEJ8h2tYp53qddybZbnX3WYVs2ecD9rg5utSSdyrsBqGsrRtjxMGa9pmgdZxrpuI9ALfTgWBXhxDTzzkqC915GDbp%2BOWfOyvMHq2da8JtaGHwKhuUJfZBVZ%2BFMBYIEpJBARaBReuISLORCzbby8zdPXeKL/DgC8NosDnf5iCtqHlE9Det53wt947966fSCdj9P7GL46yfDjcRLmqOEeK8BgK817qWyk9akmsxQBSdhVQ%2BtUGgwiUAtKiIFWp8zVpnBuQsOwwLPKLIySNEFTWQaJTGZE0EtScqrK%2BXUDh3zdNvDsFgOQQFUNTJ%2B%2BsX5uw/sQM%2B1JYgmk4fQ5iN9qRMJYdSNhHCuH7VIW/ak/DBHCLkT/AkU8PAsBYAIcBQg3BCmUMLMUO1uGVTfIbCACg0DpDlqzS6mAxFqQJG4fO3kCTnjZPWJGfZSHRWrsqKg2piBD35kTBhrj3GsOwOeIUXlfGS14ZNAJ814j8GhGE7B1sIRIS8myOQboogjU4eY0h8toQEE2JEGuoiM45MFM3L0kCWGJKSZYumuNGolzPlk5xAoohCh8k2LyBIOxCE8dgTkBYogElCByLSQgsKdzdgwVA1JvjQlhJOREyIorMBXtiDRU83SYGAA1bIAgHiTAOBky4kVcbog6dqEEAB3QgPxDDXAYD4BIKoSDwi8MgbYKZ0TEGPmwQQeJf4HGUOqAFgNiCXIYEvMECAIQYkOXgEE2R6CCBXrcpgdBZyXFQNcf43gVQQgUECkFII2gQsYMCYkU9vgqgOK8hIEJIgfISAcEwSQNBuG9N6CwqBPDhBCWIbAqh0jEAFRoA4aziD8sFUyQQZyEiSr%2BbQBV6JR7ohTL8uE4IWWHApV4CErzkyEtoEoaFBJmBsAUAabYBw2QPNZkIWljwFDYB%2BSwc8YglBRDOSqMu/LAhWGgoSRgPh3WevOqcmltACCRosAcINdBNgQhMP4NsBwNCkAON64Fvr%2BX5oOFwSNbY83RqJISP6Y5GmmIsJKY4lpVDpuJOkLwdwpIgGJNKD1KJcbJq8KmhNKJQiswgFGbOx5fKO2IaWulFZ/D1ulL8VAeA4RyFAbO%2BdkC7bQKiMuoK6760mECLWjdE8CRNtBSO3GbJHhpuvZujt2ou0WmpHWodBwIhpqYNSRhm9mHeVkb%2BqMeaC0aH/TGgkW6d0mrfUI8VtBqRWjlb6nmMGo0AelOSA4EAwBgF/TiEdM6Qa2nTdKKeUQvgQnBIBhgiLtRwliMQQwyAfhoFHjkAQDr5QgesGJjd5ip5sgQNi%2BECANi0E4xCKgBhgDiU42iFj0mtlwiELs1E0MDlYmxWRwjMoKP7txnOhDSGZTXtrYSaU9nuQ3ps/Zg4U8nWPFdRCZ9XqfUpn9T4INdrohhscIDDgaxaCcBSLwPw3BeCoE4G4MTlh4QbC2Lm0kPBSAEE0FFtYZ0Aj%2BBxP4crFXKuVaSPoTgkh4sFdIMljgvAFAgCLfljgWg1hwFgEgNALB0jErIBQCAA2hv0ESE2owGziCczOnwOgIS2sQFiI1pczBIqcFyxttoKIhSxG0JgBw23eADchQQIUoCUSNawJqYA/9aB2tO6QLA88jDiC67wfA0JuaAza19kIqhjuWhe0BhojWsxcb2x4LAjW4wsBe4iz4Sg2xz0MMALMRgCtrBU0wYAChAx4EwK8oUdiEu5f4IIEQYh2BSBkIIRQKh1CA90FwfQGOUBpZsFDtrkA1ioDfrkAHABaSY6BYOmEsNYLg1aRcAHUEjnHlfmzZMJ0Bi/02iEXbZKQ0jpAyZkrJOSwdy6gRF6osB87nb0Y7TQXCjxmH4dnYQnRLDGOzkogm8ieC6HoL3TRFijESOzuwdv%2BhTA6L7woofkFK2aJHoP1QPe2Ej07vQ8x2hJ%2BWFwNYChMvbAkNF2LDXAfNYOKoAAHEkEXSRJAHGmy8CAap5tRggLgQgAKeS594J17rax0WwjGDb4r5WytVYn%2BVmrMWOD1dIAlrQTXOCtfa3lnHpBeuIBABsAgvaCDkEoON4bURWA7CrzXuvDeDBGBIy3hgZ0Vi8FnJ3y3egqfCFEOIen7%2BmdqEa2z0gV5bjdIU7YvDgOLefRrZrIULUPfUECvavWvevRvW/Obe/NvbRCbPlbvR/NfL7W0UgUfUrSfSfWrWfUvRfZrFfDrHHMAswCgpLZfPA/vUgRFZFEASQIAA%3D%3D%3D)]. I think that the old `isIntOrBool()` function accidentally let enumerator values through, but the analysis assumed it would only ever see `CXXBoolLiteralExpr ` or `IntegerLiteral `. I'm adding a regression test that closely resembles the demo, but verifies that the analysis actually detects the unguarded access.

I also realized I was supposed to update the release notes.

New patch with these changes coming momentarily.

https://github.com/llvm/llvm-project/pull/95290


More information about the cfe-commits mailing list