Skip to content

Commit

Permalink
Handle 64-bit negative addresses when adjusting them. (#397)
Browse files Browse the repository at this point in the history
Fixes #280. 64-bit addresses with the high bit set should be adjustable
as long as the offset does not overflow the result. This change
supersedes #396 avoiding `math/big` dependency.
  • Loading branch information
aalexand authored and nolanmar511 committed Jul 9, 2018
1 parent 2b5d435 commit 2c569c7
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 9 deletions.
35 changes: 26 additions & 9 deletions internal/symbolz/symbolz.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ func symbolizeMapping(source string, offset int64, syms func(string, string) ([]
for _, l := range p.Location {
if l.Mapping == m && l.Address != 0 && len(l.Line) == 0 {
// Compensate for normalization.
addr := int64(l.Address) + offset
if addr < 0 {
return fmt.Errorf("unexpected negative adjusted address, mapping %v source %d, offset %d", l.Mapping, l.Address, offset)
addr, overflow := adjust(l.Address, offset)
if overflow {
return fmt.Errorf("cannot adjust address %d by %d, it would overflow (mapping %v)", l.Address, offset, l.Mapping)
}
a = append(a, fmt.Sprintf("%#x", addr))
}
Expand Down Expand Up @@ -144,15 +144,15 @@ func symbolizeMapping(source string, offset int64, syms func(string, string) ([]
}

if symbol := symbolzRE.FindStringSubmatch(l); len(symbol) == 3 {
addr, err := strconv.ParseInt(symbol[1], 0, 64)
origAddr, err := strconv.ParseUint(symbol[1], 0, 64)
if err != nil {
return fmt.Errorf("unexpected parse failure %s: %v", symbol[1], err)
}
if addr < 0 {
return fmt.Errorf("unexpected negative adjusted address, source %s, offset %d", symbol[1], offset)
}
// Reapply offset expected by the profile.
addr -= offset
addr, overflow := adjust(origAddr, -offset)
if overflow {
return fmt.Errorf("cannot adjust symbolz address %d by %d, it would overflow", origAddr, -offset)
}

name := symbol[2]
fn := functions[name]
Expand All @@ -166,7 +166,7 @@ func symbolizeMapping(source string, offset int64, syms func(string, string) ([]
p.Function = append(p.Function, fn)
}

lines[uint64(addr)] = profile.Line{Function: fn}
lines[addr] = profile.Line{Function: fn}
}
}

Expand All @@ -181,3 +181,20 @@ func symbolizeMapping(source string, offset int64, syms func(string, string) ([]

return nil
}

// adjust shifts the specified address by the signed offset. It returns the
// adjusted address. It signals that the address cannot be adjusted without an
// overflow by returning true in the second return value.
func adjust(addr uint64, offset int64) (uint64, bool) {
adj := uint64(int64(addr) + offset)
if offset < 0 {
if adj >= addr {
return 0, true
}
} else {
if adj < addr {
return 0, true
}
}
return adj, false
}
28 changes: 28 additions & 0 deletions internal/symbolz/symbolz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package symbolz

import (
"fmt"
"math"
"strings"
"testing"

Expand Down Expand Up @@ -139,3 +140,30 @@ func fetchSymbols(source, post string) ([]byte, error) {
}
return []byte(symbolz), nil
}

func TestAdjust(t *testing.T) {
for _, tc := range []struct {
addr uint64
offset int64
wantAdj uint64
wantOverflow bool
}{{math.MaxUint64, 0, math.MaxUint64, false},
{math.MaxUint64, 1, 0, true},
{math.MaxUint64 - 1, 1, math.MaxUint64, false},
{math.MaxUint64 - 1, 2, 0, true},
{math.MaxInt64 + 1, math.MaxInt64, math.MaxUint64, false},
{0, 0, 0, false},
{0, -1, 0, true},
{1, -1, 0, false},
{2, -1, 1, false},
{2, -2, 0, false},
{2, -3, 0, true},
{-math.MinInt64, math.MinInt64, 0, false},
{-math.MinInt64 + 1, math.MinInt64, 1, false},
{-math.MinInt64 - 1, math.MinInt64, 0, true},
} {
if adj, overflow := adjust(tc.addr, tc.offset); adj != tc.wantAdj || overflow != tc.wantOverflow {
t.Errorf("adjust(%d, %d) = (%d, %t), want (%d, %t)", tc.addr, tc.offset, adj, overflow, tc.wantAdj, tc.wantOverflow)
}
}
}

0 comments on commit 2c569c7

Please sign in to comment.