Skip to content
This repository has been archived by the owner on Dec 14, 2023. It is now read-only.

Wrong package used by parser? #43

Closed
quarnster opened this issue Sep 16, 2013 · 8 comments
Closed

Wrong package used by parser? #43

quarnster opened this issue Sep 16, 2013 · 8 comments

Comments

@quarnster
Copy link
Contributor

I'm trying to build targeting pnacl using my MBP laptop, but running into some issues. (Pull request fixing some issues will come later as they are ironed out.)

One of the issues is that Mtim appears to be Mtime + Mtimensec, and Atim is Atime + Atimensec so stat_pnacl.go have to be changed to reflect this, no biggie.

12:20 ~/code/go/src/github.com/axw/llgo/cmd/llgo-dist $ strings ../../pkg/syscall/ztypes_pnacl.go | grep Mtim
Mtime
Mtimensec
12:21 ~/code/go/src/github.com/axw/llgo/cmd/llgo-dist $ strings $GOPATH/pkg/llgo/pnacl/syscall.bc | grep Mtim
struct{Dev int64; Ino uint64; Mode uint32; Nlink uint32; Uid uint32; Gid uint32; Rdev int64; Size int64; Blksize int32; Blocks int32; Atime int64; Atimensec int64; Mtime int64; Mtimensec int64; Ctime int64; Ctimensec int64}T)aa
12:21 ~/code/go/src/github.com/axw/llgo/cmd/llgo-dist $

However, llgo's parsing now complains that there is no Mtime in the syscall.Stat_t structure which obviously isn't the case: /Users/quarnster/code/go/src/github.com/axw/llgo/pkg/os/stat_pnacl.go:24:22: invalid operation: st (variable of type *syscall.Stat_t) has no field or method Mtime

This makes me believe that the parser is picking up the standard go syscall package rather than the llgo compiled pnacl one.

@axw
Copy link
Member

axw commented Sep 16, 2013

Thanks for reporting the issue. The PNaCl support is pretty weak; I've not had time for it lately.

You're quite right about llgo picking up the standard package. llgo-build is not currently instructing llgo to use the appropriate struct tags and so on for importing.

I think it's not going to be trivial to fix this. It's probably going to require the introduction of a custom Importer to pass to go/types.

@quarnster
Copy link
Contributor Author

I figured as much :)

Are you already looking into it or would it be of any help if I did it myself?

@axw
Copy link
Member

axw commented Sep 17, 2013

It would be very helpful if you'd like to do it.

Originally I was thinking that it would be good to have an importer that works off bitcode metadata. i.e. when an archive is generated, embed export information in a metadata node.

Lately I've been considering removing the direct dependency of llgo on LLVM's native library, and instead implementing a basic pure-Go IR writer and using that. Thus, reading in metadata would not be ideal (unless an IR reader were also implemented...); something akin to gccgo's ".gox" files would be a reasonable alternative.

@quarnster
Copy link
Contributor Author

As #48 is blocked by this one, I'll look into getting this working before resuming work on #48. (btw I can't assign myself apparently, but feel free to do so if you want)

@quarnster
Copy link
Contributor Author

After spending some time with DWARF I'm not convinced it can express what's needed for this issue. In particular multiple returns from a function, which standard gc encodes in dwarf as just function parameters rather than proper returns. So right now I think the quickest solution is to dump the exports to a separate file on compile and read that back in when needed.

@axw
Copy link
Member

axw commented Oct 1, 2013

which standard gc encodes in dwarf as just function parameters rather than proper returns

Do we need to follow gc here? Even if we did encode multiple returns in the return type, it'd have to be squashed into a single struct or something. Anyway, it's not really imperative that we do it via debug info.

@quarnster
Copy link
Contributor Author

Do we need to follow gc here?

What llgo currently does for named parameters is that they are defined as variables in the function scope, which IMO is more correct.

In any case, I'd like to just get this working and we can implement a more ideal approach in the future.

@quarnster
Copy link
Contributor Author

If we write the data out in the "gc generated export data" format, we're halfway there with the help of GcImportData. Huzzah! ;)

Since it takes an io.Reader, it should be possible to merge llvm bitcode and exported data into a single file in the future.

quarnster added a commit to quarnster/llgo that referenced this issue Oct 4, 2013
I suggest making the drone.io builder run llgo-dist through twice.
The first time it'll use the "regular" gc importer for uncompiled
packages. The second run, all packages have been compiled and should
thus use the custom importer.
axw added a commit that referenced this issue Oct 4, 2013
llgo: Custom package importer/exporter. Fixes issue #43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants