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

Chdir flag #4462

Closed
wants to merge 24 commits into from
Closed

Chdir flag #4462

wants to merge 24 commits into from

Conversation

archie2x
Copy link
Contributor

No description provided.

ydnar and others added 24 commits August 20, 2024 17:03
`defer` and `GOMIPS=softfloat` together would result in a crash. This
patch fixes this issue.
I made an awkward mistake, mixing up GOOS and GOARCH. So here is a fix,
with an associated test.
This required a few compiler and runtime tricks to work, but I ran a
bunch of tests and it seems fine. (CI will of course do more exhaustive
testing).

The main benefit here is that we don't need to maintain the darwin
version of the syscall package, and reduce extra risks for bugs (because
we reuse the well-tested syscall package). For example, Go 1.23 needed a
bunch of new constants in the syscall package. That would have been
avoided if we had used the native syscall package on MacOS.
This should widen compatibility a bit, so that older CPUs can also
execute programs built by TinyGo. The performance may be lower, if
that's an issue we can look into implementing the proposal here:
golang/go#60072

This still wouldn't make programs usable on MIPS II CPUs, I suppose we
can lower compatiblity down to that CPU if needed.

I tried setting the -cpu flag in the QEMU command line to be able to
test this, but it looks like there are no QEMU CPU models that are
mips32r1 and have a FPU. So it's difficult to test this.
The interp package was assuming that all targets were little-endian. But
that's not true: we now have a big-endian target (GOARCH=mips).

This fixes the interp package to use the appropriate byte order for a
given target.
The reflect package needs to know the endianness of the system in a few
places. Before this patch, it assumed little-endian systems. But with
GOARCH=mips we now have a big-endian system which also needs to be
supported. So this patch fixes the reflect package to work on big-endian
systems.

Also, I've updated the tests for MIPS: instead of running the
little-endian tests, I've changed it to run the big-endian tests
instead. The two are very similar except for endianness so this should
be fine. To be sure we won't accidentally break little-endian support,
I've kept a single MIPS little-endian test (the CGo test, which doesn't
yet work on big-endian systems anyway).
This just makes the next fix easier to read.
The values were stored in the passed object as the values itself (not
expanded like is common in the calling convention), and read back after
assuming they were expanded. This often works for simple parameters
(int, pointer, etc), but not for more complex parameters. Especially
when there's padding.

Found this while working on `//go:wasmexport`.
It seems to have been replaced with the Component Model `run` function.
Signed-off-by: Roger Standridge <[email protected]>
Signed-off-by: Roger Standridge <[email protected]>
…hanges directory)

Signed-off-by: Roger Standridge <[email protected]>
Copy link
Contributor

@leongross leongross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@archie2x you opened the PR against release instead of dev, you have to change that.

@archie2x archie2x closed this Sep 12, 2024
@archie2x
Copy link
Contributor Author

@archie2x you opened the PR against release instead of dev, you have to change that.

I'm not sure how I opened this PR. Mis-click. Closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants