Restyle remove --backend-aggressive-opts and all the related code #8
Conversation
![restyled-io[bot]](https://avatars.githubusercontent.com/in/5851?s=60&v=4){kind=small}
|
Unable to locate .performanceTestingBot config file |
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
|
Thanks @restyled-io[bot] for opening this PR! For COLLABORATOR only :
|
![gitginie[bot]](https://avatars.githubusercontent.com/in/712912?s=60&v=4){kind=small}
There was a problem hiding this comment.
@restyled-io[bot]
Thank you for your contribution to this repository! We appreciate your effort in opening pull request.
Happy coding!
|
Processing PR updates... |
|
PR Details of @restyled-io[bot] in avast-retdec :
|
|
Important Review skippedAuto reviews are limited to specific labels. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
labels-and-badges
bot
added
NO JIRA
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| /// @{ | ||
| const std::string &getOrdinalNumbersDirectory() const; | ||
| const std::string &getInputFile() const; | ||
| const std::string &getInputPdbFile() const; | ||
| const std::string &getOutputFile() const; | ||
| const std::string &getOutputBitcodeFile() const; | ||
| const std::string &getOutputAsmFile() const; | ||
| const std::string &getOutputLlvmirFile() const; | ||
| const std::string &getOutputConfigFile() const; | ||
| const std::string &getOutputUnpackedFile() const; | ||
| const std::string &getOutputFormat() const; | ||
| const std::string &getLogFile() const; | ||
| const std::string &getErrFile() const; | ||
| uint64_t getMaxMemoryLimit() const; | ||
| uint64_t getTimeout() const; | ||
| retdec::common::Address getEntryPoint() const; | ||
| retdec::common::Address getMainAddress() const; | ||
| retdec::common::Address getSectionVMA() const; | ||
| const std::string &getBackendDisabledOpts() const; | ||
| const std::string &getBackendEnabledOpts() const; | ||
| const std::string &getBackendCallInfoObtainer() const; | ||
| const std::string &getBackendVarRenamer() const; | ||
| /// @} | ||
|
|
||
| void fixRelativePaths(const std::string &configPath); | ||
|
|
||
| template <typename Writer> void serialize(Writer &writer) const; | ||
| void deserialize(const rapidjson::Value &val); | ||
|
|
||
| public: | ||
| std::set<std::string> userStaticSignaturePaths; | ||
| std::set<std::string> staticSignaturePaths; | ||
| std::set<std::string> libraryTypeInfoPaths; | ||
| std::set<std::string> cryptoPatternPaths; | ||
| std::set<std::string> abiPaths; | ||
|
|
||
| /// Functions' names which were selected by the user through | ||
| /// selective decompilation. | ||
| std::set<std::string> selectedFunctions; | ||
|
|
||
| /// Selected functions' names from @c selectedFunctions which | ||
| /// were not found in the binary. | ||
| std::set<std::string> selectedNotFoundFunctions; | ||
|
|
||
| /// Address ranges selected by the user through selective decompilation. | ||
| common::AddressRangeContainer selectedRanges; | ||
|
|
||
| /// LLVM passes. | ||
| std::vector<std::string> llvmPasses; | ||
|
|
||
| private: | ||
| /// Decompilation will verbosely inform about the | ||
| /// decompilation process. | ||
| bool _verboseOutput = true; | ||
|
|
||
| /// Keep all functions in the decompiler's output. | ||
| /// Otherwise, only functions reachable from main are kept. | ||
| bool _keepAllFunctions = false; | ||
|
|
||
| /// Decode only parts selected through selective decompilation. | ||
| /// Otherwise, entire binary is decoded. | ||
| /// This speeds up decompilation, but usually produces lower-quality | ||
| /// results. | ||
| bool _selectedDecodeOnly = false; | ||
|
|
||
| std::string _ordinalNumbersDirectory; | ||
| std::string _inputFile; | ||
| std::string _inputPdbFile; | ||
| std::string _outputFile; | ||
| std::string _outputBitcodeFile; | ||
| std::string _outputAsmFile; | ||
| std::string _outputLlFile; | ||
| std::string _outputConfigFile; | ||
| std::string _outputUnpackedFile; | ||
| std::string _outputFormat; | ||
| std::string _logFile; | ||
| std::string _errFile; | ||
| uint64_t _maxMemoryLimit = 0; | ||
| bool _maxMemoryLimitHalfRam = true; | ||
| uint64_t _timeout = 0; | ||
|
|
||
| bool _detectStaticCode = true; | ||
| std::string _backendDisabledOpts; | ||
| std::string _backendEnabledOpts; | ||
| std::string _backendCallInfoObtainer = "optim"; | ||
| std::string _backendVarRenamer = "readable"; | ||
| bool _backendNoOpts = false; | ||
| bool _backendEmitCfg = false; | ||
| bool _backendEmitCg = false; | ||
| bool _backendKeepAllBrackets = false; | ||
| bool _backendKeepLibraryFuncs = false; | ||
| bool _backendNoTimeVaryingInfo = false; | ||
| bool _backendNoVarRenaming = false; | ||
| bool _backendNoCompoundOperators = false; | ||
| bool _backendNoSymbolicNames = false; | ||
|
|
||
| retdec::common::Address _entryPoint; | ||
| retdec::common::Address _mainAddress; | ||
| retdec::common::Address _sectionVMA; | ||
| }; |
There was a problem hiding this comment.
The Parameters class has a large number of member variables and methods, which can make it difficult to maintain and understand. This class could benefit from being broken down into smaller, more focused classes or structs. For example, you could create separate classes for handling backend options, file paths, and memory settings. This would improve modularity and make the codebase easier to manage.
| /// Decompilation will verbosely inform about the | ||
| /// decompilation process. | ||
| bool _verboseOutput = true; | ||
|
|
||
| /// Keep all functions in the decompiler's output. | ||
| /// Otherwise, only functions reachable from main are kept. | ||
| bool _keepAllFunctions = false; | ||
|
|
||
| /// Decode only parts selected through selective decompilation. | ||
| /// Otherwise, entire binary is decoded. | ||
| /// This speeds up decompilation, but usually produces lower-quality | ||
| /// results. | ||
| bool _selectedDecodeOnly = false; | ||
|
|
||
| std::string _ordinalNumbersDirectory; | ||
| std::string _inputFile; | ||
| std::string _inputPdbFile; | ||
| std::string _outputFile; | ||
| std::string _outputBitcodeFile; | ||
| std::string _outputAsmFile; | ||
| std::string _outputLlFile; | ||
| std::string _outputConfigFile; | ||
| std::string _outputUnpackedFile; | ||
| std::string _outputFormat; | ||
| std::string _logFile; | ||
| std::string _errFile; | ||
| uint64_t _maxMemoryLimit = 0; | ||
| bool _maxMemoryLimitHalfRam = true; | ||
| uint64_t _timeout = 0; | ||
|
|
||
| bool _detectStaticCode = true; | ||
| std::string _backendDisabledOpts; | ||
| std::string _backendEnabledOpts; | ||
| std::string _backendCallInfoObtainer = "optim"; | ||
| std::string _backendVarRenamer = "readable"; | ||
| bool _backendNoOpts = false; | ||
| bool _backendEmitCfg = false; | ||
| bool _backendEmitCg = false; | ||
| bool _backendKeepAllBrackets = false; | ||
| bool _backendKeepLibraryFuncs = false; | ||
| bool _backendNoTimeVaryingInfo = false; | ||
| bool _backendNoVarRenaming = false; | ||
| bool _backendNoCompoundOperators = false; | ||
| bool _backendNoSymbolicNames = false; |
There was a problem hiding this comment.
The class has many boolean flags (e.g., _verboseOutput, _keepAllFunctions, _selectedDecodeOnly, etc.). Using an enum or a bitfield to manage these flags could improve performance and reduce memory usage. This would also make it easier to add or remove flags in the future.
|
|
||
| llvmir2hll::StringSet parseListOfOpts( | ||
| const std::string &opts) const; | ||
| llvmir2hll::StringVector getIdsOfPatternFindersToBeRun() const; | ||
| llvmir2hll::PatternFinderRunner::PatternFinders instantiatePatternFinders( | ||
| const llvmir2hll::StringVector &pfsIds); | ||
| ShPtr<llvmir2hll::PatternFinderRunner> instantiatePatternFinderRunner() const; | ||
| bool initialize(llvm::Module &m); | ||
| void createSemantics(); | ||
| void createSemanticsFromParameter(); | ||
| void createSemanticsFromLLVMIR(); | ||
| bool loadConfig(); | ||
| void saveConfig(); | ||
| bool convertLLVMIRToBIR(); | ||
| void removeLibraryFuncs(); | ||
| void removeCodeUnreachableInCFG(); | ||
| void fixSignedUnsignedTypes(); | ||
| void convertLLVMIntrinsicFunctions(); | ||
| void obtainDebugInfo(); | ||
| void initAliasAnalysis(); | ||
| void runOptimizations(); | ||
| void renameVariables(); | ||
| void convertConstantsToSymbolicNames(); | ||
| void validateResultingModule(); | ||
| void findPatterns(); | ||
| void emitCFGs(); | ||
| void emitCG(); | ||
| void emitTargetHLLCode(); | ||
| void finalize(); | ||
| void cleanup(); | ||
|
|
||
| llvmir2hll::StringSet parseListOfOpts(const std::string &opts) const; | ||
| llvmir2hll::StringVector getIdsOfPatternFindersToBeRun() const; | ||
| llvmir2hll::PatternFinderRunner::PatternFinders | ||
| instantiatePatternFinders(const llvmir2hll::StringVector &pfsIds); | ||
| ShPtr<llvmir2hll::PatternFinderRunner> instantiatePatternFinderRunner() const; | ||
|
|
||
| private: | ||
| /// The input LLVM module. | ||
| llvm::Module *llvmModule = nullptr; | ||
| /// The input LLVM module. | ||
| llvm::Module *llvmModule = nullptr; | ||
|
|
||
| /// The resulting module in BIR. | ||
| ShPtr<llvmir2hll::Module> resModule; | ||
| /// The resulting module in BIR. | ||
| ShPtr<llvmir2hll::Module> resModule; | ||
|
|
||
| /// The used semantics. | ||
| ShPtr<llvmir2hll::Semantics> semantics; | ||
| /// The used semantics. | ||
| ShPtr<llvmir2hll::Semantics> semantics; | ||
|
|
||
| /// The used config. | ||
| config::Config* globalConfig = nullptr; | ||
| ShPtr<llvmir2hll::Config> config; | ||
| /// The used config. | ||
| config::Config *globalConfig = nullptr; | ||
| ShPtr<llvmir2hll::Config> config; | ||
|
|
||
| /// The used HLL writer. | ||
| ShPtr<llvmir2hll::HLLWriter> hllWriter; | ||
| /// The used HLL writer. | ||
| ShPtr<llvmir2hll::HLLWriter> hllWriter; | ||
|
|
||
| /// The used alias analysis. | ||
| ShPtr<llvmir2hll::AliasAnalysis> aliasAnalysis; | ||
| /// The used alias analysis. | ||
| ShPtr<llvmir2hll::AliasAnalysis> aliasAnalysis; | ||
|
|
||
| /// The used obtainer of information about function and function calls. | ||
| ShPtr<llvmir2hll::CallInfoObtainer> cio; | ||
| /// The used obtainer of information about function and function calls. | ||
| ShPtr<llvmir2hll::CallInfoObtainer> cio; | ||
|
|
||
| /// The used evaluator of arithmetical expressions. | ||
| ShPtr<llvmir2hll::ArithmExprEvaluator> arithmExprEvaluator; | ||
| /// The used evaluator of arithmetical expressions. | ||
| ShPtr<llvmir2hll::ArithmExprEvaluator> arithmExprEvaluator; | ||
|
|
||
| /// The used generator of variable names. | ||
| ShPtr<llvmir2hll::VarNameGen> varNameGen; | ||
| /// The used generator of variable names. | ||
| ShPtr<llvmir2hll::VarNameGen> varNameGen; | ||
|
|
||
| /// The used renamer of variables. | ||
| ShPtr<llvmir2hll::VarRenamer> varRenamer; | ||
| /// The used renamer of variables. | ||
| ShPtr<llvmir2hll::VarRenamer> varRenamer; | ||
|
|
||
| /// Output file stream. | ||
| std::unique_ptr<llvm::ToolOutputFile> outFile; | ||
| /// Output file stream. | ||
| std::unique_ptr<llvm::ToolOutputFile> outFile; | ||
|
|
||
| /// Output string stream. | ||
| std::unique_ptr<llvm::raw_string_ostream> outStringStream; | ||
| /// Output string stream. | ||
| std::unique_ptr<llvm::raw_string_ostream> outStringStream; | ||
| }; |
There was a problem hiding this comment.
The class LlvmIr2Hll has a large number of private member functions and member variables, which can make the class difficult to maintain and understand. This could be a sign that the class is trying to do too much and may benefit from being broken down into smaller, more focused classes or components.
Recommended Solution:
Consider refactoring the class to delegate some of its responsibilities to other classes. This will improve maintainability and readability by adhering to the Single Responsibility Principle (SRP).
| /// Should we recover from out-of-memory errors during optimizations? | ||
| bool recoverFromOutOfMemory; | ||
| /// Should we recover from out-of-memory errors during optimizations? | ||
| bool recoverFromOutOfMemory; |
There was a problem hiding this comment.
The recoverFromOutOfMemory member variable is declared but never initialized in the constructor. This could lead to undefined behavior if the variable is accessed before being explicitly set.
Recommended Solution:
Initialize recoverFromOutOfMemory in the constructor to a default value, such as false, to ensure it has a defined state.
There was a problem hiding this comment.
@restyled-io[bot]
Thank you for your contribution to this repository! We appreciate your effort in closing pull request.
Happy coding!
Automated style fixes for #7, created by Restyled.
The following restylers made fixes:
To incorporate these changes, merge this Pull Request into the original. We
recommend using the Squash or Rebase strategies.
NOTE: As work continues on the original Pull Request, this process will
re-run and update (force-push) this Pull Request with updated style fixes as
necessary. If the style is fixed manually at any point (i.e. this process finds
no fixes to make), this Pull Request will be closed automatically.
Sorry if this was unexpected. To disable it, see our documentation.