Skip to content

Commit

Permalink
btf: inline FixupKind into COREFixup
Browse files Browse the repository at this point in the history 
Having a boolean to skip local validation is the simplest thing to do,
after all. Remove FixupKind and inline the boolean into COREFixup, but
invert the default. We want fixups to be validated by default instead
of the inverse. This also fixes a regression from the introduction of
FixupKind, where too many fixups have their validation skipped.
  • Loading branch information
@lmb
lmb committed Apr 12, 2022
1 parent 766dbb6 commit 2b0d817
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 39 deletions.
74 changes: 36 additions & 38 deletions internal/btf/core.go
View file
Expand Up @@ -18,12 +18,15 @@ import (

// COREFixup is the result of computing a CO-RE relocation for a target.
type COREFixup struct {
kind FixupKind
kind coreKind
local uint32
target uint32
// True if there is no valid fixup. The instruction is replaced with an
// invalid dummy.
poison bool
// True if the validation of the local value should be skipped. Used by
// some kinds of bitfield relocations.
skipLocalValidation bool
}

func (f COREFixup) equal(other COREFixup) bool {
Expand All @@ -44,7 +47,7 @@ func (f COREFixup) apply(ins *asm.Instruction) error {

switch class := ins.OpCode.Class(); class {
case asm.LdXClass, asm.StClass, asm.StXClass:
if want := int16(f.local); f.kind.validateLocal && want != ins.Offset {
if want := int16(f.local); !f.skipLocalValidation && want != ins.Offset {
return fmt.Errorf("invalid offset %d, expected %d", ins.Offset, f.local)
}

Expand All @@ -59,7 +62,7 @@ func (f COREFixup) apply(ins *asm.Instruction) error {
return fmt.Errorf("not a dword-sized immediate load")
}

if want := int64(f.local); f.kind.validateLocal && want != ins.Constant {
if want := int64(f.local); !f.skipLocalValidation && want != ins.Constant {
return fmt.Errorf("invalid immediate %d, expected %d (fixup: %v)", ins.Constant, want, f)
}

Expand All @@ -77,7 +80,7 @@ func (f COREFixup) apply(ins *asm.Instruction) error {
return fmt.Errorf("invalid source %s", src)
}

if want := int64(f.local); f.kind.validateLocal && want != ins.Constant {
if want := int64(f.local); !f.skipLocalValidation && want != ins.Constant {
return fmt.Errorf("invalid immediate %d, expected %d (fixup: %v, kind: %v, ins: %v)", ins.Constant, want, f, f.kind, ins)
}

Expand All @@ -95,7 +98,7 @@ func (f COREFixup) apply(ins *asm.Instruction) error {
}

func (f COREFixup) isNonExistant() bool {
return f.kind.coreKind.checksForExistence() && f.target == 0
return f.kind.checksForExistence() && f.target == 0
}

type COREFixups map[uint64]COREFixup
Expand Down Expand Up @@ -194,16 +197,6 @@ func (k coreKind) String() string {
}
}

// FixupKind is the type of CO-RE relocation.
type FixupKind struct {
coreKind coreKind
validateLocal bool
}

func (fk FixupKind) String() string {
return fk.coreKind.String()
}

func coreRelocate(local, target *Spec, relos CORERelos) (COREFixups, error) {
if local.byteOrder != target.byteOrder {
return nil, fmt.Errorf("can't relocate %s against %s", local.byteOrder, target.byteOrder)
Expand All @@ -221,10 +214,9 @@ func coreRelocate(local, target *Spec, relos CORERelos) (COREFixups, error) {
}

result[uint64(relo.insnOff)] = COREFixup{
FixupKind{coreKind: relo.kind},
uint32(relo.typeID),
uint32(relo.typeID),
false,
kind: relo.kind,
local: uint32(relo.typeID),
target: uint32(relo.typeID),
}
continue
}
Expand Down Expand Up @@ -330,7 +322,7 @@ func coreCalculateFixups(byteOrder binary.ByteOrder, local Type, targets []Type,
// targets at all. Poison everything!
bestFixups = make([]COREFixup, len(relos))
for i, relo := range relos {
bestFixups[i] = COREFixup{kind: FixupKind{coreKind: relo.kind}, poison: true}
bestFixups[i] = COREFixup{kind: relo.kind, poison: true}
}
}

Expand All @@ -340,14 +332,17 @@ func coreCalculateFixups(byteOrder binary.ByteOrder, local Type, targets []Type,
// coreCalculateFixup calculates the fixup for a single local type, target type
// and relocation.
func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID, target Type, targetID TypeID, relo CORERelocation) (COREFixup, error) {
fixup := func(local, target uint32, validateLocal bool) (COREFixup, error) {
return COREFixup{FixupKind{relo.kind, validateLocal}, local, target, false}, nil
fixup := func(local, target uint32) (COREFixup, error) {
return COREFixup{kind: relo.kind, local: local, target: target}, nil
}
fixupWithoutValidation := func(local, target uint32) (COREFixup, error) {
return COREFixup{kind: relo.kind, local: local, target: target, skipLocalValidation: true}, nil
}
poison := func() (COREFixup, error) {
if relo.kind.checksForExistence() {
return fixup(1, 0, true)
return fixup(1, 0)
}
return COREFixup{kind: FixupKind{coreKind: relo.kind}, local: 0, target: 0, poison: true}, nil
return COREFixup{kind: relo.kind, poison: true}, nil
}
zero := COREFixup{}

Expand All @@ -367,10 +362,10 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,

switch relo.kind {
case reloTypeExists:
return fixup(1, 1, true)
return fixup(1, 1)

case reloTypeIDTarget:
return fixup(uint32(localID), uint32(targetID), true)
return fixup(uint32(localID), uint32(targetID))

case reloTypeSize:
localSize, err := Sizeof(local)
Expand All @@ -383,7 +378,7 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
return zero, err
}

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

case reloEnumvalValue, reloEnumvalExists:
Expand All @@ -397,24 +392,23 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,

switch relo.kind {
case reloEnumvalExists:
return fixup(1, 1, true)
return fixup(1, 1)

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

case reloFieldSigned:
switch local.(type) {
case *Enum:
return fixup(1, 1, true)
return fixup(1, 1)
case *Int:
return fixup(
uint32(local.(*Int).Encoding&Signed),
uint32(target.(*Int).Encoding&Signed),
true,
)
default:
return fixup(0, 0, false)
return fixupWithoutValidation(0, 0)
}

case reloFieldByteOffset, reloFieldByteSize, reloFieldExists, reloFieldLShiftU64, reloFieldRShiftU64:
Expand All @@ -433,13 +427,17 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
return zero, fmt.Errorf("target %s: %w", target, err)
}

validateLocal := localField.bitfieldSize == 0
maybeSkipValidation := func(f COREFixup, err error) (COREFixup, error) {
f.skipLocalValidation = localField.bitfieldSize > 0
return f, err
}

switch relo.kind {
case reloFieldExists:
return fixup(1, 1, validateLocal)
return fixup(1, 1)

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

case reloFieldByteSize:
localSize, err := Sizeof(localField.Type)
Expand All @@ -451,7 +449,7 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,
if err != nil {
return zero, err
}
return fixup(uint32(localSize), uint32(targetSize), validateLocal)
return maybeSkipValidation(fixup(uint32(localSize), uint32(targetSize)))

case reloFieldLShiftU64:
var target uint32
Expand All @@ -470,15 +468,15 @@ func coreCalculateFixup(byteOrder binary.ByteOrder, local Type, localID TypeID,

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

case reloFieldRShiftU64:
targetSize, err := targetField.sizeBits()
if err != nil {
return zero, err
}

return fixup(0, 64-targetSize, false)
return fixupWithoutValidation(0, 64-targetSize)
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/btf/core_test.go
View file
Expand Up @@ -557,7 +557,7 @@ func TestCORERelocation(t *testing.T) {
}

for offset, relo := range relos {
if want := relo.local; relo.kind.validateLocal && want != relo.target {
if want := relo.local; !relo.skipLocalValidation && want != relo.target {
// Since we're relocating against ourselves both values
// should match.
t.Errorf("offset %d: local %v doesn't match target %d (kind %s)", offset, relo.local, relo.target, relo.kind)
Expand Down

0 comments on commit 2b0d817

Please sign in to comment.