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

proposal: cmd/go: add flag to allow internal paths #46792

Open
stevemk14ebr opened this issue Jun 16, 2021 · 15 comments
Open

proposal: cmd/go: add flag to allow internal paths #46792

stevemk14ebr opened this issue Jun 16, 2021 · 15 comments

Comments

@stevemk14ebr
Copy link

@stevemk14ebr stevemk14ebr commented Jun 16, 2021

Hello!

Now before i say anything, i know this is going to be controversial, but please don't close this immediately and hear me out, please :)

The goruntime code (here) is essentially the living documentation of internal compiler information. This is things like types, metadata table layouts, table parsers, etc. To me, as a reverse engineer, this is insanely useful information. And better yet, this is information and code I can just use out of the box to parse these complex file format structures. However, there's a really big problem with this, alot of the parsers and structures I care about are behind internal paths. For me to build tooling that uses this information I have to, by hand, re-path and re-factor hundreds of files transitively. I understand these structures change often, but when this occurs I could just update this runtime code and perform the required interface changes manually, rather than having to ALSO do that + re-path everything to not be behind a /internal path.

My ask is simple. Please allow a compiler flag that allows imports from internal or other restricted paths. I understand why this rule exists, and why this is probably not wanted, but like all rules there are exceptional cases where the bad option is much nicer than the good option, and this is one of them. Please give me an escape hatch.

@mdempsky mdempsky changed the title Add compiler flag to allow internal paths cmd/go: add flag to allow internal paths Jun 16, 2021
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Jun 16, 2021

"Internal" packages are a cmd/go detail. The compiler isn't involved in enforcing them.

FWIW, I've similarly thought at times in the past that it would be helpful to be able to write tools out of tree that had access to compiler/runtime internals, accepting that things can and likely will break.

@stevemk14ebr
Copy link
Author

@stevemk14ebr stevemk14ebr commented Jun 16, 2021

sorry I didn't know that, thanks for clarifying! I'm glad you see it may be useful too, I thought there may have been something I was missing to get my use case working but unfortunately had to rely on the horrible hack i mentioned to re-path everything

@tankbusta
Copy link

@tankbusta tankbusta commented Jun 16, 2021

I too have copied out some of the internal runtime code for RE purposes but I feel this isn't the best way to go about it. I fear that adding an escape hatch like this would cause issues downstream with others using it to bypass internal packages used by external libraries.

@seankhliao seankhliao changed the title cmd/go: add flag to allow internal paths proposal: cmd/go: add flag to allow internal paths Jun 16, 2021
@gopherbot gopherbot added this to the Proposal milestone Jun 16, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Jun 17, 2021
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 17, 2021

I think the effect of this change would be that if we change the runtime we will break an increasing number of external packages, and the effect of that will be that we are increasingly reluctant to change the runtime. We already run into trouble with external packages that use go:linkname to reach into the runtime, and that is more clearly unsupported than this option would be.

@stevemk14ebr
Copy link
Author

@stevemk14ebr stevemk14ebr commented Jun 17, 2021

I am open to alternatives, i just want to fix my problem! Perhaps this code here can decide to not put things behind /internal, i don't want to have to put my few little tools into the entire runtime tree.

We already run into trouble with external packages that use go:linkname to reach into the runtime, and that is more clearly unsupported than this option would be.

This is a people problem i think. Make it clear the use of these flags or features is not a 'supported' configuration and stuff will break if you rely on internal details. I'm advocating for an escape hatch when i KNOW i want to rely on internal details that totally can break at any time. As it is i have to do way more work to rely on internal details because of this rule, it would be way better for me to just have to chase the interfaces, right now i have to change just about everything

  • interfaces in code
  • source paths
  • source imports
  • capitalize non-public structure members

@ianthehat
Copy link

@ianthehat ianthehat commented Jun 17, 2021

I think a better solution might be a tool that removed the "by hand" from the problem statement.
If it was trivial to take the internal packages and move them to a new location, rewriting as needed, then most of your issue would go away. The same tool would also be far more generally useful (things like creating persistent forks).
If we could take something like rf and give it those capabilities, would that solve your problem?

@stevemk14ebr
Copy link
Author

@stevemk14ebr stevemk14ebr commented Jun 17, 2021

I'm not familiar with rf. I will say i do not understand the wisdom of refactoring the entire code base to not be behind /internal, you achieve the same effect of relying on internal details, with all the issues that entails, but you also now have the additional problem/complexity of making sure the refactor is perfect.

This boils down to an artificial limitation (which makes sense 90% of the time!), bypassing this limitation seems odd since it need not exist at all.

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 17, 2021

If it was trivial to take the internal packages and move them to a new location, rewriting as needed, then most of your issue would go away.

Note that my goforward prototype in CL 137076 had the capability to do that as part of its -move operation, including a -filter flag to copy only selected declarations. It never did quite work reliably, but the code in that CL could perhaps serve as the basis for a general “copy a subset of declarations” tool.

@bcmills
Copy link
Member

@bcmills bcmills commented Jun 17, 2021

IMO if we want to expose the compiler and runtime internals, they should be public packages in some explicitly-unstable module — for the sake of argument, call it golang.org/x/compiler. Then cmd/compile could be a thin wrapper around golang.org/x/compiler/... in much the same way that cmd/vet is currently a thin wrapper around golang.org/x/tools/analysis/....

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 17, 2021

We absolutely do not want to expose the compiler and runtime internals. That will slow down the pace of compiler/runtime development for no commensurate advantage.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 17, 2021

@stevemk14ebr I think that what @ianthehat was suggesting was that you could use a tool that copies types out of the runtime and renames them (and their fields) to be exported. Then you could run that tool once for each new Go release and use the resulting types. The new packages would not be in the standard library at all.

@stevemk14ebr
Copy link
Author

@stevemk14ebr stevemk14ebr commented Jun 17, 2021

for no commensurate advantage.

golang malware is very real, and rather popular. These internal details are necessary for me to use to combat this. Just because the issue is not frequent, does not mean it is not real or useful.

I would be fine with a tool that does this renaming and exporting and stuff. My point was more that you wouldn't really have to do this at all if you just created a way to toggle the artificial limitation off, and it'd be much simpler since you don't have to make some tool that can handle refactoring everything correctly in ever case. I imagine that would be kinda complex to do correctly, and probably not used all the time. Complex + Infrequent use == bugs

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 17, 2021

To be clear, I didn't say that the need was not real or useful. These kinds of decisions have costs and benefits. In this case I believe that the cost would be vastly greater than the benefit. That doesn't mean that there is no benefit. It means that the cost is high.

@zephyrtronium
Copy link
Contributor

@zephyrtronium zephyrtronium commented Jun 19, 2021

As #46792 (comment) mentioned, internal packages are a detail of the go tool. If you need different behavior, you can use a different tool that invokes go tool compile and friends directly. A fork of cmd/go seems straightforward. Back in the day, we used makefiles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants