Generate asmparse.c using a standard bison/yacc

[kyulee1]

asmparse.c (for ilasm) is currently prebuilt using an internal version of yacc. This also caused many compilations warnings on x-platforms.

[sivarv]

@kyulee1 Is this needed for RC2 or can be moved to future?

[ranma42]

Would it be ok if the parser was generated using byacc instead of Bison? It generates a parser that almost works, except for requiring realloc(). I experimented with injecting a (kind of hackish, as it is based on new[]/delete[], but simple and working) C++ reimplementation of realloc, but better options might be possible.

I would be happy to try and work on this issue.

[RussKeldorph]

@ranma42 Sorry your response fell through the cracks. byacc seems like a reasonable choice. If you're still interested, I can check on the license (if any). The most important thing is to get the source into a state where anyone can compile asmparse.y with public tools. After that, we may want to figure out how to integrate the tools with the build and delete the generated prebuilt/asmparse.cpp. The choice of tool should take the latter possibility into account.

[ranma42]

I am still interested and I have gathered some more data and questions :) I tried to generate the parser using several tools:

• btyacc can generate an asmparse.cpp that compiles without any other intervention on the grammar or to the other files (as shown in https://github.com/ranma42/coreclr/commit/3dcf6dfd4e18eafbbf0419ee2154ac5427595cbd).
• byacc can generate a parser that compiles if provided with an implementation of realloc (which is currently intentionally made unavailable)
• bison generates a parser that does not compile for non-trivial reasons (such as conflicts in the defines)
• bisonc++ rejects the grammar

All of tools are available as packages for ubuntu, but they might not be as convenient to get and install in other environments.

If the current direction (btyacc) looks promising, I can work on completing the integration, otherwise I can revert to byacc by adding and documenting the hackery needed to let it get some kind of realloc.

Regarding the integration of btyacc, I wonder if it makes sense to build it as part of the compilation process instead of adding it as an external requirement.

[RussKeldorph]

@ranma42 Awesome! The fact that btyacc can be used without changes is great news. At the very least people will be able to test changes to asmparse.y before submitting a PR.

@mmitche @markwilkie @gkhanna79 This is a case where we would want to integrate a new tool into the coreclr build system, but we'd prefer not to add a "machine prep" dependency and instead have the coreclr build pull down the tool itself--even if the tool is easy to acquire on Linux. In this case, it looks like we could indeed build BtYacc from source as suggested by @ranma42, but I feel like that should be something done in BuildTools or even offline and perhaps uploaded to Nuget. Has this problem been solved in BuildTools? Is there prior art or recommendations?

[markwilkie]

This is not the only tool where this is needed - for example, cmake. There's guidance that @dleeapho can point to for how this works today. In fact, @ravimeda is slated to do this same work for cmake in S117.

[ranma42]

If you give me some directions on which is the best way to integrate tools in the build, I can try and update https://github.com/ranma42/coreclr/tree/fix-2305. Should I go ahead and do a PR out of it, so that the discussion can move there?

[ravimeda]

Thanks very much for the investigation, and proposed solution @ranma42

I am working on improving the experience around searching and acquiring build prerequisites. Please see https://github.com/dotnet/corefx/pull/19404

For official builds, the plan is to host build prerequisites, for instance btyacc, on a storage location accessible to build machinery. Before consuming a public tool in official builds, we need to ensure there is no security risks. I plan to work on this setup in June. So, at that time @ranma42 we can plan on integrating your proposed changes.

[ranma42]

Any updates on this? (@ravimeda's PR has been closed without being merged)

[RussKeldorph]

@dleeapho Can you help with guidance?

[dleeapho]

Though we are going to tackle the toolset issue all-up in the context of source-build efforts I don't think we are ready yet. Go ahead and bring in byacc the customary way through init-tools.*

[glenn-slayden]

Ping, voting for this, Any progress?

It would be nice to simply be able to build ILAsm.exe, in full, on any maximally-default open-source system of anyone's choosing...

[Frassle]

Go ahead and bring in byacc the customary way through init-tools.*

What exactly is that? It seems that's pulling all dependencies from a nuget package "microsoft.dotnet.buildtools"? How would one go about helping out with adding btyacc to that?

[Frassle]

Furthermore could this be taken even further and remove ilasm/ildasm from build tools? Is there a reason we need to download ilasm/ildasm? Can't we always just build from source?

[john-h-k]

btyacc can generate an asmparse.cpp that compiles without any other intervention on the grammar or to the other files (as shown in https://github.com/ranma42/coreclr/commit/3dcf6dfd4e18eafbbf0419ee2154ac5427595cbd)

I tried using btyacc, and while it does compile, it doesn't successfully parse a lot of valid IL. It seems there are some differences in the internal tool's grammar interpretation

[hez2010]

Any update? I'm trying to do some changes on asmparse.y but immediately hit this issue, because I don't have the Microsoft VCBU Internal YACC to generate asmparse.cpp.

[TIHan]

It is a big problem that an internal tool has to be used to generate asmparse.cpp as it means the IL grammar cannot be updated without the help from someone within Microsoft.

@john-h-k perhaps you could share how you integrated btyacc - it may only require a few tweaks to the grammar or btyacc itself to have it successfully parse, though it is hard to say.

[Frassle]

It is very old, but this is a commit from 4 years ago (when this was still coreclr) in my fork to try replacing asmparse.cpp with one built from btyacc, maybe it'll help: https://github.com/Frassle/coreclr/commit/3bb775c1ffbf8371c512237b90e6e8b8796be701

[john-h-k]

@TIHan my approach was just some small tweaks to the attempt by @ranma42 here

[hez2010]

mono has a managed implementation of ilasm, maybe we can switch to the managed one and compile it with NativeAOT?

[kant2002]

I see some strange thing when try to generate asmparse.cpp using btyacc.

This looks a lot like missing changes in Y file. Where Somewhere in grammar was declared GENCONSTRAINT_ but never made it to git. That change was done by @AaronRobinsonMSFT in https://github.com/dotnet/runtime/commit/6be799e5a637545a631ebdee0b4c3e3681ef3bb3

maybe that’s artifact of internal tooling? Can somebody clarify

[AaronRobinsonMSFT]

The change in question was more about BYREFLIKE_, which seems to remain. I made a few changes and tried several things during development, but don't recall GENCONSTRAINT_. I would see if the previous version had it and if not, assume it was simply an artifact of local development that didn't make the cut. The important change in that commit is BYREFLIKE_.