Skip to content
This repository was archived by the owner on Jan 31, 2025. It is now read-only.

Parse modules for hotpatchable functions.#4497

Merged
danielfenner merged 3 commits intogoogle:mainfrom
danielfenner:hotpatchable_integrate
Nov 29, 2022
Merged

Parse modules for hotpatchable functions.#4497
danielfenner merged 3 commits intogoogle:mainfrom
danielfenner:hotpatchable_integrate

Conversation

@danielfenner
Copy link
Copy Markdown
Collaborator

In order to represent functions in Orbit we read information from various sources: elf files, coff files, pdb's. In the end all of these produce SymbolInfo protos. This change adds a is_hotpatchable field to SymbolInfo. Only for elf files we parse for hotpatchable functions and set the new field to true if appropriate. Msvc also supports hotpatchable functions but for now we ignore this.

SymbolInfo is translated into FunctionInfo - so this also gets the is_hotpatchable field.

FunctionInfo is translated to a InstrumentedFunction in the capture options. The is_hotpatchable field is added there as well.

The rest of the PR is basically adding the new is_hotpatchable field to all the tests. The only real addition to testing is in ElfFileTest - it is parsing the hotpatchable function section from a test binary.

In order to represent functions in Orbit we read information from
various sources: elf files, coff files, pdb's. In the end all of these
produce SymbolInfo protos. This change adds a is_hotpatchable field to
SymbolInfo. Only for elf files we parse for hotpatchable functions and
set the new field to true if appropriate. Msvc also supports
hotpatchable functions but for now we ignore this.

SymbolInfo is translated into FunctionInfo - so this also gets the
is_hotpatchable field.

FunctionInfo is translated to a InstrumentedFunction in the capture
options. The is_hotpatchable field is added there as well.

The rest of the PR is basically adding the new is_hotpatchable field to
all the tests. The only real addition to testing is in ElfFileTest - it
is parsing the hotpatchable function section from a test binary.
Copy link
Copy Markdown
Collaborator

@florian-kuebler florian-kuebler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the change. Lgtm. Some minor comments and thoughts.

Comment thread src/ClientData/DataManagerTest.cpp Outdated
Comment thread src/ClientData/UserDefinedCaptureDataTest.cpp Outdated
Comment thread src/CodeReport/AnnotateDisassemblyTest.cpp
Comment thread src/ObjectUtils/CoffFile.cpp
Comment thread src/ObjectUtils/CoffFileTest.cpp Outdated
Comment thread src/ObjectUtils/ElfFile.cpp
Comment thread src/ObjectUtils/ElfFile.cpp
Comment thread src/ObjectUtils/ElfFile.cpp Outdated
Comment thread src/ObjectUtils/ElfFileTest.cpp
Comment thread src/ObjectUtils/ElfFileTest.cpp Outdated
Comment thread src/ObjectUtils/ElfFileTest.cpp
Comment thread src/ObjectUtils/ElfFile.cpp
Comment thread src/ObjectUtils/ElfFile.cpp
Copy link
Copy Markdown
Collaborator

@florian-kuebler florian-kuebler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the comments!

Comment thread src/ObjectUtils/ElfFile.cpp
Comment thread src/ObjectUtils/ElfFile.cpp
Comment thread src/ObjectUtils/ElfFileTest.cpp
Comment thread src/ObjectUtils/CoffFile.cpp
Comment thread src/ObjectUtils/ElfFile.cpp Outdated
Copy link
Copy Markdown
Collaborator

@ronaldfw ronaldfw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danielfenner
Copy link
Copy Markdown
Collaborator Author

Thanks for the reviews!

@danielfenner danielfenner merged commit fbfa63d into google:main Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants