From 507736ee5df5906b39d9af8662b3d198b48c918d Mon Sep 17 00:00:00 2001 From: Ron Federman Date: Sat, 13 Apr 2024 14:12:35 +0300 Subject: [PATCH 1/2] Only consider child entries in DWARF lookup once we found the struct entry. Fix fallback to single offset only when version is unknown --- internal/pkg/inject/offset_results.json | 206 +----------------------- internal/pkg/structfield/structfield.go | 3 +- internal/tools/inspect/app.go | 22 ++- 3 files changed, 26 insertions(+), 205 deletions(-) diff --git a/internal/pkg/inject/offset_results.json b/internal/pkg/inject/offset_results.json index df439c7ab..9de8e2919 100644 --- a/internal/pkg/inject/offset_results.json +++ b/internal/pkg/inject/offset_results.json @@ -204,23 +204,6 @@ "1.0.2", "1.0.3", "1.0.4", - "1.0.5", - "1.2.0", - "1.2.1", - "1.3.0", - "1.4.0", - "1.4.1", - "1.4.2", - "1.5.0", - "1.5.1", - "1.5.2", - "1.6.0", - "1.7.0", - "1.7.1", - "1.7.2", - "1.7.3", - "1.7.4", - "1.7.5", "1.8.0", "1.8.2", "1.9.0", @@ -392,6 +375,7 @@ { "offset": 24, "versions": [ + "1.14.0", "1.15.0", "1.16.0", "1.17.0", @@ -423,36 +407,6 @@ "1.0.2", "1.0.3", "1.0.4", - "1.3.0", - "1.4.0", - "1.4.1", - "1.4.2", - "1.5.0", - "1.5.1", - "1.5.2", - "1.6.0", - "1.7.0", - "1.7.1", - "1.7.2", - "1.7.3", - "1.7.4", - "1.7.5", - "1.8.0", - "1.8.2", - "1.9.0", - "1.9.1", - "1.9.2", - "1.10.0", - "1.10.1", - "1.11.0", - "1.11.1", - "1.11.2", - "1.11.3", - "1.12.0", - "1.12.1", - "1.12.2", - "1.13.0", - "1.14.0", "1.25.0", "1.25.1", "1.26.0", @@ -582,38 +536,6 @@ "1.0.2", "1.0.3", "1.0.4", - "1.0.5", - "1.2.0", - "1.2.1", - "1.3.0", - "1.4.0", - "1.4.1", - "1.4.2", - "1.5.0", - "1.5.1", - "1.5.2", - "1.6.0", - "1.7.0", - "1.7.1", - "1.7.2", - "1.7.3", - "1.7.4", - "1.7.5", - "1.8.0", - "1.8.2", - "1.9.0", - "1.9.1", - "1.9.2", - "1.10.0", - "1.10.1", - "1.11.0", - "1.11.1", - "1.11.2", - "1.11.3", - "1.12.0", - "1.12.1", - "1.12.2", - "1.13.0", "1.14.0", "1.15.0", "1.16.0", @@ -760,6 +682,7 @@ { "offset": 64, "versions": [ + "1.14.0", "1.15.0", "1.16.0", "1.17.0", @@ -833,36 +756,6 @@ "1.0.2", "1.0.3", "1.0.4", - "1.3.0", - "1.4.0", - "1.4.1", - "1.4.2", - "1.5.0", - "1.5.1", - "1.5.2", - "1.6.0", - "1.7.0", - "1.7.1", - "1.7.2", - "1.7.3", - "1.7.4", - "1.7.5", - "1.8.0", - "1.8.2", - "1.9.0", - "1.9.1", - "1.9.2", - "1.10.0", - "1.10.1", - "1.11.0", - "1.11.1", - "1.11.2", - "1.11.3", - "1.12.0", - "1.12.1", - "1.12.2", - "1.13.0", - "1.14.0", "1.37.0", "1.37.1", "1.38.0-dev", @@ -960,38 +853,6 @@ "1.0.2", "1.0.3", "1.0.4", - "1.0.5", - "1.2.0", - "1.2.1", - "1.3.0", - "1.4.0", - "1.4.1", - "1.4.2", - "1.5.0", - "1.5.1", - "1.5.2", - "1.6.0", - "1.7.0", - "1.7.1", - "1.7.2", - "1.7.3", - "1.7.4", - "1.7.5", - "1.8.0", - "1.8.2", - "1.9.0", - "1.9.1", - "1.9.2", - "1.10.0", - "1.10.1", - "1.11.0", - "1.11.1", - "1.11.2", - "1.11.3", - "1.12.0", - "1.12.1", - "1.12.2", - "1.13.0", "1.14.0", "1.15.0", "1.16.0", @@ -1143,38 +1004,6 @@ "1.0.2", "1.0.3", "1.0.4", - "1.0.5", - "1.2.0", - "1.2.1", - "1.3.0", - "1.4.0", - "1.4.1", - "1.4.2", - "1.5.0", - "1.5.1", - "1.5.2", - "1.6.0", - "1.7.0", - "1.7.1", - "1.7.2", - "1.7.3", - "1.7.4", - "1.7.5", - "1.8.0", - "1.8.2", - "1.9.0", - "1.9.1", - "1.9.2", - "1.10.0", - "1.10.1", - "1.11.0", - "1.11.1", - "1.11.2", - "1.11.3", - "1.12.0", - "1.12.1", - "1.12.2", - "1.13.0", "1.14.0", "1.15.0", "1.16.0", @@ -1412,6 +1241,7 @@ { "offset": 356, "versions": [ + "1.14.0", "1.15.0", "1.16.0", "1.17.0", @@ -1437,36 +1267,6 @@ { "offset": 404, "versions": [ - "1.3.0", - "1.4.0", - "1.4.1", - "1.4.2", - "1.5.0", - "1.5.1", - "1.5.2", - "1.6.0", - "1.7.0", - "1.7.1", - "1.7.2", - "1.7.3", - "1.7.4", - "1.7.5", - "1.8.0", - "1.8.2", - "1.9.0", - "1.9.1", - "1.9.2", - "1.10.0", - "1.10.1", - "1.11.0", - "1.11.1", - "1.11.2", - "1.11.3", - "1.12.0", - "1.12.1", - "1.12.2", - "1.13.0", - "1.14.0", "1.58.0", "1.58.1", "1.58.2", diff --git a/internal/pkg/structfield/structfield.go b/internal/pkg/structfield/structfield.go index 30a0506eb..859868319 100644 --- a/internal/pkg/structfield/structfield.go +++ b/internal/pkg/structfield/structfield.go @@ -19,6 +19,7 @@ import ( "encoding/json" "fmt" "sort" + "strings" "sync" "github.com/hashicorp/go-version" @@ -248,7 +249,7 @@ func (o *Offsets) Get(ver *version.Version) (uint64, bool) { v, ok := o.values[newVerKey(ver)] o.mu.RUnlock() - if !ok && o.uo.valid { + if strings.HasPrefix(ver.String(), "0.0.0") && !ok && o.uo.valid { // If we don't have the exact version, but we only have one offset, we // fallback to use that offset. This can happen when a non official version is being used // which contains commit hash in the version string. diff --git a/internal/tools/inspect/app.go b/internal/tools/inspect/app.go index 2cfd6921c..753d67997 100644 --- a/internal/tools/inspect/app.go +++ b/internal/tools/inspect/app.go @@ -100,7 +100,7 @@ func (a *app) GetOffset(id structfield.ID) (uint64, bool) { return 0, false } - e, err := findEntry(r, dwarf.TagMember, id.Field) + e, err := findEntryInChildren(r, dwarf.TagMember, id.Field) if err != nil { return 0, false } @@ -145,6 +145,26 @@ func findEntry(r *dwarf.Reader, tag dwarf.Tag, name string) (*dwarf.Entry, error return nil, errors.New("not found") } +// findEntryInChildren returns the DWARF entry with a tag equal to name read from r, only +// considering the children of the current entry. An error is returned if the entry cannot be found. +func findEntryInChildren(r *dwarf.Reader, tag dwarf.Tag, name string) (*dwarf.Entry, error) { + for { + entry, err := r.Next() + if err == io.EOF || entry == nil || entry.Tag == 0 { + break + } + + if entry.Tag == tag { + if f, ok := entryField(entry, dwarf.AttrName); ok { + if name == f.Val.(string) { + return entry, nil + } + } + } + } + return nil, errors.New("not found") +} + // entryField returns the DWARF field from DWARF entry e that has the passed // DWARF attribute a. func entryField(e *dwarf.Entry, a dwarf.Attr) (dwarf.Field, bool) { From 69e4fb00b68e1916a09f48b339fa6ffc2bd5c0c4 Mon Sep 17 00:00:00 2001 From: Ron Federman Date: Fri, 19 Apr 2024 21:08:22 +0300 Subject: [PATCH 2/2] offsetgen - add go get command when building an app to make sure we are using the intended version --- internal/pkg/inject/offset_results.json | 66 ------------------------- internal/tools/inspect/app.go | 7 ++- internal/tools/inspect/builder.go | 14 ++++-- 3 files changed, 17 insertions(+), 70 deletions(-) diff --git a/internal/pkg/inject/offset_results.json b/internal/pkg/inject/offset_results.json index 078185c50..d990ce467 100644 --- a/internal/pkg/inject/offset_results.json +++ b/internal/pkg/inject/offset_results.json @@ -787,30 +787,6 @@ "0.4.46", "0.4.47" ] - }, - { - "offset": 24, - "versions": [ - "0.1.0", - "0.2.0", - "0.2.1", - "0.2.2", - "0.2.3", - "0.2.4", - "0.2.5", - "0.3.0", - "0.3.1", - "0.3.2", - "0.3.3", - "0.3.4", - "0.3.5", - "0.3.6", - "0.3.7", - "0.3.8", - "0.3.9", - "0.3.10", - "0.4.0" - ] } ] } @@ -892,13 +868,6 @@ { "offset": 64, "versions": [ - "0.2.2", - "0.2.3", - "0.3.0", - "0.4.0", - "0.4.1", - "0.4.2", - "0.4.3", "1.20.0", "1.21.0", "1.22.0", @@ -1020,11 +989,6 @@ { "offset": 24, "versions": [ - "1.0.0", - "1.0.1-GA", - "1.0.2", - "1.0.3", - "1.0.4", "1.8.0", "1.8.2", "1.9.0", @@ -1223,11 +1187,6 @@ { "offset": 32, "versions": [ - "1.0.0", - "1.0.1-GA", - "1.0.2", - "1.0.3", - "1.0.4", "1.25.0", "1.25.1", "1.26.0", @@ -1352,11 +1311,6 @@ { "offset": 0, "versions": [ - "1.0.0", - "1.0.1-GA", - "1.0.2", - "1.0.3", - "1.0.4", "1.14.0", "1.15.0", "1.16.0", @@ -1572,11 +1526,6 @@ { "offset": 80, "versions": [ - "1.0.0", - "1.0.1-GA", - "1.0.2", - "1.0.3", - "1.0.4", "1.37.0", "1.37.1", "1.38.0-dev", @@ -1669,11 +1618,6 @@ { "offset": 8, "versions": [ - "1.0.0", - "1.0.1-GA", - "1.0.2", - "1.0.3", - "1.0.4", "1.14.0", "1.15.0", "1.16.0", @@ -1820,11 +1764,6 @@ { "offset": 0, "versions": [ - "1.0.0", - "1.0.1-GA", - "1.0.2", - "1.0.3", - "1.0.4", "1.14.0", "1.15.0", "1.16.0", @@ -2112,11 +2051,6 @@ { "offset": 412, "versions": [ - "1.0.0", - "1.0.1-GA", - "1.0.2", - "1.0.3", - "1.0.4", "1.52.0", "1.52.1", "1.52.3", diff --git a/internal/tools/inspect/app.go b/internal/tools/inspect/app.go index 753d67997..233591fc1 100644 --- a/internal/tools/inspect/app.go +++ b/internal/tools/inspect/app.go @@ -69,7 +69,12 @@ func newApp(ctx context.Context, l logr.Logger, j job) (*app, error) { return nil, err } - a.exec, err = j.Builder.Build(ctx, a.tmpDir, a.AppVer) + if len(j.Fields) == 0 { + return nil, errors.New("no fields to analyze") + } + modName := j.Fields[0].ModPath + + a.exec, err = j.Builder.Build(ctx, a.tmpDir, a.AppVer, modName) if err != nil { return nil, err } diff --git a/internal/tools/inspect/builder.go b/internal/tools/inspect/builder.go index 9b3af1446..956aa235f 100644 --- a/internal/tools/inspect/builder.go +++ b/internal/tools/inspect/builder.go @@ -66,16 +66,24 @@ func newBuilder(l logr.Logger, cli *client.Client, goVer *version.Version) *buil } // Build builds the appV version of a Go application located in dir. -func (b *builder) Build(ctx context.Context, dir string, appV *version.Version) (string, error) { +func (b *builder) Build(ctx context.Context, dir string, appV *version.Version, modName string) (string, error) { b.log.V(2).Info("building application...", "version", appV, "dir", dir, "image", b.GoImage) app := fmt.Sprintf("app%s", appV.Original()) + goGetCmd := fmt.Sprintf("go get %s@%s", modName, appV.Original()) + goModTidyCmd := "go mod tidy -compat=1.17" var cmd string + if b.goVer != nil && b.goVer.LessThan(minCompatVer) { - cmd = "go mod tidy && go build -o " + app + goModTidyCmd = "go mod tidy" + } + + if b.goVer == nil { + cmd = fmt.Sprintf("%s && %s && go build -o %s", goModTidyCmd, goGetCmd, app) } else { - cmd = "go mod tidy -compat=1.17 && go build -o " + app + cmd = fmt.Sprintf("%s && go build -o %s", goModTidyCmd, app) } + if err := b.runCmd(ctx, []string{"sh", "-c", cmd}, dir); err != nil { return "", err }