Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected pointer wrapping when manually inserting LLVM modules #17

Closed
MarcelHB opened this issue Nov 20, 2018 · 1 comment
Closed

Comments

@MarcelHB
Copy link
Contributor

Hallo again,

There is something I noticed. I made my project a standalone Phasar tool, and I preprocess LLVM IR code by making it feed to llvm::parseIRFile first, then doing some transformations, and then passing it to psr::ProjectIRDB#insert_module. For the next run, I was greeted by a double free error.

The cause is located here: https://github.com/secure-software-engineering/phasar/blob/master/lib/DB/ProjectIRDB.cpp#L706

I originally created the llvm::LLVMContext as a stack variable in my main and used that for parsing. So I suggest to change the signature of that method to something like:

void ProjectIRDB::insertModule(
  std::unique_ptr<llvm::Module> M, 
  std::unique_ptr<llvm::LLVMContext> C
);

So that I can create and move the required context pointer explicitly. For now, I do:

// Phasar cleans this up.
auto context = new llvm::LLVMContext;

That's Ok, but secretly wrapping M->getContext() into a unique pointer is a sneaky move. 😉

@pdschubert
Copy link
Member

Thanks for your feedback. Yeah, you might be right. We have been running into problems when using the ProjectIRDB for more complex and exotic scenarios and therefore, are going to improve it. While doing so, we will certainly adjust the behavior and make the transfer in ownership explicit.

