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

stop using go:linkname across modules #546

Open
adonovan opened this issue May 9, 2024 · 4 comments
Open

stop using go:linkname across modules #546

adonovan opened this issue May 9, 2024 · 4 comments

Comments

@adonovan
Copy link
Collaborator

adonovan commented May 9, 2024

go1.23 will start to enforce "two-party consent" on linkname: that is, you can use it to share variables between packages controlled by the same author (in practice: within the module), but not to punch through the drywall of other modules.

Starlark-go uses cross-module linkname for two purposes:

  1. the profiler uses nanotime (see comments in the code for why alternatives are inferior). This tstime/mono: remove package tailscale/tailscale#8839 (comment) comment suggests an efficient portable alternative.
  2. lib/proto pokes detrand=false into the google3 protobuf formatter to defeat its annoying intentional nondeterminism. The solution there is to (ugh) fork the formatter.
@rsc
Copy link

rsc commented May 9, 2024

It is unclear if Go 1.23 will enforce exactly that level, but either way it's good for starlark not to be using linkname.

@adonovan
Copy link
Collaborator Author

https://protobuf.dev/programming-guides/serialization-not-canonical/ explains why proto serialization is inherently unstable: essentially, known and unknown fields are ordered differently, which means the age of the descriptors linked into your executable (and thus the set of known fields) affects the ordering. If this was just for text marshaling, forking the formatter would be unpleasant but viable, but this argument applies to binary marshaling too, and there's no way we're going to fork that code.

I can't yet see a way to both avoid linkname and retain the determinism that is essential to many applications of Starlark... though perhaps it has always been less deterministic than I thought.

@fenollp
Copy link
Contributor

fenollp commented May 18, 2024

Could this help? https://github.com/planetscale/vtprotobuf
It provides strict marshaling: sorted by field number.

And about proto descriptors being inherently incompatible with Starlark goals: how about updating lib/proto's APIs so some kind of reference to the descriptors gets passed around? A hash maybe?

@adonovan
Copy link
Collaborator Author

Could this help? https://github.com/planetscale/vtprotobuf

Yes, it could, though it's a substantial dependency.

And about proto descriptors being inherently incompatible with Starlark goals

I'm not sure exactly what you mean. Certainly descriptors can evolve, and that means the API they present to Starlark problems will change, but I don't see a fundamental problem there. It's also possible the "unknown fields" issue just doesn't matter much in practice: if all calls to marshal are based on Starlark values constructed from whole cloth, the outcome is deterministic; it's only if you call unmarshal and then pass that result back to marshal that there will be any unknown fields. I suspect the number of applications that do that is relatively small, and only a subset of them will care about strict determinism of the encoding.

Further research is needed before we know what action to take, and we should be guided by the actual users of the proto module.

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

No branches or pull requests

3 participants