Skip to content

Commit 6a90446

Browse files
committed
Address review feedback: extract findJITRegion, add tests, fix Detach cleanup
- Extract JIT region detection into standalone findJITRegion() function that handles both prctl-labeled mappings (spanning full reserved area including holes) and heuristic fallback (first anonymous executable mapping) - Add comprehensive unit tests for JIT region detection: no mappings, file-backed only, labeled single/multiple, heuristic fallback, precedence - Fix Detach() to clean up PidInterpreterMapping prefixes (previously only deleted proc data, leaking eBPF map entries) - Remove dead code: m.Vaddr < jitMapping.Vaddr branch that could never be true since /proc/pid/maps is sorted by address
1 parent 2ad4802 commit 6a90446

2 files changed

Lines changed: 196 additions & 21 deletions

File tree

interpreter/ruby/ruby.go

Lines changed: 68 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,18 @@ type rubyInstance struct {
455455
}
456456

457457
func (r *rubyInstance) Detach(ebpf interpreter.EbpfHandler, pid libpf.PID) error {
458-
return ebpf.DeleteProcData(libpf.Ruby, pid)
458+
var err error
459+
err = ebpf.DeleteProcData(libpf.Ruby, pid)
460+
461+
for prefix := range r.prefixes {
462+
if err2 := ebpf.DeletePidInterpreterMapping(pid, prefix); err2 != nil {
463+
err = errors.Join(err,
464+
fmt.Errorf("failed to remove ruby prefix 0x%x/%d: %v",
465+
prefix.Key, prefix.Length, err2))
466+
}
467+
}
468+
469+
return err
459470
}
460471

461472
// UpdateLibcInfo is called when libc introspection data becomes available.
@@ -1268,12 +1279,59 @@ func profileFrameFullLabel(classPath, label, baseLabel, methodName libpf.String,
12681279
return libpf.Intern(profileLabel)
12691280
}
12701281

