Skip to content

Commit

Permalink
btf: fix enum64 CO-RE relocations
Browse files Browse the repository at this point in the history 
Using CO-RE to do enum64 relocations is currently broken, since
our internal representation of a relocation only carries 32 bit of
expected constant value.

Fix this by extending the field to 64 bit and add a test.

Signed-off-by: Lorenz Bauer <[email protected]>
  • Loading branch information
@lmb @ti-mo
lmb authored and ti-mo committed Oct 30, 2023
1 parent ddb8518 commit 1df82a8
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 22 deletions.
38 changes: 19 additions & 19 deletions btf/core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
// COREFixup is the result of computing a CO-RE relocation for a target.
type COREFixup struct {
kind coreKind
local uint32
target uint32
local uint64
target uint64
// True if there is no valid fixup. The instruction is replaced with an
// invalid dummy.
poison bool
Expand Down Expand Up @@ -196,12 +196,12 @@ func CORERelocate(relos []*CORERelocation, target *Spec, bo binary.ByteOrder) ([

result[i] = COREFixup{
kind: relo.kind,
local: uint32(relo.id),
local: uint64(relo.id),
// NB: Using relo.id as the target here is incorrect, since
// it doesn't match the BTF we generate on the fly. This isn't
// too bad for now since there are no uses of the local type ID
// in the kernel, yet.
target: uint32(relo.id),
target: uint64(relo.id),
}
continue
}
Expand Down Expand Up @@ -311,10 +311,10 @@ var errNoSignedness = errors.New("no signedness")
// coreCalculateFixup calculates the fixup for a single local type, target type
// and relocation.
func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo binary.ByteOrder) (COREFixup, error) {
fixup := func(local, target uint32) (COREFixup, error) {
fixup := func(local, target uint64) (COREFixup, error) {
return COREFixup{kind: relo.kind, local: local, target: target}, nil
}
fixupWithoutValidation := func(local, target uint32) (COREFixup, error) {
fixupWithoutValidation := func(local, target uint64) (COREFixup, error) {
return COREFixup{kind: relo.kind, local: local, target: target, skipLocalValidation: true}, nil
}
poison := func() (COREFixup, error) {
Expand Down Expand Up @@ -346,7 +346,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
return fixup(1, 1)

case reloTypeIDTarget:
return fixup(uint32(relo.id), uint32(targetID))
return fixup(uint64(relo.id), uint64(targetID))

case reloTypeSize:
localSize, err := Sizeof(local)
Expand All @@ -359,7 +359,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
return zero, err
}

return fixup(uint32(localSize), uint32(targetSize))
return fixup(uint64(localSize), uint64(targetSize))
}

case reloEnumvalValue, reloEnumvalExists:
Expand All @@ -376,7 +376,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
return fixup(1, 1)

case reloEnumvalValue:
return fixup(uint32(localValue.Value), uint32(targetValue.Value))
return fixup(localValue.Value, targetValue.Value)
}

case reloFieldByteOffset, reloFieldByteSize, reloFieldExists, reloFieldLShiftU64, reloFieldRShiftU64, reloFieldSigned:
Expand Down Expand Up @@ -405,7 +405,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
return fixup(1, 1)

case reloFieldByteOffset:
return maybeSkipValidation(fixup(localField.offset, targetField.offset))
return maybeSkipValidation(fixup(uint64(localField.offset), uint64(targetField.offset)))

case reloFieldByteSize:
localSize, err := Sizeof(localField.Type)
Expand All @@ -417,24 +417,24 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
if err != nil {
return zero, err
}
return maybeSkipValidation(fixup(uint32(localSize), uint32(targetSize)))
return maybeSkipValidation(fixup(uint64(localSize), uint64(targetSize)))

case reloFieldLShiftU64:
var target uint32
var target uint64
if bo == binary.LittleEndian {
targetSize, err := targetField.sizeBits()
if err != nil {
return zero, err
}

target = uint32(64 - targetField.bitfieldOffset - targetSize)
target = uint64(64 - targetField.bitfieldOffset - targetSize)
} else {
loadWidth, err := Sizeof(targetField.Type)
if err != nil {
return zero, err
}

target = uint32(64 - Bits(loadWidth*8) + targetField.bitfieldOffset)
target = uint64(64 - Bits(loadWidth*8) + targetField.bitfieldOffset)
}
return fixupWithoutValidation(0, target)

Expand All @@ -444,7 +444,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
return zero, err
}

return fixupWithoutValidation(0, uint32(64-targetSize))
return fixupWithoutValidation(0, uint64(64-targetSize))

case reloFieldSigned:
switch local := UnderlyingType(localField.Type).(type) {
Expand All @@ -454,16 +454,16 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
return zero, fmt.Errorf("target isn't *Enum but %T", targetField.Type)
}

return fixup(boolToUint32(local.Signed), boolToUint32(target.Signed))
return fixup(boolToUint64(local.Signed), boolToUint64(target.Signed))
case *Int:
target, ok := as[*Int](targetField.Type)
if !ok {
return zero, fmt.Errorf("target isn't *Int but %T", targetField.Type)
}

return fixup(
uint32(local.Encoding&Signed),
uint32(target.Encoding&Signed),
uint64(local.Encoding&Signed),
uint64(target.Encoding&Signed),
)
default:
return zero, fmt.Errorf("type %T: %w", local, errNoSignedness)
Expand All @@ -474,7 +474,7 @@ func coreCalculateFixup(relo *CORERelocation, target Type, targetID TypeID, bo b
return zero, ErrNotSupported
}

func boolToUint32(val bool) uint32 {
func boolToUint64(val bool) uint64 {
if val {
return 1
}
Expand Down
Binary file modified btf/testdata/relocs-eb.elf
View file
Binary file not shown.
Binary file modified btf/testdata/relocs-el.elf
View file
Binary file not shown.
12 changes: 9 additions & 3 deletions btf/testdata/relocs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
#include "bpf_core_read.h"

enum e {
// clang-12 doesn't allow enum relocations with zero value.
// See https://reviews.llvm.org/D97659
ONE = 1,
ZERO = 0,
ONE,
TWO,
};

enum e64 {
LARGE = 0x1ffffffff,
};

typedef enum e e_t;

struct s {
Expand Down Expand Up @@ -147,12 +150,15 @@ __section("socket_filter/enums") int enums() {
enum_value_exists(volatile enum e, ONE);
enum_value_exists(const enum e, ONE);
enum_value_exists(e_t, TWO);
enum_value_exists(enum e64, LARGE);
// TODO: Check non-existence.

enum_value_matches(enum e, ZERO);
enum_value_matches(enum e, TWO);
enum_value_matches(e_t, ONE);
enum_value_matches(volatile e_t, ONE);
enum_value_matches(const e_t, ONE);
enum_value_matches(enum e64, LARGE);

return 0;
}
Expand Down

0 comments on commit 1df82a8

Please sign in to comment.