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

there may be an omission of pcdata handling #91

Closed
fumeboy opened this issue Nov 13, 2023 · 2 comments
Closed

there may be an omission of pcdata handling #91

fumeboy opened this issue Nov 13, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@fumeboy
Copy link

fumeboy commented Nov 13, 2023

compare 2 code snippet below:

// goloader/ld.go
	for _, pcdata := range symbol.Func.PCData {
		Func.PCData = append(Func.PCData, uint32(len(linker.pclntable)))
		linker.pclntable = append(linker.pclntable, pcdata...)
	}
// GoSDK(1.19.6)/cmd/link/internal/ld/pcln.go
	saveOffset := func(pcSym loader.Sym) {
		if _, ok := seen[pcSym]; !ok {
			datSize := ldr.SymSize(pcSym)
			if datSize != 0 {
				ldr.SetSymValue(pcSym, size)
			} else {
				// Invalid PC data, record as zero.
				ldr.SetSymValue(pcSym, 0)
			}
			size += datSize
			seen[pcSym] = struct{}{}
		}
	}

and the diff is goloader have not handle the case of datSize == 0, i think the right solution should be this:

	for _, pcdata := range symbol.Func.PCData {
		if len(pcdata) == 0 { append( ,0); continue }
		Func.PCData = append(Func.PCData, uint32(len(linker.pclntable)))
		linker.pclntable = append(linker.pclntable, pcdata...)
	}
@fumeboy
Copy link
Author

fumeboy commented Nov 13, 2023

if the compiler disable inline, funcinfo may have an PCDATA[2] InlTreeIndex with empty data.

i found this by reviewing go source code, and the runtime haven’t encountered some error in my executions.

@pkujhd pkujhd added the bug Something isn't working label Nov 15, 2023
@pkujhd
Copy link
Owner

pkujhd commented Nov 15, 2023

@fumeboy , it is a bug, if a PCData is empty data, pcdata offset needs zero.
you haven't encountered some error because golang runtime checks funcdata before get pcdata.

@pkujhd pkujhd closed this as completed in ac97855 Nov 15, 2023
eh-steve pushed a commit to eh-steve/goloader that referenced this issue Dec 7, 2023
eh-steve pushed a commit to eh-steve/goloader that referenced this issue Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants