Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

debug/gosym: data race in PCToLine #47044

Open
TcM1911 opened this issue Jul 2, 2021 · 2 comments
Open

debug/gosym: data race in PCToLine #47044

TcM1911 opened this issue Jul 2, 2021 · 2 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@TcM1911
Copy link

TcM1911 commented Jul 2, 2021 •
edited

What version of Go are you using (go version)?

$ go version
1.16.5

Does this issue reproduce with the latest release?

I have not tested 1.17 beta but the issue in the code is still present in the main branch.

What did you do?

The LineTable attribute strings is not guarded against concurent reads and writes nor is it documented. If the same *sym.Table is used by multiple go routines that calls the method PCToLin the process may crash due to a race condition.

Call flow: PCToLin -> go12PCToFile -> stringFrom -> read and write to map without mutex.

It looks like the same can happen to the other map funcNames.

Part of panic:

fatal error: concurrent map read and map write

goroutine 16 [running]:
runtime.throw(0x5a3bb9, 0x21)
	runtime/panic.go:1117 +0x72 fp=0xc0006d8c08 sp=0xc0006d8bd8 pc=0x434bd2
runtime.mapaccess2_fast32(0x56e8a0, 0xc0000faae0, 0x32, 0x3006cd968, 0x402364)
	runtime/map_fast32.go:61 +0x1ac fp=0xc0006d8c30 sp=0xc0006d8c08 pc=0x4115ec
debug/gosym.(*LineTable).stringFrom(0xc00007a000, 0xc0005d2960, 0xf74a0, 0xf74a0, 0x32, 0x4021b0, 0x48)
	debug/gosym/pclntab.go:344 +0x4a fp=0xc0006d8c80 sp=0xc0006d8c30 pc=0x4fc5aa
debug/gosym.(*LineTable).go12PCToFile(0xc00007a000, 0x4021b4, 0x0, 0x0)
	debug/gosym/pclntab.go:483 +0x371 fp=0xc0006d8d00 sp=0xc0006d8c80 pc=0x4fcf11
debug/gosym.(*Table).PCToLine(0xc0000be180, 0x4021b4, 0x0, 0x1, 0x48, 0xc0001803c0)
	debug/gosym/symtab.go:510 +0xc5 fp=0xc0006d8d50 sp=0xc0006d8d00 pc=0x4ff645

What did you expect to see?

No panic

What did you see instead?

Panic
@smasher164 smasher164 changed the title debug/gosym LineTable race conditions -> Panic debug/gosym: race in LineTable.strings Jul 3, 2021
@smasher164 smasher164 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 3, 2021
@smasher164
Copy link
Member

Can you attach a program that reproduces this?

/cc @jeremyfaller @josharian

@smasher164 smasher164 changed the title debug/gosym: race in LineTable.strings debug/gosym: data race in PCToLine Jul 3, 2021
@TcM1911
Copy link
Author

TcM1911 commented Jul 3, 2021

Here is the location in my project where it is triggered. It's a library for analyzing Go binaries (I use it for malware analysis) so a single program isn't enough. This file triggers it consistantly. If I analyze the binary using this code logic it panics (a tool using it):

% redress -src 03fcfe56348cc1ff272e340ea1f401feee1f4f4b9446fb05d65e9d58ecadbc78 
fatal error: concurrent map read and map write

goroutine 12 [running]:
runtime.throw(0x11a367d, 0x21)
	runtime/panic.go:1117 +0x72 fp=0xc000038408 sp=0xc0000383d8 pc=0x1034452
runtime.mapaccess2_fast32(0x116e740, 0xc0000f8bd0, 0x32, 0x3012cf598, 0x402364)
	runtime/map_fast32.go:61 +0x1ac fp=0xc000038430 sp=0xc000038408 pc=0x101144c
debug/gosym.(*LineTable).stringFrom(0xc000078000, 0xc00055c960, 0xf74a0, 0xf74a0, 0x32, 0x4020d4, 0x4f)
	debug/gosym/pclntab.go:344 +0x4a fp=0xc000038480 sp=0xc000038430 pc=0x10fceca
debug/gosym.(*LineTable).go12PCToFile(0xc000078000, 0x4020d8, 0x0, 0x0)
	debug/gosym/pclntab.go:483 +0x371 fp=0xc000038500 sp=0xc000038480 pc=0x10fd831
