Add Maps to Instruction metadata

[dylandreimerink]

This PR improves the existing per-instruction metadata, as discussed in #564 and fixes #515.

This PR moves the existing Instruction.Reference and Instruction.Symbol metadata to a separate struct called metadata. asm.Instruction now has a pointer to this metadata struct. The Instruction struct now has exported getters and setters to get and modify this metadata. The design is such that we only assign metadata to instructions if we have to, since most instructions don't have metadata.

Copying instructions will not automatically copy the metadata since Instruction has a pointer to metadata. Therefor the setters will perform a copy-on-write, so upon the first modification of metadata, the instruction will get a new metadata object which is copied from the existing one and then modified.

These setters all have value receivers which return the modified instruction. This choice was made because it makes it easier to use the setters in the DSL, since you can't call functions with pointer receivers on literals. By returning a modified instruction we make method chaining possible, which fits well with the rest of the DSL.

We also added Filename, Line and LineCol to the metadata and getters and setters for them to Instruction. The ELF reader was modified to add looked-up LineInfo from the .BTF.ext section to the program instructions. This allows us to print the original source code info along with the program instructions when formatting asm.Instructions.

By attaching this info the the instruction makes it relocation-proof. In future PRs we might even be able to marshal blobs to be loaded into the kernel so would guarantee correct BTF line info for relocated programs and BTF line info for programs created via the DSL.

Lastly, this PR adds map references to the per-instruction metadata. This allows us to add references to ebpf.Map to the program instructions themselves, specifically during rewriting, which prevents the maps from being garbage collected, thus fixing #515

[lmb]

Hi Dylan,

Thanks for the PR! I started the review yesterday, but only managed to finish it today.

• Can you explain what the kernel uses the various bits of information for? I know that SetLine ends up in the verifier log, but what about File, Line, Column?
• How does the library use this information? Seems like we ignore File, Line, Column?
• Is it possible to only pass e.g. Line, but leave other bits empty?

Regarding the metadata, I'd prefer if the setters modified the instruction instead of making a copy, so pointer receivers. Behind the scenes metadata would still be copied. Intuitively I expect a setter to make changes to the object I'm calling it on. Calling the setters also becomes awkward judging from the changes in the PRs.

I also saw that you merged master into your feature branch, maybe that was an accident? Doing that makes merges much messier, please rebase instead.

[dylandreimerink]

Can you explain what the kernel uses the various bits of information for? I know that SetLine ends up in the verifier log, but what about File, Line, Column?

As far as I know, the kernel doesn't use any of the .BTF.ext data(LineInfos and FuncInfos) itself, as far as I can tell from the initial commit the main goal was to add source code references for in the verifier as well as when reading the xlated and jitted instructions back from the kernel.

How does the library use this information? Seems like we ignore File, Line, Column?

I used the Line to add to Instructions.Format which now has a output similar to bpftool and llvm when outputting instructions with lineinfo.

The Filename and Column information isn't currently used by the library directly. My motivation for adding is was to use them outside of the library in edb where I use the information to display the relevant source file and place the cursor at the correct line within that file.

Is it possible to only pass e.g. Line, but leave other bits empty?

Yea it should be. If we were to create a LineInfo object from the metadata we can leave the LineCol and FileNameOff at 0. Which I believe are valid values, since the line and column can both be zero and a offset of 0 within the strings table will just result in an empty string. I think the most important part is to not have duplicate InsnOff values within the same function, which should not be an issue.

[dylandreimerink]

Got almost everything. The only thing left is to make btf.LineInfo a fmt.Stringer, which we can do as soon as #572 has been merged. I can rebase and update WIP: asm: Replaced seperate getters/setters with Context once that is done.

[ti-mo]

Went through it in detail, great work! This has been on our minds for a few months now and will open some exciting new possibilities.

[lmb]

@dylandreimerink can you add a Fixes #515 to the commit with map metadata?

[ti-mo]

Have a few last comments. Looks like the commit title and message of asm,elf_reader: added instruction context is now outdated.

As discussed offline, let's only merge the Map support here, the .BTF.ext unmarshaler needs a few changes since we're processing LineInfos per section again.

[ti-mo]

The LineInfo metadata was split off from this PR as this one already touched several subtle interactions regarding maps between the loader, linker and marshaler. This one is good to go for now.