If multiple threads executes addOption concurrently, there is a race condition between find and insert operation to SC->OptionsMap.
It is fixed by adding a mutex.
wenju on Jul 5 2021, 11:57 PM.Authored by
Thanks for the fix! This LGTM as far as the change goes, though I wonder if there are other race conditions hiding in this file. I'm not an expert in this particular area, so you should wait a few days before landing in case any of the other reviewers have comments (I added a few more folks who have touched this file in recent history).
I think the problem is larger than this fix implies. I don't think this spot fix is the right approach. In the long run, it will make it harder to implement a comprehensive design for thread safety. Today, one cannot safely concurrently construct cl::opts, because they modify the global options registry.
Even if synchronization is added at this point, aren't there still logical races between the thread registering new command line options and another thread parsing a command line?
To address your issue, I recommend that you register all cl::opts prior to launching threads, or add synchronization between the threads while registering options. Alternatively, I'd like to see more comprehensive synchronization.
That's my sentiment as well, I'm wary of just patching over without stepping back and looking at this a bit more globally.
Adding/deleting options is typically done in a separate initialization stage, and just once.
https://lists.llvm.org/pipermail/llvm-dev/2018-October/127039.html ([RFC] Enable thread specific cl::opt values for multi-threaded support) is a related previous thread.
cl::opt (and its friends like cl::list) does have some problems, but we need to figure out the needs and look at the big picture.
The issue is that DefaultOptions is special and delay-processed in CommandLineParser::ParseCommandLineOptions. Suppose the main thread calls llvm:🆑:ParseCommandLineOptions, then DefaultOptions is processed but it is not cleared after processing.
I find this issue is fixed (workaround) by https://reviews.llvm.org/D99740
If we can assume that ParseCommandLineOptions will never be called from setCommandLineOpts (CodeGen/BackendUtil.cpp) simultaneously by multiple threads, then we don't need to fix DefaultOptions.