pkujhd / goloader

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

实现 MarshalJSON 接口方法, 不会被执行 #74

Closed fumeboy closed 1 year ago

fumeboy commented 1 year ago

go1.19 AMD64

package a

import "encoding/json"

type impl struct {
    a int
}

func (impl) MarshalJSON() ([]byte, error) {
    println(11)
    return []byte("1"), nil
}

func main() (output int) {
    i := impl{}
    b, _ := json.Marshal(&i)
    println(string(b))
    return len(b)
}

loader 执行将输出 “{}” 而不是 "1"

eh-steve commented 1 year ago

This doesn't seem to happen on my fork https://github.com/eh-steve/goloader/pull/6

fumeboy commented 1 year ago

This doesn't seem to happen on my fork eh-steve#6

This problem seems to happen because reflect.Func not be handled at typelinkinit.

fumeboy commented 1 year ago

you are right, your repo handle the reflect.Func at typelinkinit so the RELOCATE could get address of function type in firstmoduledata's DATA segment.

but the function type was visited by R_ADDROFF, and both your and this repo not deal the offset > 0x7FFFFFFF || offset < -0x80000000 case for R_ADDROFF

eh-steve commented 1 year ago

but the function type was visited by R_ADDROFF, and both your and this repo not deal the offset > 0x7FFFFFFF || offset < -0x80000000 case for R_ADDROFF

In my fork thanks to mmap_manager, this isn't possible as code is always laid out within 32-bits of the firstmodule for all platforms/architectures

fumeboy commented 1 year ago

@eh-steve About mmap, perhaps we can simplify the manager by pre-allocating a sufficiently large memory block when the process starts, such as 128MB. (a simple JIT module may only occupy 1KB)

eh-steve commented 1 year ago

About mmap, perhaps we can simplify the manager by pre-allocating a sufficiently large memory block when the process starts, such as 128MB. (a simple JIT module may only occupy 1KB)

This wouldn't simplify it very much as you still need to check all existing process mappings to ensure the segment you're claiming isn't used by something else

eh-steve commented 1 year ago

@pkujhd Your implementation in https://github.com/pkujhd/goloader/commit/58bb448b91e1749565836879da8a95a4fd8b40ff still doesn’t produce the correct symbol names or correctly recurse over all the available types.

See the difference in registered symbol names between your implementation and mine for the same binary:

eh-steve_registerType.txt pkujhd_registerType.txt

pkujhd commented 1 year ago

@eh-steve

I know the replaced pkgpath on go.mod is not dealed on this version.

the diffent symbols in txts are only your loader and goloader pacakge import other packages

eh-steve commented 1 year ago

the diffent symbols in txts are only your loader and goloader pacakge import other packages

No I copied and pasted your implementation of registerType into my master branch and printed both the symbols registered by mine and yours for the same binary (I didn't check out your master branch).

There are lots of differences, mainly around fully qualified symbol package naming, anonymous types and invalid kinds from the use of Elem(). You can review the 2 lists to see the differences.

pkujhd commented 1 year ago

the diffent symbols in txts are only your loader and goloader pacakge import other packages

No I copied and pasted your implementation of registerType into my master branch and printed both the symbols registered by mine and yours for the same binary (I didn't check out your master branch).

There are lots of differences, mainly around fully qualified symbol package naming, anonymous types and invalid kinds from the use of Elem(). You can review the 2 lists to see the differences.

on your gived pkujhd_register.txt, the symbols type.sync.RWMutex not found, but i get this symbol run with my master branch. Maybe some operators have errors.

eh-steve commented 1 year ago

Apologies I think I uploaded the wrong file, these should be the right ones: eh-steve_registerType.txt pkujhd_registerType.txt

pkujhd commented 1 year ago

@eh-steve , It doesn't seem much different, except for pkgpath

If I have a good idea for pkgpath, I will fix it

I think the compiler of Golang should have already completed the issue of pkgpath but the part of the compiler code has not been fully read yet. I think it is not necessary to complete the corresponding logic in the goloader package again

eh-steve commented 1 year ago

https://www.diffchecker.com/0tAMwcaI/

eh-steve commented 1 year ago

If I have a good idea for pkgpath, I will fix it

I think the compiler of Golang should have already completed the issue of pkgpath but the part of the compiler code has not been fully read yet. I think it is not necessary to complete the corresponding logic in the goloader package again

Annoyingly the go compiler never has to translate a runtime type name to a fully qualified symbol name using only the pkgpath and the rtype name - the compiler builds the correct symbol names using type information from the typecheck/IR, which we don’t have access to at runtime…

pkujhd commented 1 year ago

If I have a good idea for pkgpath, I will fix it I think the compiler of Golang should have already completed the issue of pkgpath but the part of the compiler code has not been fully read yet. I think it is not necessary to complete the corresponding logic in the goloader package again

Annoyingly the go compiler never has to translate a runtime type name to a fully qualified symbol name using only the pkgpath and the rtype name - the compiler builds the correct symbol names using type information from the typecheck/IR, which we don’t have access to at runtime…

Thank you for informations, i read source code about type name, we need rewrite type name as you said

eh-steve commented 1 year ago

I think my implementation is correct as I’ve included a panic if the loader’s type is identical but the symbol name doesn’t match, and stopped seeing panics eventually