pkujhd / goloader

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

the handle of inline is incorrect #77

Closed fumeboy closed 1 year ago

fumeboy commented 1 year ago

the current handle in this repo is incorrect, it fills the caller's funcID instead of the callee's to the InlinedCall.funcID

https://github.com/pkujhd/goloader/blob/master/inlinetree.go#L32

in go source code (go1.19.6), the funcID is got from other object file by linker.Package[SymRef.Pkg][SymRef.SymIndex].funcID

but we cannot do this because we are unable to obtain these original Go object files used when compiling at runtime.

I suggest to use the -l option of go tool compile to disable inline

go source code (go1.19.6)

        // genInlTreeSym generates the InlTree sym for a function with the
    // specified FuncInfo.
    func genInlTreeSym(ctxt *Link, cu *sym.CompilationUnit, fi loader.FuncInfo, arch *sys.Arch, nameOffsets map[loader.Sym]uint32) loader.Sym {
        ldr := ctxt.loader
        its := ldr.CreateExtSym("", 0)
        inlTreeSym := ldr.MakeSymbolUpdater(its)
        // Note: the generated symbol is given a type of sym.SGOFUNC, as a
        // signal to the symtab() phase that it needs to be grouped in with
        // other similar symbols (gcdata, etc); the dodata() phase will
        // eventually switch the type back to SRODATA.
        inlTreeSym.SetType(sym.SGOFUNC)
        ldr.SetAttrReachable(its, true)
        ldr.SetSymAlign(its, 4) // it has 32-bit fields
        ninl := fi.NumInlTree()
        for i := 0; i < int(ninl); i++ {
            call := fi.InlTree(i)
            val := call.File
            nameoff, ok := nameOffsets[call.Func]
            if !ok {
                panic("couldn't find function name offset")
            }

            inlTreeSym.SetUint16(arch, int64(i*20+0), uint16(call.Parent))
            inlFunc := ldr.FuncInfo(call.Func)

            var funcID objabi.FuncID
            if inlFunc.Valid() {
                funcID = inlFunc.FuncID()
            }
            inlTreeSym.SetUint8(arch, int64(i*20+2), uint8(funcID))

            // byte 3 is unused
            inlTreeSym.SetUint32(arch, int64(i*20+4), uint32(val))
            inlTreeSym.SetUint32(arch, int64(i*20+8), uint32(call.Line))
            inlTreeSym.SetUint32(arch, int64(i*20+12), uint32(nameoff))
            inlTreeSym.SetUint32(arch, int64(i*20+16), uint32(call.ParentPC))
        }
        return its
    }
pkujhd commented 1 year ago

@fumeboy, it is a bug, we could read funcId from objsymbolMap[inl.Func].Func.FuncID.

pkujhd commented 1 year ago

could you give me a testcase for it

fumeboy commented 1 year ago

@fumeboy, it is a bug, we could read funcId from objsymbolMap[inl.Func].Func.FuncID.

我的意思是, 比如我内联了 fmt 包的一个函数, 但是我没有 fmt 包的 object file, 怎么能根据 symidx 找到它是哪个符号, 并获取 funcID 呢

我也很惊讶这样似乎也能正常运行; 我目前也构造不了错误 case

pkujhd commented 1 year ago

是的,如果是外部包是无法得到信息的,所以如果查不到符号,用了默认值funcID_Normal

pkujhd commented 1 year ago

要处理的话,golang1.12-1.15可以直接去读firstmodule的_func查到funcID,因为较早版本的obj里存了名字(但是1.15的名字是空的,应该是即将使用新的obj结构引入的问题); 1.16以后,就需要把fmt包读进来,才能知道相应的id是的symbol的名字是啥,现在对于外部包resolveSymRef回来的名字都是空字符串。 所以关闭inline也是可选的,毕竟1.9才开始支持,之前都没有inline, 1.12才算比较好用

fumeboy commented 1 year ago

直接使用 FuncID_normal 没问题吗, 为什么呢

pkujhd commented 1 year ago

直接使用 FuncID_normal 没问题吗, 为什么呢

大部分情况下都是FuncID_normal ,实际上遇到的函数应该都是FuncID_normal 的, 而且这个inlineTree只在输出stack的时候用,对于执行应该没有太大的影响。 对于系统函数是错的,目前除了上面说的方案没有更好的方案.

fumeboy commented 1 year ago

直接使用 FuncID_normal 没问题吗, 为什么呢

大部分情况下都是FuncID_normal ,实际上遇到的函数应该都是FuncID_normal 的, 而且这个inlineTree只在输出stack的时候用,对于执行应该没有太大的影响。 对于系统函数是错的,目前除了上面说的方案没有更好的方案.

感觉并不是那么简单, 因为我看 inlineTree funcID 被 gentraceback 使用, gentraceback 应该和 GC 有关

pkujhd commented 1 year ago

gentraceback

是和栈相关,因为runtime.morestack, runtime.systemstack和cgocall会改变栈结构,所以需要特殊处理

fumeboy commented 1 year ago

看了下 funcID 的非 0 值对应的函数, 都不可能被内联, 那看来直接设置成 normal 是没问题的, 但既然如此为啥他们还要保留这个字段呢 ..

pkujhd commented 1 year ago

看了下 funcID 的非 0 值对应的函数, 都不可能被内联, 那看来直接设置成 normal 是没问题的, 但既然如此为啥他们还要保留这个字段呢 ..

这个玩意之前没有,1.12版本加进去的