1282+
// findJITRegion detects the YJIT JIT code region from process memory mappings.
1283+
// YJIT reserves a large contiguous address range (typically 48-128 MiB) via mmap
1284+
// with PROT_NONE and then mprotects individual 16k codepages to r-x as needed.
1285+
// On systems with CONFIG_ANON_VMA_NAME, Ruby labels the region via prctl(PR_SET_VMA)
1286+
// giving it a path like "[anon:Ruby:rb_yjit_reserve_addr_space]".
1287+
// On systems without that config, we fall back to a heuristic: the first anonymous
1288+
// executable mapping (by address) is assumed to be the JIT region since YJIT
1289+
// initializes before any gems could create anonymous executable mappings.
1290+
// Returns (start, end, found).
1291+
func findJITRegion(mappings []process.RawMapping) (uint64, uint64, bool) {
1292+
var jitStart, jitEnd uint64
1293+
labelFound := false
1294+
var heuristicStart, heuristicEnd uint64
1295+
heuristicFound := false
1296+
1297+
for idx := range mappings {
1298+
m := &mappings[idx]
1299+
1300+
// Check for prctl-labeled JIT region. These mappings may be ---p (PROT_NONE)
1301+
// or r-xp depending on whether YJIT has activated codepages in this region.
1302+
if strings.Contains(m.Path, "jit_reserve_addr_space") {
1303+
if !labelFound || m.Vaddr < jitStart {
1304+
jitStart = m.Vaddr
1305+
}
1306+
if !labelFound || m.Vaddr+m.Length > jitEnd {
1307+
jitEnd = m.Vaddr + m.Length
1308+
}
1309+
labelFound = true
1310+
continue
1311+
}
1312+
1313+
// Heuristic fallback: first anonymous executable mapping by address.
1314+
// Mappings from /proc/pid/maps are sorted by address, so the first
1315+
// match is the lowest address.
1316+
if !heuristicFound && m.IsExecutable() && m.IsAnonymous() {
1317+
heuristicStart = m.Vaddr
1318+
heuristicEnd = m.Vaddr + m.Length
1319+
heuristicFound = true
1320+
}
1321+
}
1322+
1323+
if labelFound {
1324+
return jitStart, jitEnd, true
1325+
}
1326+
if heuristicFound {
1327+
return heuristicStart, heuristicEnd, true
1328+
}
1329+
return 0, 0, false
1330+
}
1331+
12711332
func (r *rubyInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler,
12721333
_ reporter.ExecutableReporter, pr process.Process, mappings []process.RawMapping) error {
1273-
var jitMapping *process.RawMapping
1274-
12751334
pid := pr.PID()
1276-
jitFound := false
12771335
r.mappingGeneration++
12781336

12791337
log.Debugf("Synchronizing ruby mappings")
@@ -1283,19 +1341,6 @@ func (r *rubyInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler,
12831341
if !m.IsExecutable() || !m.IsAnonymous() {
12841342
continue
12851343
}
1286-
// If prctl is allowed, ruby should label the memory region
1287-
// always prefer that
1288-
if strings.Contains(m.Path, "jit_reserve_addr_space") {
1289-
jitMapping = m
1290-
jitFound = true
1291-
}
1292-
// Use the first executable anon region we find if it isn't labeled
1293-
// If we find more, prefer ones earlier in memory or larger in size
1294-
if !jitFound && (jitMapping == nil || m.Vaddr < jitMapping.Vaddr || m.Length > jitMapping.Length) {
1295-
// Don't set jitFound here as it is a heuristic, we aren't sure
1296-
// could be on a system without linux config flag to allow prctl to label memoy
1297-
jitMapping = m
1298-
}
12991344

13001345
if _, exists := r.mappings[*m]; exists {
13011346
*r.mappings[*m] = r.mappingGeneration
@@ -1326,13 +1371,15 @@ func (r *rubyInstance) SynchronizeMappings(ebpf interpreter.EbpfHandler,
13261371
r.prefixes[prefix] = &mappingGeneration
13271372
}
13281373
}
1329-
if jitMapping != nil && (r.procInfo.Jit_start != jitMapping.Vaddr || r.procInfo.Jit_end != jitMapping.Vaddr+jitMapping.Length) {
1330-
r.procInfo.Jit_start = jitMapping.Vaddr
1331-
r.procInfo.Jit_end = jitMapping.Vaddr + jitMapping.Length
1374+
// Detect JIT region from all mappings and update proc data if changed.
1375+
jitStart, jitEnd, jitFound := findJITRegion(mappings)
1376+
if jitFound && (r.procInfo.Jit_start != jitStart || r.procInfo.Jit_end != jitEnd) {
1377+
r.procInfo.Jit_start = jitStart
1378+
r.procInfo.Jit_end = jitEnd
13321379
if err := ebpf.UpdateProcData(libpf.Ruby, pr.PID(), unsafe.Pointer(r.procInfo)); err != nil {
13331380
return err
13341381
}
1335-
log.Debugf("Added jit mapping %08x ruby proc info, %08x", r.procInfo.Jit_start, r.procInfo.Jit_end)
1382+
log.Debugf("Updated JIT region %#x-%#x in ruby proc info", jitStart, jitEnd)
13361383
}
13371384
// Remove prefixes not seen
13381385
for prefix, generationPtr := range r.prefixes {

interpreter/ruby/ruby_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
package ruby // import "go.opentelemetry.io/ebpf-profiler/interpreter/ruby"
55

66
import (
7+
"debug/elf"
78
"testing"
89

910
"go.opentelemetry.io/ebpf-profiler/libpf"
11+
"go.opentelemetry.io/ebpf-profiler/process"
1012

1113
"github.com/stretchr/testify/assert"
1214
)
@@ -234,3 +236,129 @@ func TestProfileFrameFullLabel(t *testing.T) {
234236
})
235237
}
236238
}
239+
240+
func TestFindJITRegion(t *testing.T) {
241+
execAnon := func(vaddr, length uint64) process.RawMapping {
242+
return process.RawMapping{
243+
Vaddr: vaddr,
244+
Length: length,
245+
Flags: elf.PF_R | elf.PF_X,
246+
Path: "",
247+
}
248+
}
249+
labeled := func(vaddr, length uint64) process.RawMapping {
250+
return process.RawMapping{
251+
Vaddr: vaddr,
252+
Length: length,
253+
Flags: 0, // ---p (PROT_NONE)
254+
Path: "[anon:Ruby:rb_yjit_reserve_addr_space]",
255+
}
256+
}
257+
fileBacked := func(vaddr, length uint64, path string) process.RawMapping {
258+
return process.RawMapping{
259+
Vaddr: vaddr,
260+
Length: length,
261+
Flags: elf.PF_R | elf.PF_X,
262+
Path: path,
263+
}
264+
}
265+
266+
tests := []struct {
267+
name string
268+
mappings []process.RawMapping
269+
wantStart uint64
270+
wantEnd uint64
271+
wantFound bool
272+
}{
273+
{
274+
name: "no mappings",
275+
mappings: nil,
276+
wantFound: false,
277+
},
278+
{
279+
name: "only file-backed mappings",
280+
mappings: []process.RawMapping{
281+
fileBacked(0x400000, 0x1000, "/usr/bin/ruby"),
282+
fileBacked(0x7f0000, 0x2000, "/lib/libc.so.6"),
283+
},
284+
wantFound: false,
285+
},
286+
{
287+
name: "labeled JIT region (single mapping)",
288+
mappings: []process.RawMapping{
289+
fileBacked(0x400000, 0x1000, "/usr/bin/ruby"),
290+
labeled(0x7f17d99b9000, 0x8000000),
291+
},
292+
wantStart: 0x7f17d99b9000,
293+
wantEnd: 0x7f17d99b9000 + 0x8000000,
294+
wantFound: true,
295+
},
296+
{
297+
name: "labeled JIT region with holes (multiple contiguous mappings)",
298+
mappings: []process.RawMapping{
299+
fileBacked(0x400000, 0x1000, "/usr/bin/ruby"),
300+
{
301+
Vaddr: 0x7f17d99b9000,
302+
Length: 0x15f000,
303+
Flags: elf.PF_R | elf.PF_X,
304+
Path: "[anon:Ruby:rb_yjit_reserve_addr_space]",
305+
},
306+
{
307+
Vaddr: 0x7f17d9b18000,
308+
Length: 0x119000,
309+
Flags: elf.PF_R | elf.PF_X,
310+
Path: "[anon:Ruby:rb_yjit_reserve_addr_space]",
311+
},
312+
{
313+
Vaddr: 0x7f17d9c31000,
314+
Length: 0x7d88000,
315+
Flags: 0, // ---p reserved
316+
Path: "[anon:Ruby:rb_yjit_reserve_addr_space]",
317+
},
318+
},
319+
wantStart: 0x7f17d99b9000,
320+
wantEnd: 0x7f17d9c31000 + 0x7d88000,
321+
wantFound: true,
322+
},
323+
{
324+
name: "heuristic fallback - first anonymous executable mapping",
325+
mappings: []process.RawMapping{
326+
fileBacked(0x400000, 0x1000, "/usr/bin/ruby"),
327+
execAnon(0x7f0000100000, 0x4000),
328+
execAnon(0x7f0000200000, 0x8000),
329+
},
330+
wantStart: 0x7f0000100000,
331+
wantEnd: 0x7f0000100000 + 0x4000,
332+
wantFound: true,
333+
},
334+
{
335+
name: "labeled takes precedence over heuristic",
336+
mappings: []process.RawMapping{
337+
execAnon(0x1000000, 0x4000),
338+
labeled(0x7f0000000000, 0x3000000),
339+
},
340+
wantStart: 0x7f0000000000,
341+
wantEnd: 0x7f0000000000 + 0x3000000,
342+
wantFound: true,
343+
},
344+
}
345+
346+
for _, tt := range tests {
347+
t.Run(tt.name, func(t *testing.T) {
348+
start, end, found := findJITRegion(tt.mappings)
349+
if found != tt.wantFound {
350+
t.Errorf("found = %v, want %v", found, tt.wantFound)
351+
return
352+
}
353+
if !found {
354+
return
355+
}
356+
if start != tt.wantStart {
357+
t.Errorf("start = %#x, want %#x", start, tt.wantStart)
358+
}
359+
if end != tt.wantEnd {
360+
t.Errorf("end = %#x, want %#x", end, tt.wantEnd)
361+
}
362+
})
363+
}
364+
}

0 commit comments

Comments
 (0)