fabianbs96 added a commit that referenced this issue Nov 25, 2023
* Analysis Printer (#17)

* Initial Commit

* AnalysisPrinter Second commit

* Initial Commit

* Integrate Printer with client Analysis and test

* Addressing Review comments

* Integrate AnalysisPrinter with all analyses and template class modified

* vector emplace_back instead of push_back

* Testcase for AnalysisPrinter

* GroundTruth derived class initial commit

* AnalysisPrinter Test complete and Test

* fixing myphasartool file

* Test pre-commit fix

* Adding Test cases and fixing PR failure

* 1.template params to N,D,L  2.remove AnalysisType param from AnalysisResults 3.rearranging class variables

* 1.template params to N,D,L 2.remove AnalysisType param from AnalysisResults 3.rearranging class variables

* Null AnalysisPrinter singleton

* Adding AnalysisPrinter to IDETabulation Problem

* making free (N,D,L)ToString functions

* disable copy and move for analysis-printer

* Default NullAnalysisPrinter and explicit print methods

* removing SetAnalysisPrinter from client analyses and modified Testcase for AnalysisPrinter

* Adding superclass for AnalysisPrinter

* Addressing review comments and fixing PR build failure

* fix: minors

* fix: minor (clang-tidy)

* fix: review feedback

* misc: minor refactoring

---------

Co-authored-by: SanthoshMohan <[email protected]>
Co-authored-by: Sriteja Kummita <[email protected]>

* fix: review feedback

---------

Co-authored-by: SanthoshMohan <[email protected]>
Co-authored-by: Sriteja Kummita <[email protected]>
fabianbs96 added a commit that referenced this issue Feb 25, 2024
* Analysis Printer (#17)

* Initial Commit

* AnalysisPrinter Second commit

* Initial Commit

* Integrate Printer with client Analysis and test

* Addressing Review comments

* Integrate AnalysisPrinter with all analyses and template class modified

* vector emplace_back instead of push_back

* Testcase for AnalysisPrinter

* GroundTruth derived class initial commit

* AnalysisPrinter Test complete and Test

* fixing myphasartool file

* Test pre-commit fix

* Adding Test cases and fixing PR failure

* 1.template params to N,D,L  2.remove AnalysisType param from AnalysisResults 3.rearranging class variables

* 1.template params to N,D,L 2.remove AnalysisType param from AnalysisResults 3.rearranging class variables

* Null AnalysisPrinter singleton

* Adding AnalysisPrinter to IDETabulation Problem

* making free (N,D,L)ToString functions

* disable copy and move for analysis-printer

* Default NullAnalysisPrinter and explicit print methods

* removing SetAnalysisPrinter from client analyses and modified Testcase for AnalysisPrinter

* Adding superclass for AnalysisPrinter

* Addressing review comments and fixing PR build failure

* fix: minors

* fix: minor (clang-tidy)

* fix: review feedback

* misc: minor refactoring

---------

Co-authored-by: SanthoshMohan <[email protected]>
Co-authored-by: Sriteja Kummita <[email protected]>

* OnTheFlyReporting Initial Commit

* fix: review feedback

* onTheFlyAnalysis makeUniquePtr

* OnTheFlyReporting Initial Commit

* onTheFlyAnalysis makeUniquePtr

* addressing minor error in prev commit

* Refactoring Warn Variable and Refactoring lambda flow for AnalysisPrinter

* Integrating sourceMgr to AnalysisPrinter

* Testcase for OnTheFlyAnalysisPrinting

* Testcase for SourceMgrPrinter

* MaybeUniquePtr for SourceMgrPrinter

* Minor review comments

* refactoring the printers

* adding TODOs

* more TODOs

* Refactor AnalysisPrinter part1

* Refactor AnalysisPrinter part2

* dev: update AnalysisPrinterBase, integrate and fix tests

* adding warning kind

* fix ci

* update codeowners for analysis-printer

* Refactor SourceManagerPrinter

* Minor in OTF Printer Test

* fixing review feedback

* fix l_t printing

* Mandating analysisType to tyestate descriptions

---------

Co-authored-by: SanthoshMohan <[email protected]>
Co-authored-by: Fabian Schiebel <[email protected]>
MMory added a commit that referenced this issue Feb 29, 2024
* small backup safe

* backup save, still needs metadata extraction

* refactoring + some basic functions implemented

* basic structure of constructHierarchy()

* DIBasedTypeHierarchy structure

* basic impl of constructor and hasVFTable

* impl edges of graph, isSubType, getSubType and print

* untested version of transitive closure

* added transitive closure and changed print

* fixed transitive closure + refactoring

* bug fixes + tests

* debugging Debug info extraction

* Fixed type extraction, untested transitive hull

* fixed includes + more debug info

* bug fixes and non recursive transitive hull

* working direct edge detection

* BitVector, cleanup, start of vtable impl

* vtables and dotgraph

* review changes + vtable fix, 50% finished

* impl review suggestions

* removed old type_hierarchy unittests

* impl .set_bits() loop

* fixed vtables

* fixes and code cleanup

* added llvm::interleaveComma

* fixed wrong assertion

* public LLVMVFTable constructor

* small refactor

* important bugfixes

* unittests for multiple base classes

* unittests not finished, backup

* more unittests, all pass

* reworked unittests

* review changes

* review changes

* myphasartools.cpp revert

* current final version

* Bump submodules

* backup of fixes + unittests

* more unittests

* new unittest

* Pin swift version

* basicRecoTH backup

* backup of structure

* Analysis Printer (#17)

* Initial Commit

* AnalysisPrinter Second commit

* Initial Commit

* Integrate Printer with client Analysis and test

* Addressing Review comments

* Integrate AnalysisPrinter with all analyses and template class modified

* vector emplace_back instead of push_back

* Testcase for AnalysisPrinter

* GroundTruth derived class initial commit

* AnalysisPrinter Test complete and Test

* fixing myphasartool file

* Test pre-commit fix

* Adding Test cases and fixing PR failure

* 1.template params to N,D,L  2.remove AnalysisType param from AnalysisResults 3.rearranging class variables

* 1.template params to N,D,L 2.remove AnalysisType param from AnalysisResults 3.rearranging class variables

* Null AnalysisPrinter singleton

* Adding AnalysisPrinter to IDETabulation Problem

* making free (N,D,L)ToString functions

* disable copy and move for analysis-printer

* Default NullAnalysisPrinter and explicit print methods

* removing SetAnalysisPrinter from client analyses and modified Testcase for AnalysisPrinter

* Adding superclass for AnalysisPrinter

* Addressing review comments and fixing PR build failure

* fix: minors

* fix: minor (clang-tidy)

* fix: review feedback

* misc: minor refactoring

---------

Co-authored-by: SanthoshMohan <[email protected]>
Co-authored-by: Sriteja Kummita <[email protected]>

* new unittests, some fail

* unittests fixed, all pass

* Add LLVM-RTTI-style type-hierarchy layout

* fix: review feedback

* Fix logging macro invocation

* Split giant DIBasedTypeHierarchy ctor into multiple functions

---------

Co-authored-by: mxHuber <[email protected]>
Co-authored-by: SanthoshMohan <[email protected]>
Co-authored-by: Sriteja Kummita <[email protected]>
Co-authored-by: Martin Mory <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants