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

the handle of inline is incorrect #77

Closed
fumeboy opened this issue May 21, 2023 · 11 comments
Closed

the handle of inline is incorrect #77

fumeboy opened this issue May 21, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@fumeboy
Copy link

fumeboy commented May 21, 2023

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
Copy link
Owner

pkujhd commented May 22, 2023

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

@pkujhd
Copy link
Owner

pkujhd commented May 22, 2023

could you give me a testcase for it

@pkujhd pkujhd closed this as completed in 09f36c8 May 22, 2023
@fumeboy
Copy link
Author

fumeboy commented May 22, 2023

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

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

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

@pkujhd
Copy link
Owner

pkujhd commented May 22, 2023

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

@pkujhd
Copy link
Owner

pkujhd commented May 22, 2023

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

@pkujhd pkujhd added the bug Something isn't working label May 22, 2023
@fumeboy
Copy link
Author

fumeboy commented May 23, 2023

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

@pkujhd
Copy link
Owner

pkujhd commented May 23, 2023

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

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

@fumeboy
Copy link
Author

fumeboy commented May 25, 2023

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

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

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

@pkujhd
Copy link
Owner

pkujhd commented May 26, 2023

gentraceback

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

@fumeboy
Copy link
Author

fumeboy commented May 26, 2023

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

@pkujhd
Copy link
Owner

pkujhd commented May 26, 2023

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

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

eh-steve pushed a commit to eh-steve/goloader that referenced this issue May 31, 2023
eh-steve pushed a commit to eh-steve/goloader that referenced this issue Jun 10, 2023
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