Introduce Finally to help track section rewrites #363
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Similarly to a concept used in NixOS/nix codebase, introduce the Finally class that can be given a lambda to execute on destructor. The desired effect is to execute some code on a scope leave, in this case the method return. This is used to make sure we only apply sectionRewrite once per modification and can now support early returns in the code paths.
|
This is desired since I saw some pull-requests may add early returns in the codepaths. Open to discussion on it :) |
fzakaria
commented
Dec 22, 2021
Comment on lines
+3
to
+9
| #include <string> | ||
| #include <functional> | ||
| #include <vector> | ||
| #include <optional> | ||
| #include <map> | ||
| #include <set> | ||
| #include <memory> |
There was a problem hiding this comment.
Tried to make this header file more "correct".
fzakaria
commented
Dec 22, 2021
| @@ -157,3 +169,15 @@ class ElfFile | |||
| return (const Elf_Ehdr *)fileContents->data(); | |||
| } | |||
| }; | |||
|
|
|||
| /* A trivial class to run a function at the end of a scope. */ | |||
| class Finally | |||
There was a problem hiding this comment.
Mic92
reviewed
Dec 23, 2021
|
|
||
| bool soNameModified = false; | ||
|
|
||
| Finally finally([&]() { |
There was a problem hiding this comment.
I would actually prefer the old code in cases where we don't need to handle early returns
to avoid hidden control flow. This one is such a case.
Mic92
reviewed
Dec 23, 2021
| } | ||
|
|
||
| template<ElfFileParams> | ||
| void ElfFile<ElfFileParamNames>::setInterpreter(const std::string & newInterpreter) | ||
| { | ||
| bool interpreterChanged = false; | ||
|
|
||
| Finally finally([&]() { |
Mic92
reviewed
Dec 23, 2021
| } else if (rdi(dyn->d_tag) == DT_RUNPATH) { | ||
| debug("removing DT_RUNPATH entry\n"); | ||
| changed = true; | ||
| rpathChanged = true; |
Mic92
reviewed
Dec 23, 2021
| } | ||
|
|
||
| if (rpath && rpath == newRPath) { | ||
| return; | ||
| } | ||
| changed = true; | ||
| rpathChanged = true; |
Mic92
reviewed
Dec 23, 2021
| @@ -1392,6 +1414,14 @@ void ElfFile<ElfFileParamNames>::removeNeeded(const std::set<std::string> & libs | |||
| { | |||
| if (libs.empty()) return; | |||
|
|
|||
| bool neededChanged = false; | |||
| Finally finally([&]() { | |||
There was a problem hiding this comment.
This one seems fine, but there should be no rewriteSections at the end.
Mic92
reviewed
Dec 23, 2021
| } | ||
|
|
||
| template<ElfFileParams> | ||
| void ElfFile<ElfFileParamNames>::addNeeded(const std::set<std::string> & libs) | ||
| { | ||
| if (libs.empty()) return; | ||
|
|
||
| bool neededChanged = false; | ||
| Finally finally([&]() { |
Mic92
reviewed
Dec 23, 2021
| } | ||
|
|
||
| template<ElfFileParams> | ||
| void ElfFile<ElfFileParamNames>::clearSymbolVersions(const std::set<std::string> & syms) | ||
| { | ||
| if (syms.empty()) return; | ||
|
|
||
| bool symbolVersionsChanged = false; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similarly to a concept used in NixOS/nix codebase, introduce the
Finallyclass that can be given a lambda to execute on destructor.The desired effect is to execute some code on a scope leave, in this case the method return.
This is used to make sure we only apply
sectionRewriteonce per modification and can now support early returns in the code paths.CC @reidpr