Browse Source

Handle 64-bit negative addresses when adjusting them. (#397)

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.
Alexey Alexandrov 6 years ago
parent
commit
2c569c7227
2 changed files with 54 additions and 9 deletions
  1. 26
    9
      internal/symbolz/symbolz.go
  2. 28
    0
      internal/symbolz/symbolz_test.go

+ 26
- 9
internal/symbolz/symbolz.go View File

111
 	for _, l := range p.Location {
111
 	for _, l := range p.Location {
112
 		if l.Mapping == m && l.Address != 0 && len(l.Line) == 0 {
112
 		if l.Mapping == m && l.Address != 0 && len(l.Line) == 0 {
113
 			// Compensate for normalization.
113
 			// Compensate for normalization.
114
-			addr := int64(l.Address) + offset
115
-			if addr < 0 {
116
-				return fmt.Errorf("unexpected negative adjusted address, mapping %v source %d, offset %d", l.Mapping, l.Address, offset)
114
+			addr, overflow := adjust(l.Address, offset)
115
+			if overflow {
116
+				return fmt.Errorf("cannot adjust address %d by %d, it would overflow (mapping %v)", l.Address, offset, l.Mapping)
117
 			}
117
 			}
118
 			a = append(a, fmt.Sprintf("%#x", addr))
118
 			a = append(a, fmt.Sprintf("%#x", addr))
119
 		}
119
 		}
144
 		}
144
 		}
145
 
145
 
146
 		if symbol := symbolzRE.FindStringSubmatch(l); len(symbol) == 3 {
146
 		if symbol := symbolzRE.FindStringSubmatch(l); len(symbol) == 3 {
147
-			addr, err := strconv.ParseInt(symbol[1], 0, 64)
147
+			origAddr, err := strconv.ParseUint(symbol[1], 0, 64)
148
 			if err != nil {
148
 			if err != nil {
149
 				return fmt.Errorf("unexpected parse failure %s: %v", symbol[1], err)
149
 				return fmt.Errorf("unexpected parse failure %s: %v", symbol[1], err)
150
 			}
150
 			}
151
-			if addr < 0 {
152
-				return fmt.Errorf("unexpected negative adjusted address, source %s, offset %d", symbol[1], offset)
153
-			}
154
 			// Reapply offset expected by the profile.
151
 			// Reapply offset expected by the profile.
155
-			addr -= offset
152
+			addr, overflow := adjust(origAddr, -offset)
153
+			if overflow {
154
+				return fmt.Errorf("cannot adjust symbolz address %d by %d, it would overflow", origAddr, -offset)
155
+			}
156
 
156
 
157
 			name := symbol[2]
157
 			name := symbol[2]
158
 			fn := functions[name]
158
 			fn := functions[name]
166
 				p.Function = append(p.Function, fn)
166
 				p.Function = append(p.Function, fn)
167
 			}
167
 			}
168
 
168
 
169
-			lines[uint64(addr)] = profile.Line{Function: fn}
169
+			lines[addr] = profile.Line{Function: fn}
170
 		}
170
 		}
171
 	}
171
 	}
172
 
172
 
181
 
181
 
182
 	return nil
182
 	return nil
183
 }
183
 }
184
+
185
+// adjust shifts the specified address by the signed offset. It returns the
186
+// adjusted address. It signals that the address cannot be adjusted without an
187
+// overflow by returning true in the second return value.
188
+func adjust(addr uint64, offset int64) (uint64, bool) {
189
+	adj := uint64(int64(addr) + offset)
190
+	if offset < 0 {
191
+		if adj >= addr {
192
+			return 0, true
193
+		}
194
+	} else {
195
+		if adj < addr {
196
+			return 0, true
197
+		}
198
+	}
199
+	return adj, false
200
+}

+ 28
- 0
internal/symbolz/symbolz_test.go View File

16
 
16
 
17
 import (
17
 import (
18
 	"fmt"
18
 	"fmt"
19
+	"math"
19
 	"strings"
20
 	"strings"
20
 	"testing"
21
 	"testing"
21
 
22
 
139
 	}
140
 	}
140
 	return []byte(symbolz), nil
141
 	return []byte(symbolz), nil
141
 }
142
 }
143
+
144
+func TestAdjust(t *testing.T) {
145
+	for _, tc := range []struct {
146
+		addr         uint64
147
+		offset       int64
148
+		wantAdj      uint64
149
+		wantOverflow bool
150
+	}{{math.MaxUint64, 0, math.MaxUint64, false},
151
+		{math.MaxUint64, 1, 0, true},
152
+		{math.MaxUint64 - 1, 1, math.MaxUint64, false},
153
+		{math.MaxUint64 - 1, 2, 0, true},
154
+		{math.MaxInt64 + 1, math.MaxInt64, math.MaxUint64, false},
155
+		{0, 0, 0, false},
156
+		{0, -1, 0, true},
157
+		{1, -1, 0, false},
158
+		{2, -1, 1, false},
159
+		{2, -2, 0, false},
160
+		{2, -3, 0, true},
161
+		{-math.MinInt64, math.MinInt64, 0, false},
162
+		{-math.MinInt64 + 1, math.MinInt64, 1, false},
163
+		{-math.MinInt64 - 1, math.MinInt64, 0, true},
164
+	} {
165
+		if adj, overflow := adjust(tc.addr, tc.offset); adj != tc.wantAdj || overflow != tc.wantOverflow {
166
+			t.Errorf("adjust(%d, %d) = (%d, %t), want (%d, %t)", tc.addr, tc.offset, adj, overflow, tc.wantAdj, tc.wantOverflow)
167
+		}
168
+	}
169
+}