Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v4]
> WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits: - Capstone Support - Add check for compiler in configure - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis - Changes: https://git.openjdk.org/jdk/pull/18915/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18915=03 Stats: 395 lines in 19 files changed: 201 ins; 41 del; 153 mod Patch: https://git.openjdk.org/jdk/pull/18915.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18915/head:pull/18915 PR: https://git.openjdk.org/jdk/pull/18915
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v3]
> WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Capstone Support - Add check for compiler in configure - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis - Changes: - all: https://git.openjdk.org/jdk/pull/18915/files - new: https://git.openjdk.org/jdk/pull/18915/files/09fb3d65..b0b088e9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18915=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18915=01-02 Stats: 121745 lines in 2137 files changed: 85534 ins; 24227 del; 11984 mod Patch: https://git.openjdk.org/jdk/pull/18915.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18915/head:pull/18915 PR: https://git.openjdk.org/jdk/pull/18915
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Wed, 15 May 2024 13:32:38 GMT, Magnus Ihse Bursie wrote:
> Hi Julian, sorry for not getting back to you.
>
> The problem from my PoV is that this is a very large and intrusive patch in
> the build of the actual product, for a claimed problem in the tangential
> hsdis library. I have not understood a pressing business need to get a
> Windows/gcc port for hsdis, which means I can't really prioritize trying to
> understand this patch.
>
> As you know, the build system does not currently really separate between "the
> OS is Windows" and "the toolchain is Microsoft". I understand that you want
> to fix that, and in theory I support it. However, you must also realize that
> making a complete fix of this requires a lot of work, for something that
> there is no clearly defined need. (After all, cl.exe works fine to create the
> build, has no apparent issues, and is as far as an "official" compiler for
> Windows as you can get.) That makes it fall squarely in the WIBNIs bucked
> ("wouldn't it be nice if...").
>
> If you can fix just the hsdis build without changing anything outside the
> hsdis Makefiles, that would be a different story. Such a change would be
> limited in scope, easy to say it will not affect the product proper, and be
> easier to review.
Oh, no worries. Sorry for sounding a little impatient.
The problem is that there are areas in the build system that will need changing
to support hsdis compilation, not just the hsdis Makefile and autoconf file
itself. I can try to work on minimizing the amount of changes to non-hsdis
files (I was hoping the current changes were small enough, but it seems they
are not), but I believe it's impossible to achieve this by only touching the
hsdis Makefile and lib-hsdis.m4. That is, there will necessarily be changes, no
matter how minimal, to non-hsdis files.
As for why do this at all, I was mainly driven by seeing the hack in place for
the binutils backend on Windows. It only supports Cygwin, of which the gcc
installation is subpar compared to the newer gcc provided by some MSYS2
subsystems (MINGW64 gcc is primarily battle tested on MSYS2, on Cygwin it
doesn't get as much attention and misses out further fixes and enhancements
from the MSYS2 team, for instance it even links to the obsolete msvcrt
library), and the hack itself has several flaws that don't seem apparent at
first (For instance, -lpthread will link to libwinpthread.dll and result in an
invisible dependency on that dll depending on the Windows platform, which will
cause loading hsdis to silently fail depending on how it's loaded for seemingly
random reasons!). I wanted to replace that (or I guess provide a better
alternative) by adding semi proper support to the hsdis build for the more
modern and better battle tested gcc that some MSYS2 subsystems provide, to
streamline comp
iling the binutils hsdis on Windows. So this is mainly about hsdis-binutils on
Windows. hsdis-capstone I also decided to support because it's small and
relatively easy to pile on top of the existing work, and MSYS2 also provides
Capstone as well. The same unfortunately could not be said for hsdis-llvm,
which was significantly more complex than Capstone is
I'm not sure where to go from here. I could support gcc for hsdis binutils by
extending the hack that exists for Cygwin, but I _really_ don't want to do
that. I could enhance the build system to semi properly support gcc for hsdis
only, as I've done, but the changes will necessarily touch non hsdis files as
well, no matter how minimal they are. I'll return this to draft for the time
being I suppose, I'd be interested in hearing which way forward you prefer
though
But while I work on making this change even smaller and easier to review, could
I ask the above questions again? (Well, except for the FIXPATH one, you can
ignore that)
> @magicus I think I might need some help here. Currently all the Cygwin stuff
> is gated behind ifeq ($(TOOLCHAIN_TYPE), microsoft) checks... should they be
> behind isBuildOsEnv cygwin checks instead? I don't know how to add support
> for Cygwin gcc for most of hsdis, since it is quite different from the more
> modern gcc distributions that are found in places like MSYS2 and MINGW64. But
> most of the existing logic seems to accomodate more for the Microsoft
> compiler than it is concerned about the OS environment being used, and for
> this reason I can't tell which of the 2 checks to use for the existing hack
> that switches from microsoft to gcc. Also, gcc doesn't require FIXPATH, but
> Microsoft does, but I don't want to check for microsoft inside
> TOOLCHAIN_FIND_COMPILER, what should I do instead to ensure gcc doesn't get
> FIXPATH while Microsoft does?
> Also @magicus, what is the typical path passed to --with-binutils like on
> Windows? Something like
> --with-binutils=/c/Users/vertig0/Downloads/binutils-2.42 doesn't work
> correctly, since the include path to dis-asm.h
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote:
>> WIP
>>
>> This changeset contains hsdis for Windows/gcc Port. It supports both the
>> binutils and capstone backends, though the LLVM backend is left out due to
>> compatibility issues encountered during the build. Currently, which gcc
>> distributions are supported is still to be clarified, as several, ranging
>> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason,
>> the build system hack in place at the moment to compile the binutils backend
>> on Windows is still left in place, for now.
>
> Julian Waters has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains two commits:
>
> - Add check for compiler in configure
> - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
Hi Julian, sorry for not getting back to you.
The problem from my PoV is that this is a very large and intrusive patch in the
build of the actual product, for a claimed problem in the tangential hsdis
library. I have not understood a pressing business need to get a Windows/gcc
port for hsdis, which means I can't really prioritize trying to understand this
patch.
As you know, the build system does not currently really separate between "the
OS is Windows" and "the toolchain is Microsoft". I understand that you want to
fix that, and in theory I support it. However, you must also realize that
making a complete fix of this requires a lot of work, for something that there
is no clearly defined need. (After all, cl.exe works fine to create the build,
has no apparent issues, and is as far as an "official" compiler for Windows as
you can get.) That makes it fall squarely in the WIBNIs bucked ("wouldn't it be
nice if...").
If you can fix just the hsdis build without changing anything outside the hsdis
Makefiles, that would be a different story. Such a change would be limited in
scope, easy to say it will not affect the product proper, and be easier to
review.
-
PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2112546029
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Julian Waters has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Add check for compiler in configure > - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis Magnus? Erik? You guys there? :( - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2112465392
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
On Thu, 9 May 2024 07:50:00 GMT, Julian Waters wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Julian Waters has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains two commits: > > - Add check for compiler in configure > - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis Hmm, it seems Magnus isn't available at the moment. @erikj79 might you be able to answer my questions regarding hsdis? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2109049333
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Wed, 24 Apr 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Please mark the PR as draft it is not intended for review. @magicus? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2106619396
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Wed, 24 Apr 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Please mark the PR as draft it is not intended for review. @magicus? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2103667760
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Wed, 24 Apr 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Please mark the PR as draft it is not intended for review. Also @magicus, what is the typical path passed to --with-binutils like on Windows? Something like --with-binutils=/c/Users/vertig0/Downloads/binutils-2.42 doesn't work correctly, since the include path to dis-asm.h would then become `#include "/c/Users/vertig0/Downloads/binutils-2.42/include/dis-asm.h"` Which causes a configure check to fail on the compile stage since gcc cannot recognise the MINGW-esque /c/ as a drive, and then causes configure to erroneously report binutils as using the Old API when it's in fact using the New API. --with-binutils=C:/Users/vertig0/Downloads/binutils-2.42 on the other hand works as expected. Should there be a fixup for the path there so gcc can recognise it properly? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2102146604
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis [v2]
> WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. Julian Waters has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - Add check for compiler in configure - 8330988: Implementation of 8288293: Windows/gcc Port for hsdis - Changes: https://git.openjdk.org/jdk/pull/18915/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18915=01 Stats: 334 lines in 19 files changed: 154 ins; 25 del; 155 mod Patch: https://git.openjdk.org/jdk/pull/18915.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18915/head:pull/18915 PR: https://git.openjdk.org/jdk/pull/18915
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Wed, 24 Apr 2024 09:15:21 GMT, Magnus Ihse Bursie wrote: >> WIP >> >> This changeset contains hsdis for Windows/gcc Port. It supports both the >> binutils and capstone backends, though the LLVM backend is left out due to >> compatibility issues encountered during the build. Currently, which gcc >> distributions are supported is still to be clarified, as several, ranging >> from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, >> the build system hack in place at the moment to compile the binutils backend >> on Windows is still left in place, for now. > > Please mark the PR as draft it is not intended for review. @magicus I think I might need some help here. Currently all the Cygwin stuff is gated behind ifeq ($(TOOLCHAIN_TYPE), microsoft) checks... should they be behind isBuildOsEnv cygwin checks instead? I don't know how to add support for Cygwin gcc for most of hsdis, since it is quite different from the more modern gcc distributions that are found in places like MSYS2 and MINGW64. But most of the existing logic seems to accomodate more for the Microsoft compiler than it is concerned about the OS environment being used, and for this reason I can't tell which of the 2 checks to use for the existing hack that switches from microsoft to gcc. Also, gcc doesn't require FIXPATH, but Microsoft does, but I don't want to check for microsoft inside TOOLCHAIN_FIND_COMPILER, what should I do instead to ensure gcc doesn't get FIXPATH while Microsoft does? - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2097196121
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Tue, 23 Apr 2024 13:56:32 GMT, Julian Waters wrote: > WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. Please mark the PR as draft it is not intended for review. - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2074481887
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Tue, 23 Apr 2024 14:25:22 GMT, Magnus Ihse Bursie wrote: > There's a huge amount of changes for just hsdis... You might have to separate > out the infrastructure changes that seem to amount to most of the changes > here. > > This is going to take me a while to get through. Sorry, it's still a WIP, and I believe something broke and scattered changes from one of my other local branches into this current changeset - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2072483937
Re: RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
On Tue, 23 Apr 2024 13:56:32 GMT, Julian Waters wrote: > WIP > > This changeset contains hsdis for Windows/gcc Port. It supports both the > binutils and capstone backends, though the LLVM backend is left out due to > compatibility issues encountered during the build. Currently, which gcc > distributions are supported is still to be clarified, as several, ranging > from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, > the build system hack in place at the moment to compile the binutils backend > on Windows is still left in place, for now. There's a huge amount of changes for just hsdis... You might have to separate out the infrastructure changes that seem to amount to most of the changes here. This is going to take me a while to get through. - PR Comment: https://git.openjdk.org/jdk/pull/18915#issuecomment-2072458273
RFR: 8330988: Implementation of 8288293: Windows/gcc Port for hsdis
WIP This changeset contains hsdis for Windows/gcc Port. It supports both the binutils and capstone backends, though the LLVM backend is left out due to compatibility issues encountered during the build. Currently, which gcc distributions are supported is still to be clarified, as several, ranging from Cygwin, to MinGW64, to the gcc subsystems from MSYS2. For this reason, the build system hack in place at the moment to compile the binutils backend on Windows is still left in place, for now. - Commit messages: - Implementation of hsdis for 8288293: Windows/gcc Port Changes: https://git.openjdk.org/jdk/pull/18915/files Webrev: https://webrevs.openjdk.org/?repo=jdk=18915=00 Issue: https://bugs.openjdk.org/browse/JDK-8330988 Stats: 347 lines in 28 files changed: 161 ins; 22 del; 164 mod Patch: https://git.openjdk.org/jdk/pull/18915.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18915/head:pull/18915 PR: https://git.openjdk.org/jdk/pull/18915
