pkujhd / goloader

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

Add support for deduplicating type descriptors by relocating code to point at main module's types #62

Closed eh-steve closed 2 years ago

eh-steve commented 2 years ago

Previously, compiled code which used type assertions or switches wouldn't always work as expected, since duplicate *_types would be present in both the main module and the object file, and type assertion uses pointer equality to determine whether types match (and the object code would reference the local types)

This PR relocates code which references identical type descriptors (according to their compiler generated type hash being identical and type definitions being identical according to runtime.typesEqual()) to point at the main module's *_types:

Example

func HandleBytes(input interface{}) {
    bytesData := input.([]byte)
}

might previously fail with interface conversion: interface {} is []uint8, not []uint8 (types from different scopes), even if the incoming input is a []byte.

Instead, in these cases you had to use reflection to (safely) extract the underlying concrete type from the interface, e.g.:

func HandleBytes(input interface{}) {
    rval := reflect.ValueOf(input)
    if rval.Type().Kind() == reflect.Slice && rval.Type().Elem().Kind() == reflect.Uint8 {
        bytesData := rval.Bytes()
    }
}

This PR fixed the broken behaviour so type assertion and type switches on duplicated types works as expected

pkujhd commented 2 years ago

@eh-steve, thanks for your pr, but some changes cannot adapt to the lower golang version, i will review it and merge it manually.

eh-steve commented 2 years ago

I'm happy to port the pclntab changes to the lower versions - I might be making some more changes once I get to the bottom of some other bugs

pkujhd commented 2 years ago

@eh-steve

When the loader adds a symbol (ld.go:AddSymbol), if the symbol already exists in loader, the loader will be use the symbol in loader, it will be avoid to duplicate type symbol, but it will be lead to far address for R_ADDR on windows platform. As the same time, i will deal it with issue #61 and this pr.

pkujhd commented 2 years ago

I will merge bug fix manually, thanks for your pr

eh-steve commented 2 years ago

Hi, your fixes don't actually cover everything that has been fixed in this PR - would you be up for discussing these changes further?

I might add some tests which demonstrate each fix

pkujhd commented 2 years ago

@eh-steve Some bugs I don't find testcases. Now, I am finding golang source code by import path and ASM function warpper, if you have testcases, please give me an issue or some code, thanks.

eh-steve commented 2 years ago

Hi @pkujhd,

I've opened a chunky PR which hopefully demonstrates the purpose of each commit (and adds a new JIT compiler package). The jit tests should be able to be run for each commit on the branch.

I would suggest we try to fix all the outstanding goloader bugs in that branch, then attempt to backport the changes for older Go versions rather than picking individual commits into master which aren't 100% working.