refactor: move loadModuleData from runtimeTypeToDIE and expose the apis

[jayantxie]

No description provided.

[jayantxie]

@aarzilli Could you help me take a look at these changes to see if they are appropriate? If they are okay, I will add some necessary comments to the codes.

[aarzilli]

It seems premature to do this for something that isn't released.

[jayantxie]

I will later publish this repository under the organization cloudwego. However, currently, I can only specify it to use my personal forked repository using the go mod replace command. This prevents me from installing it using the "go install" command. Alternatively, could the go-delve/delve repository create a branch for me so that I can directly reference it? And after the features are released and stable for a while, we will merge this PR.

root:~/gopath/src/github.com/jayantxie# go install github.com/jayantxie/goref/cmd/grf@main
go: downloading github.com/jayantxie/goref v0.0.0-20240606095346-cfbff48b5720
go: github.com/jayantxie/goref/cmd/grf@main (in github.com/jayantxie/goref@v0.0.0-20240606095346-cfbff48b5720):
	The go.mod file for the module providing named packages contains one or
	more replace directives. It must not contain directives that would cause
	it to be interpreted differently than if it were the main module.

Do you think it's OK? @aarzilli

[aarzilli]

Anything in this struct can be calculated externally to delve using dwarf info, why export it?

[jayantxie]

Because parsing the closure type separately from the dwarf requires accessing private functions or variables of the Image type, such as getDwarfTree, dwarf, index, etc., it is not easy to obtain them.

[jayantxie]

@aarzilli Any suggestions? Decoding dwarf externally seems quite complex.

[aarzilli]

You can figure out which image it is by looking at Entry, the only other thing you need at that point is offset, which we can make public.

[aarzilli]

You can call this function through EvalExpression.

[aarzilli]

The same that goes for functionExtra goes for this. It can be implemented externally.

[jayantxie]

Functions like findType, expandPackagesInType, and findArrayType are all private functions, and it can be quite challenging to implement these functions externally.

[aarzilli]

The only call to FindTypeExpr in the repository you linked that I can find is this:

				t, err := parser.ParseExpr("runtime.specialfinalizer")
				if err != nil {
					return err
				}
				spty, _ := special.bi.FindTypeExpr(t)

You don't need FindTypeExpr for this, you can just use godwarf.ReadType.

[aarzilli]

This is something we don't want. We don't want to accidentally turn memory access into a linear search through a series of caches.

[aarzilli]

Do you really need this exported? The special thing about this is that it treats compositeMemory specially, but that's only something you will encounter for local variables.

[jayantxie]

You are right, I made a mistake before by caching local variables, which caused Dereference error. Now, I only cache heap objects, and there is indeed no need to reference them anymore.

[aarzilli]

I'd rather not export this at this point, I would suggest that you fish this out with unsafe and write a test for correctness.

[jayantxie]

Using unsafe to access fields in the short term is also an option. I'll handle it by this way for now.

[aarzilli]

I'm actually thinking that you shouldn't need this at all. The only time this is meaningfully different from the object you get from calling proc.(*Target).Memory is for stack variables, but really you shouldn't rely on those to find heap references: for one thing they might not be alive anymore and for another there could be (large) chunks of memory kept alive from references that are not reachable from any local variable that we have access to. Think for example the return value of a function that just returned.

[jayantxie]

The 'locals' variable is necessary for memory references. If the memory referenced by these variables becomes invalid and the corresponding heap memory cannot be found, my approach is to skip that variable. And when retrieving references, I also consider the 'args' as a 'locals' variable to prevent the second case.

[aarzilli]

You can't call Locals, it's too high level for what you are doing, you need to read the registers directly or you will not find some live memory. Also, to support range-over-func I will be making changes to Locals that will break it completely for your use case.

[aarzilli]

Judging from what you are using I think using pkg/dwarf/reader.Variables and then BinInfo.Location is more appropriate, if you want to go the route where you are using DWARF (which I think is the wrong approach).

[jayantxie]

I'm considering using gc bitmap to traverse objects, using field offset combined with dwarf types to locate downstream types in order to avoid the issue of potentially missing some live memory by purely traversing the dwarf.

[jayantxie]

You can't call Locals, it's too high level for what you are doing, you need to read the registers directly or you will not find some live memory. Also, to support range-over-func I will be making changes to Locals that will break it completely for your use case.

Get~

[aarzilli]

This change is because you want to cache the module data list right?

[jayantxie]

Yes, if module data is created every time calling findModuleDataForType, the performance will be very bad.

[jayantxie]

@aarzilli I marked a TODO to read the registers directly instead of reading from Locals, and searching sub objects by bitmap rather than dwarf types. And I rollbacked the API functions you commented. Can you please take a look again?

[aarzilli]

LGTM

[jayantxie]

LGTM

Please help to merge it.

[aarzilli]

It will be merged by Derek when he has time.

[derekparker]

One question.

I don't have access to the repo you listed in the earlier comment so couldn't investigate myself.

[derekparker]

Why export this when all members are unexported without accessor methods?

[aarzilli]

It's for storing it somewhere else and then passing it to RuntimeTypeToDIE.

[jayantxie]

One question.

I don't have access to the repo you listed in the earlier comment so couldn't investigate myself.

I made the temporary repo private, but I will soon open source the code to the github.com/cloudwego/goref repository.

[derekparker]

LGTM