pkujhd / goloader

load and run golang code at runtime.
Apache License 2.0
497 stars 58 forks source link

there may be an omission of pcdata handling #91

Closed fumeboy closed 10 months ago

fumeboy commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

@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.