-
Notifications
You must be signed in to change notification settings - Fork 25
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
Introduce Input enum for symbolization APIs #347
Closed
Closed
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
With upcoming changes we are going to rework what amount of work is performed as part of normalization (#321). As a result of this rework, normalization will produce file offsets as opposed to (normalized) addresses. In order to keep supporting end-to-end symbolization workflow where the output of normalization is fed to the symbolization APIs, this change introduces the Input enum. This enum can be used to convey to the symbolization APIs what kind of input is being provided. Not all sources support all input types and a runtime error will be reported if an unsupported input combination is used. This enum effectively formalizes (via the type system) part of what previously was implicit knowledge: that different symbolization sources require different inputs. Specifically, process symbolization will work with what we refer to as absolute addresses while ELF/Gsym will work with virtual offsets. Signed-off-by: Daniel Müller <[email protected]>
This is just a sneak peek at this point. Naming suggestions would be welcome. |
danielocfb
force-pushed
the
topic/rework-normalization
branch
2 times, most recently
from
October 4, 2023 22:07
4aa650b
to
af2c683
Compare
anakryiko
reviewed
Oct 4, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming is hard, but left my thoughts
danielocfb
force-pushed
the
topic/rework-normalization
branch
7 times, most recently
from
October 11, 2023 17:44
798678c
to
3cbe5e1
Compare
This change converts the normalization result from reporting addresses to reporting file offsets. As a result, we perform less work on the tangentially slower "remote" system, eliminating the need for any kind of object file caching there. As another consequence, we report less data for the APK case: instead of identifying the contained ELF file in which an address lies, we just report the path to the APK itself. It will only be the symbolization step that identifies the ELF file in question and uses it to translate the original address. Closes: #321 Signed-off-by: Daniel Müller <[email protected]>
danielocfb
force-pushed
the
topic/rework-normalization
branch
3 times, most recently
from
October 11, 2023 23:29
0f8432a
to
670e4b9
Compare
This change adds support for symbolization of file offsets (in addition to virtual offsets) of ELF sources. When providing file offsets the library will take care of the file offset to virtual offset conversion as a first step. Signed-off-by: Daniel Müller <[email protected]>
With upcoming changes our normalization functionality will return file offsets instead of normalized addresses. That renders a bunch of the existing names containing "addr" unusable. Rename all those types, opting for "output" or similar terminology instead of "addr". Signed-off-by: Daniel Müller <[email protected]>
Once we rework the normalization APIs to work with file offsets (and perform fewer steps on the "remote" device), we are going to need to perform the ELF-in-APK lookup as part of the "actual" symbolization step. This change introduces a symbolization source that can be used to go about that. Signed-off-by: Daniel Müller <[email protected]>
With this change we implement the APK symbolization logic. Signed-off-by: Daniel Müller <[email protected]>
danielocfb
force-pushed
the
topic/rework-normalization
branch
from
October 12, 2023 20:26
670e4b9
to
73183f5
Compare
Will open a new PR. |
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.
No description provided.