Add metadata to instructions returned by program info #1118
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
dylandreimerink
force-pushed
the
feature/program-info-metadata
branch
2 times, most recently
from
a7efb4a
to
917fec1
Compare
lmb
requested changes
Aug 16, 2023
There was a problem hiding this comment.
This is a really nice idea, when we wrote the current code to pull out instructions metadata wasn't a thing yet. This reminds me that we could also use metadata to tag map load isns we get from the kernel, I should create an issue.
Some high level thoughts: seems like what you really want to do is calling Assign(). All of the other stuff with ExtInfoBuilder is just a side-show. What do you think about the following?
- Export
funcInfoandlineInfo - Export constructors for
[]FuncInfo,[]LineInfo. Those should take a byte order, reader and a Spec probably. These are somewhere betweenparseFuncInfosandparseFuncInfoRecords: they take infos without the header and return a slice instead of a map. - Add a function
AssignInstructionMetadata(Instructions, []FuncInfo, []LineInfo)and call that fromExtInfos.Assign. The core relocation part we can keep inExtInfos.Assignfor now.
In general I want to avoid public APIs which assume native endian as much as possible. The same goes for the record size.
Yea, that is fair, sounds like a good way to approach this. I will make it so |
dylandreimerink
force-pushed
the
feature/program-info-metadata
branch
from
917fec1
to
303496f
Compare
lmb
requested changes
Sep 11, 2023
lmb
reviewed
Sep 13, 2023
dylandreimerink
force-pushed
the
feature/program-info-metadata
branch
2 times, most recently
from
428e523
to
2bc0dc6
Compare
Added exported types which contain slices of metadata objects so they can be used by other packages. In particular to in a followup commit. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Added two constructors which allow for parsing of BTF FuncInfos and LineInfos from their wire format. This will be used in a followup commit to parse BTF metadata from program info. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
The `ProgInfo.LineInfo` and `ProgInfo.FuncInfo` fields were uint64. This commit changes their type from uint64 to Pointer so pointers can be assigned to these fields in a follow up commit. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
dylandreimerink
force-pushed
the
feature/program-info-metadata
branch
from
2bc0dc6
to
ea5a274
Compare
lmb
reviewed
Sep 13, 2023
info_test.go
Outdated
|
|
||
| spec, err := LoadCollectionSpec("testdata/raw_tracepoint-el.elf") | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
Nit: these should be t.Fatal
Edit: i'll fix these up.
The instructions returned by program info gotten from the kernel did not contain any metadata such as line info or function info. This makes it harder to read the instructions when comparing the xlated instruction in the kernel with the spec. This commit uses the line info and function info that is available via the program info and adds it to the returned instructions. Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
lmb
force-pushed
the
feature/program-info-metadata
branch
from
ea5a274
to
4204efe
Compare
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.
The instructions returned by program info gotten from the kernel did
not contain any metadata such as line info or function info. This
makes it harder to read the instructions when comparing the xlated
instruction in the kernel with the spec.
This PR uses the line info and function info that is available via
the program info and adds it to the returned instructions.