Reusable amd64 interpreter for extraction of values from disassembly

[korniltsev]

This PR is continuation of #412
In #412, I've added amd instruction interpreting logic to the python stub decode routine.

In this followup, I extract this interpreting logic into reusable amd.Interpreter.

To make it reusable in other packages I introduce new variable package.

The main new type here is variable.Expression interface which is common ground to facilitate arithmetic operations on register and unknown memory values.
The variable package support the following:

• immediate values (variable.Imm),
• some arithmetic operations like Add, Mul on Expression value operands
• Zero/Sign Extend operation (used to load eax from RAX)
• holding unknown values loaded from memory with variable.Mem,
• holding variable unknown values variable.Variable, which can Match and extract immediate values

The most useful method of Expression is Match. Here is the snippet from its doc:

	// Match compares this Expression value against a pattern Expression.
	// The order of the arguments matters: a.Match(b) or b.Match(a) may
	// produce different results. The intended order The pattern should be passed as an argument, not
	// the other way around.
	// It returns true if the values are considered equal or compatible according to
	// the type-specific rules:
	// - For operations (add, mul): checks if operation types and operands match
	// - For immediate: checks if values are equal and extracts value into a Variable
	// - For memory references: checks if segments and addresses match
	// - For extend operations: checks if sizes and inner values match
	// - For variables: checks if they are pointing to the same object instance.

Now with the help of the new variable package, the new Interpreter can interpret amd instructions up to a certain point. Previously we used to have regState as the state of registers which had limited reusablity

type regState struct {
	LoadedFrom uint64
	Value      uint64
}

Now we use variable.Expression as the state of registers.

This allows us to interpret needed instructions up to a certain point and then Match the result of the computation (Expression) against some expected pattern(Expression) and extract needed offsets/addresses along the way.

In this PR I only change the python decoding routine to keep the change smaller. You can see how it will be used in other decoding routines (php, libc, fsbase) in this branch https://github.com/open-telemetry/opentelemetry-ebpf-profiler/compare/main...grafana:opentelemetry-ebpf-profiler:libc?expand=1
I've uploaded php, libc changes as well.

[fabled]

Wow. Cool stuff! First round of comments added. Any particular reason why this is marked draft?

[korniltsev]

Wow. Cool stuff! First round of comments added. Any particular reason why this is marked draft?

I found a need to rework Crop into SignExtend ZeroExtend. And I did not have time to finish this yet. I hope to get back to this PR this week.

Thanks for the review. I will address your comments

[korniltsev]

@fabled Thank you for your review. I've addressed your questions and marked the PR as ready for review.
I'd appreciate if you take another look. 🙏

[fabled]

First quick round done. Will do also a bit more detailed review on the code since the draft state was now removed.

Mostly just naming things things. The other thing missing is a bit improved documentation. Typically most structs and functions have some sort of comment attached. But I'd love to start building things on this, so perhaps we can leave the documentation as a follow up PR if other maintainer(s) agree.

[fabled]

The number of registers does not make sense to me. Perhaps make a const maxRegs, and explain where it comes from?

Currently it seems: maximum register is TR7 = 154, or maximum register used in this code is RIP = 71. Could use directly the register name in the definition of the maximum?

If I read the code correctly regEntry is a mapping from x86asm register to the register indexes used internally in this code? How about making this registerMappings [maxX86Register]registerMapping or similar?

[korniltsev]

made id RIP+1

[korniltsev]

on a second thought. I think this code may break if x86asm decides to change the values for the constants. I decoded to change this to a switch case code.

[fabled]

regMappingFor?

[fabled]

The idx values used here are internal register assignments. Perhaps constants could be added for these?

[korniltsev]

e4e2771

[fabled]

nit: MachineState? or RegStates/RegisterStates? or just Registers?

[fabled]

This would also look a lot neater if we had our own constants for the registers. Or could this be made a loop?

[korniltsev]

made a loop

[fabled]

I feel that Any() and Var() are bit ambiguous and overloaded. Since Var is used to construct both non-capturing (register initial states; basically just named unknown value) and capturing (Match target to extract matched subexpression) variants.

Perhaps the constructors could be something like:

• CaptureExpression()
• CaptureImmediate()
• NamedState()
• Constant()

or similar?

Perhaps even construct different type of struct out of these unless it makes the Match implementation too much more complicated. But I'd assume the NamedState type such as the initial register states when used as match target should match anything else then itself?

[korniltsev]

I've removed Any. I propose to keep the single Var for now. It has a name and it can match & capture an immediate. Sometimes we use the capture functionality sometimes we don't. It looks fine to me.

Let me know if you strongly believe we need to split Var to NamedState and CaptureImmediate, I will do this.

[fabled]

Let me know if you strongly believe we need to split Var to NamedState and CaptureImmediate, I will do this.

From programming safety point of view, I think it would be good to split these. The problem I see is that unknown developer might misuse Var. It also creates ambiguity as Match target. I would feel much more safer if these two functional use cases have different backing data type.

[korniltsev]

ok. Split into Named and ImmediateCapture

[fabled]

Few comments still, but looking good enough so I'll give an early approval to get a second review from @christos68k or @florianl ?

[fabled]

Seems this was like this, but why not inline this at the only call site, and use the pfelf.File.Machine to determine the machine type.

[florianl]

Just minor comments. Thanks for working on this!

Once merge conflicts are resolved, this should be merged :)

[florianl]

General observation: All switch cases call i.Regs.setX86asm() if everything works as expected. Should we inform the user, that something unexpected happened, if i.Regs.setX86asm() is not used in some cases?

[korniltsev]

There are instructions and instruction types that currently do nothing in the interpreter and ignored, for example writing or reading memory, tests, jumps. We should not inform on those. We have tests for stub decodings, they should be enough.