debug/gosym.(*Table).PCToLine(0xc0000bc180, 0x4020d8, 0x0, 0x1, 0x4f, 0xc00010a3c0)
	debug/gosym/symtab.go:510 +0xc5 fp=0xc000038550 sp=0xc000038500 pc=0x10fff65
github.com/goretk/gore.findSourceLines(0x401d40, 0x402370, 0xc0000bc180, 0x1, 0x12)
	github.com/goretk/gore@v0.9.0-alpha3/file.go:188 +0xa9 fp=0xc0000385c0 sp=0xc000038550 pc=0x1131969
github.com/goretk/gore.(*GoFile).enumPackages.func1(0xc000016540, 0xc00008c180, 0xc00008c1e0, 0xc00008c240)
	github.com/goretk/gore@v0.9.0-alpha3/file.go:242 +0x145 fp=0xc0000387c0 sp=0xc0000385c0 pc=0x1141ca5
....

If I use a different version that use an older logic that doesn't use multiple go routines, it's fine:

% redress-legacy -src 03fcfe56348cc1ff272e340ea1f401feee1f4f4b9446fb05d65e9d58ecadbc78
Package github.com/GeertJohan/go%2erice: .
File: .	
	findBox Lines: 28 to 136 (108)	
	init Lines: 30 to 30 (0)	
	(*File)Close Lines: 30 to 124 (94)	
	init0 Lines: 32 to 125 (93)	
	(*virtualFile)read Lines: 82 to 117 (35)	
	glob.func1 Lines: 117 to 129 (12)	
	(*File)Read Lines: 124 to 153 (29)	
	(*appendedDirInfo)Name Lines: 125 to 128 (3)	
	(*appendedDirInfo)Size Lines: 128 to 131 (3)	
	(*appendedDirInfo)Mode Lines: 131 to 137 (6)	
	(*Box)resolveAbsolutePathFromCaller Lines: 136 to 146 (10)	
	(*appendedDirInfo)IsDir Lines: 137 to 138 (1)	
	(*Box)resolveAbsolutePathFromWorkingDirectory Lines: 146 to 182 (36)	
	(*File)Seek Lines: 153 to 170 (17)	
	(*Box)Open Lines: 182 to 281 (99)	
	(*Box)Bytes Lines: 281 to 284 (3)
....

One thing that I can tell is that the file path for all files appears to have been patched to "." which may be why it's crashing consistantly vs randomly.

Here is how it normaly should look like:

% redress -src /usr/local/bin/gofmt
Package main: /usr/local/Cellar/go/1.16.5/libexec/src/cmd/gofmt
File: <autogenerated>	
	(*simplifier)Visit Lines: 1 to 1 (0)	
File: gofmt.go	
	init Lines: 29 to 53 (24)	
	usage Lines: 64 to 76 (12)	
	isGoFile Lines: 76 to 83 (7)	
	processFile Lines: 83 to 166 (83)	
	visitFile Lines: 166 to 182 (16)	
	main Lines: 182 to 190 (8)	
	gofmtMain Lines: 190 to 236 (46)	
	diffWithReplaceTempFile Lines: 236 to 253 (17)	
	replaceTempFilename Lines: 253 to 278 (25)	
	backupFile Lines: 278 to 300 (22)	
File: internal.go	
	parse Lines: 23 to 94 (71)	
	parsefunc1 Lines: 45 to 69 (24)	
	parsefunc2 Lines: 69 to 80 (11)	
	format Lines: 94 to 175 (81)	
File: rewrite.go	
	initRewrite Lines: 19 to 38 (19)	
	initRewritefunc1 Lines: 31 to 64 (33)	
	parseExpr Lines: 38 to 57 (19)	
	rewriteFile Lines: 57 to 85 (28)	
	rewriteFilefunc1 Lines: 64 to 90 (26)	
	set Lines: 85 to 117 (32)	
	setfunc1 Lines: 90 to 97 (7)	
	apply Lines: 117 to 152 (35)	
	isWildcard Lines: 152 to 160 (8)	
	match Lines: 160 to 248 (88)	
	subst Lines: 248 to 308 (60)	
File: simplify.go	
	simplifierVisit Lines: 15 to 130 (115)	
	simplifiersimplifyLiteral Lines: 102 to 133 (31)	
	simplify Lines: 133 to 141 (8)	
	removeEmptyDeclGroups Lines: 141 to 152 (11)	
	isEmpty Lines: 152 to 164 (12)

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
3 participants
@TcM1911 @seankhliao @smasher164