Skip to content

Commit 73fabd3

Browse files
rabbitstackrabbitstack
committed
perf(symbolizer): Query live modules once
Avoid repetitive module enumeration and handle potential race condition situations when operating on process modules.
1 parent dedd157 commit 73fabd3

File tree

3 files changed

+227
-21
lines changed

3 files changed

+227
-21
lines changed

‎pkg/ps/types/types_windows.go‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ type PS struct {
104104
// IsCreatedFromSystemLogger is the metadata attribute that indicates if the
105105
// process state is created from the event published by the NT kernel logger.
106106
IsCreatedFromSystemLogger bool `json:"-"`
107+
108+
// modules are process modules obtained by direct invocation to the API call.
109+
modules []sys.ProcessModule
110+
onceMods sync.Once
107111
}
108112

109113
// UUID is meant to offer a more robust version of process ID that
@@ -497,11 +501,15 @@ func (ps *PS) AddModule(mod Module) {
497501
if m != nil {
498502
return
499503
}
504+
ps.Lock()
505+
defer ps.Unlock()
500506
ps.Modules = append(ps.Modules, mod)
501507
}
502508

503509
// RemoveModule removes a specified module from this process state.
504510
func (ps *PS) RemoveModule(addr va.Address) {
511+
ps.Lock()
512+
defer ps.Unlock()
505513
for i, mod := range ps.Modules {
506514
if mod.BaseAddress == addr {
507515
ps.Modules = append(ps.Modules[:i], ps.Modules[i+1:]...)
@@ -512,6 +520,8 @@ func (ps *PS) RemoveModule(addr va.Address) {
512520

513521
// FindModule finds the module by name.
514522
func (ps *PS) FindModule(path string) *Module {
523+
ps.RLock()
524+
defer ps.RUnlock()
515525
for _, mod := range ps.Modules {
516526
if filepath.Base(mod.Name) == filepath.Base(path) {
517527
return &mod
@@ -522,6 +532,8 @@ func (ps *PS) FindModule(path string) *Module {
522532

523533
// FindModuleByAddr finds the module by its base address.
524534
func (ps *PS) FindModuleByAddr(addr va.Address) *Module {
535+
ps.RLock()
536+
defer ps.RUnlock()
525537
for _, mod := range ps.Modules {
526538
if mod.BaseAddress == addr {
527539
return &mod
@@ -530,11 +542,57 @@ func (ps *PS) FindModuleByAddr(addr va.Address) *Module {
530542
return nil
531543
}
532544

545+
var queryLiveModules = func(pid uint32) []sys.ProcessModule {
546+
return sys.EnumProcessModules(pid)
547+
}
548+
533549
// FindModuleByVa finds the module name by
534550
// probing the range of the given virtual address.
535551
func (ps *PS) FindModuleByVa(addr va.Address) *Module {
552+
mod := ps.findModuleByVa(addr)
553+
if mod != nil {
554+
return mod
555+
}
556+
557+
ps.onceMods.Do(func() {
558+
// query live process modules
559+
ps.modules = queryLiveModules(ps.PID)
560+
})
561+
562+
// try to find the module within the VA space
563+
// and if found, add it to process modules for
564+
// future lookups
565+
for _, m := range ps.modules {
566+
b := va.Address(m.BaseOfDll)
567+
size := uint64(m.SizeOfImage)
568+
569+
if addr < b || addr >= b.Inc(size) {
570+
continue
571+
}
572+
573+
mod := Module{
574+
Name: m.Name,
575+
BaseAddress: b,
576+
Size: size,
577+
DefaultBaseAddress: b,
578+
}
579+
580+
ps.Lock()
581+
ps.Modules = append(ps.Modules, mod)
582+
ps.Unlock()
583+
584+
return &mod
585+
}
586+
587+
return nil
588+
}
589+
590+
func (ps *PS) findModuleByVa(addr va.Address) *Module {
591+
ps.RLock()
592+
defer ps.RUnlock()
536593
for _, mod := range ps.Modules {
537-
if addr >= mod.BaseAddress && addr <= mod.BaseAddress.Inc(mod.Size) {
594+
end := mod.BaseAddress.Inc(mod.Size)
595+
if addr >= mod.BaseAddress && addr < end {
538596
return &mod
539597
}
540598
}

‎pkg/ps/types/types_windows_test.go‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,13 @@ package types
2020

2121
import (
2222
"os"
23+
"sync"
2324
"testing"
2425
"time"
2526

27+
"github.com/rabbitstack/fibratus/pkg/sys"
2628
"github.com/rabbitstack/fibratus/pkg/util/bootid"
29+
"github.com/rabbitstack/fibratus/pkg/util/va"
2730
"github.com/stretchr/testify/assert"
2831
"github.com/stretchr/testify/require"
2932
"golang.org/x/sys/windows"
@@ -130,3 +133,168 @@ func TestIsAppinfoSvc(t *testing.T) {
130133
})
131134
}
132135
}
136+
137+
func TestFindModuleByVa(t *testing.T) {
138+
base := va.Address(0x1000)
139+
140+
tests := []struct {
141+
name string
142+
initialModules []Module
143+
liveModules []sys.ProcessModule
144+
addr va.Address
145+
expectNil bool
146+
expectName string
147+
expectCachedAdd bool
148+
}{
149+
{
150+
name: "hit lower bound inclusive",
151+
initialModules: []Module{
152+
{
153+
Name: "C:\\Windows\\System32\\ntdll.dll",
154+
BaseAddress: base,
155+
Size: 0x200,
156+
},
157+
},
158+
addr: base,
159+
expectName: "C:\\Windows\\System32\\ntdll.dll",
160+
},
161+
{
162+
name: "hit upper bound exclusive",
163+
initialModules: []Module{
164+
{
165+
Name: "C:\\Windows\\System32\\ntdll.dll",
166+
BaseAddress: base,
167+
Size: 0x200,
168+
},
169+
},
170+
addr: base.Inc(0x200),
171+
expectNil: true,
172+
},
173+
{
174+
name: "address inside range",
175+
initialModules: []Module{
176+
{
177+
Name: "C:\\Windows\\System32\\ntdll.dll",
178+
BaseAddress: base,
179+
Size: 0x200,
180+
},
181+
{
182+
Name: "C:\\Windows\\System32\\kernel32.dll",
183+
BaseAddress: base.Inc(10),
184+
Size: 0x100,
185+
},
186+
},
187+
addr: base.Inc(0x100),
188+
expectName: "C:\\Windows\\System32\\ntdll.dll",
189+
},
190+
{
191+
name: "miss cached but hit live modules",
192+
liveModules: []sys.ProcessModule{
193+
{
194+
ModuleInfo: windows.ModuleInfo{
195+
BaseOfDll: 0x2000,
196+
SizeOfImage: 0x300,
197+
},
198+
Name: "C:\\Windows\\System32\\ntdll.dll",
199+
},
200+
},
201+
addr: va.Address(0x2100),
202+
expectName: "C:\\Windows\\System32\\ntdll.dll",
203+
expectCachedAdd: true,
204+
},
205+
{
206+
name: "miss both cached and live",
207+
initialModules: []Module{
208+
{
209+
Name: "C:\\Windows\\System32\\ntdll.dll",
210+
BaseAddress: base,
211+
Size: 0x200,
212+
},
213+
},
214+
liveModules: []sys.ProcessModule{
215+
{
216+
ModuleInfo: windows.ModuleInfo{
217+
BaseOfDll: 0x3000,
218+
SizeOfImage: 0x100,
219+
},
220+
Name: "C:\\Windows\\System32\\ntdll.dll",
221+
},
222+
},
223+
addr: va.Address(0x9999),
224+
expectNil: true,
225+
},
226+
{
227+
name: "multiple modules choose correct one",
228+
initialModules: []Module{
229+
{
230+
Name: "C:\\Windows\\System32\\ntdll.dll",
231+
BaseAddress: va.Address(0x1000),
232+
Size: 0x100,
233+
},
234+
{
235+
Name: "C:\\Windows\\System32\\kernel32.dll",
236+
BaseAddress: va.Address(0x2000),
237+
Size: 0x200,
238+
},
239+
},
240+
addr: va.Address(0x2100),
241+
expectName: "C:\\Windows\\System32\\kernel32.dll",
242+
},
243+
}
244+
245+
for _, tt := range tests {
246+
t.Run(tt.name, func(t *testing.T) {
247+
248+
ps := &PS{
249+
PID: 1234,
250+
Modules: append([]Module{}, tt.initialModules...),
251+
}
252+
253+
ps.onceMods = sync.Once{}
254+
255+
queryLiveModules = func(_ uint32) []sys.ProcessModule {
256+
var mods []sys.ProcessModule
257+
for _, m := range tt.liveModules {
258+
mods = append(mods, sys.ProcessModule{
259+
ModuleInfo: windows.ModuleInfo{
260+
BaseOfDll: m.BaseOfDll,
261+
SizeOfImage: m.SizeOfImage,
262+
},
263+
Name: m.Name,
264+
})
265+
}
266+
return mods
267+
}
268+
269+
mod := ps.FindModuleByVa(tt.addr)
270+
271+
if tt.expectNil {
272+
if mod != nil {
273+
t.Fatalf("expected nil, got %+v", mod)
274+
}
275+
return
276+
}
277+
278+
if mod == nil {
279+
t.Fatalf("expected module %s, got nil", tt.expectName)
280+
}
281+
282+
if mod.Name != tt.expectName {
283+
t.Fatalf("expected module %s, got %s", tt.expectName, mod.Name)
284+
}
285+
286+
if tt.expectCachedAdd {
287+
found := false
288+
for _, m := range ps.Modules {
289+
if m.Name == tt.expectName {
290+
found = true
291+
break
292+
}
293+
}
294+
if !found {
295+
t.Fatalf("expected module to be cached")
296+
}
297+
}
298+
})
299+
}
300+
}

‎pkg/symbolize/symbolizer.go‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ var (
6363
// symModulesCount counts the number of loaded module exports
6464
symModulesCount = expvar.NewInt("symbolizer.modules.count")
6565

66-
// symEnumModulesHits counts the number of hits from enumerated modules
67-
symEnumModulesHits = expvar.NewInt("symbolizer.enum.modules.hits")
68-
6966
// debugHelpFallbacks counts how many times we Debug Help API was called
7067
// to resolve symbol information since we fail to do this from process
7168
// modules and PE export directory data
@@ -461,23 +458,6 @@ func (s *Symbolizer) produceFrame(addr va.Address, e *event.Event) callstack.Fra
461458
if mod == nil && ps.Parent != nil {
462459
mod = ps.Parent.FindModuleByVa(addr)
463460
}
464-
if mod == nil {
465-
// our last resort is to enumerate process modules
466-
modules := sys.EnumProcessModules(pid)
467-
for _, m := range modules {
468-
b := va.Address(m.BaseOfDll)
469-
size := uint64(m.SizeOfImage)
470-
if addr >= b && addr <= b.Inc(size) {
471-
mod = &pstypes.Module{
472-
Name: m.Name,
473-
BaseAddress: b,
474-
Size: size,
475-
}
476-
symEnumModulesHits.Add(1)
477-
break
478-
}
479-
}
480-
}
481461

482462
if mod != nil {
483463
frame.Module = mod.Name

0 commit comments

Comments
 (0)