Get LLVMContext instance directly from llvm::Module when building SVFModule

[Johanmyst]

The recent commit merged in #1258 modified the interface to build an SVF::SVFModule instance from an llvm::Module object to include a std::unique_ptr<llvm::LLVMContext> parameter to specify the context used when loading the ExtAPI module. This change introduces a number of issues however, particularly when using SVF from an LLVM pass.

Most critically, the use of std::unique_ptr<> creates problems when SVF is used from an LLVM pass, as automatic garbage collection on the context is done incorrectly. Creating a unique pointer based on the context of the LLVM module incorrectly means ownership of the context is taken by the LLVMModuleSet object. This causes issues because the context is obviously not uniquely owned by the module set; when an SVFModule is built from an LLVM module, calling LLVMModuleSet::releaseLLVMModuleSet() triggers an invalid free/double free and crashes. Wrapping the unconditional deletion on line 110 also does not solve this issue, as the static pointer remains set after the underlying object is garbage collected.

This issue occurs using LLVM 15 (debug build with RTTI & exception handling enabled) and running SVF on the module from a module pass in a full LTO pipeline. I did not check if the issue also exists when running a pass separately through opt, but it could be possible this issue does not exist in that case (as I saw above issues reported by Clang for the test programs it compiles to check compiler functionality before compiling the actual target).

Additionally, the change to explicitly passing an LLVMContext breaks backwards compatibility unnecessarily. Since the context used when loading ExtAPI should match the module used to build the SVFModule from, it is unnecessary to require the context be passed through a separate argument when it can be gotten directly from the module.

[yuleisui]

Thanks for fixing it. Could you make sure your pr only contains the changes made on LLVMModule.h/cpp?

[Johanmyst]

Thanks for fixing it. Could you make sure your pr only contains the changes made on LLVMModule.h/cpp?

Done!

[yuleisui]

Thanks for fixing it. Could you make sure your pr only contains the changes made on LLVMModule.h/cpp?

Done!

This pull request looks to me is a mix of previous commits which were not supposed to be here..

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (e09c431) 64.46% compared to head (f1d02fa) 64.47%.
Report is 8 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1292   +/-   ##
=======================================
  Coverage   64.46%   64.47%           
=======================================
  Files         223      223           
  Lines       23817    23832   +15     
=======================================
+ Hits        15353    15365   +12     
- Misses       8464     8467    +3     

Files	Coverage Δ
svf-llvm/include/SVF-LLVM/LLVMModule.h	97.89% <ø> (ø)
svf-llvm/lib/LLVMModule.cpp	82.63% <70.00%> (-0.14%)	⬇️

... and 1 file with indirect coverage changes

[Johanmyst]

This pull request looks to me is a mix of previous commits which were not supposed to be here..

Ah, my bad. I didn't realise selecting "pick" during an interactive rebase for commits on the master branch would include them as part of this branch like that. Should be fixed now. :)