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

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

Closed
fumeboy opened this issue May 17, 2023 · 16 comments
Closed

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

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

Comments

@fumeboy
Copy link

fumeboy commented May 17, 2023

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 pushed a commit to eh-steve/goloader that referenced this issue May 17, 2023
@eh-steve
Copy link

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

@fumeboy
Copy link
Author

fumeboy commented May 17, 2023

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

fumeboy commented May 17, 2023

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

eh-steve commented May 17, 2023

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

fumeboy commented May 17, 2023

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

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

@pkujhd pkujhd added the bug Something isn't working label May 30, 2023
eh-steve pushed a commit to eh-steve/goloader that referenced this issue Jun 9, 2023
eh-steve pushed a commit to eh-steve/goloader that referenced this issue Jun 10, 2023
@pkujhd pkujhd closed this as completed in 58bb448 Jul 1, 2023
@eh-steve
Copy link

eh-steve commented Jul 1, 2023

@pkujhd Your implementation in 58bb448 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
Copy link
Owner

pkujhd commented Jul 1, 2023

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

eh-steve commented Jul 1, 2023

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

pkujhd commented Jul 1, 2023

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

eh-steve commented Jul 1, 2023

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

@pkujhd
Copy link
Owner

pkujhd commented Jul 1, 2023

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

eh-steve commented Jul 1, 2023

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

@eh-steve
Copy link

eh-steve commented Jul 1, 2023

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

pkujhd commented Jul 3, 2023

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

eh-steve commented Jul 3, 2023

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

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

3 participants