Skip to content
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

btf: fix CO-RE bitfield relocations #1086

Merged
merged 4 commits into from
Jul 5, 2023
Merged

btf: fix CO-RE bitfield relocations #1086

merged 4 commits into from
Jul 5, 2023

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Jul 4, 2023

program: make CO-RE relocation failures more descriptive

It's difficult to figure out what went wrong during CO-RE relocation since
we don't know what the original relocation was. Add it to the error message:

    apply CO-RE relocations: fixup for CORERelocation(signed,
   Struct:"bits"[0:5], local_id=6): invalid immediate 0, expected 1
   (fixup: signed=1->1, kind: signed, ins: MovImm dst: r2 imm: 0)

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: fix reloFieldSigned

reloFieldSigned is used to figure out whether a struct or union field is
signed or unsigned. As such it must use coreAccessor to traverse a type and
find the correct fields. This currently doesn't happen and so we always
calculate the fixup against the struct or union. This means we never
classify a field as signed.

This behaviour has masked another bug: the current code assumes that enums
are always signed. Fix this by deriving the correct value from Enum.Signed.

Fix reloFieldSigned by moving the logic in with the other field based
relocation logic. This also fixes a possible panic due to an incorrect type
assertion when local and target are not of the same type.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: return an error when CO-RE bitfield relo encounters unsized type

It is valid to access a non-bitfield member of a struct using bitfield 
relocations. This happens when a member goes from being a bitfield to a
regular field. In this case we synthesize the size of the bitfield shifts
via the size of the regular field. Due to a bug we don't return an error
when retrieving the size.

Fix this by bubbling up the error and adding some testcases.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

btf: return an error if type has no signedness

Refuse to create a fixup for reloFieldSigned if the type isn't an Int or an
Enum. On clang-14 it's not even possible to create such a relocation, so no
need to be extra lenient.

Add a unit test since we can't get clang to emit a wonky relocation.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>

lmb added 4 commits July 4, 2023 14:50
It's difficult to figure out what went wrong during CO-RE relocation
since we don't know what the original relocation was. Add it to the
error message:

    apply CO-RE relocations: fixup for CORERelocation(signed,
    Struct:"bits"[0:5], local_id=6): invalid immediate 0, expected 1
    (fixup: signed=1->1, kind: signed, ins: MovImm dst: r2 imm: 0)

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
reloFieldSigned is used to figure out whether a struct or union
field is signed or unsigned. As such it must use coreAccessor to
traverse a type and find the correct fields. This currently doesn't
happen and so we always calculate the fixup against the struct or
union. This means we never classify a field as signed.

This behaviour has masked another bug: the current code assumes
that enums are always signed. Fix this by deriving the correct
value from Enum.Signed.

Fix reloFieldSigned by moving the logic in with the other field
based relocation logic. This also fixes a possible panic due to
an incorrect type assertion when local and target are not of the
same type.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
It is valid to access a non-bitfield member of a struct using bitfield
relocations. This happens when a member goes from being a bitfield to
a regular field. In this case we synthesize the size of the bitfield
shifts via the size of the regular field. Due to a bug we don't return
an error when retrieving the size.

Fix this by bubbling up the error and adding some testcases.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
Refuse to create a fixup for reloFieldSigned if the type isn't an
Int or an Enum. On clang-14 it's not even possible to create such
a relocation, so no need to be extra lenient.

Add a unit test since we can't get clang to emit a wonky relocation.

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
@lmb lmb requested review from rgo3 and joamaki July 4, 2023 14:09
@lmb lmb marked this pull request as ready for review July 4, 2023 14:09
btf/core.go Show resolved Hide resolved
@lmb lmb merged commit 4267cbc into cilium:master Jul 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants