On 21/03/2019 2:11 am, Ioi Lam wrote:
That's the problem with ld and static libraries. If libbfd.a depends on
libz.a, you must put libbfd.a before libz.a.
Since this is a rarely touched Makefile, I think what you're doing in
webrev.02 is fine. We can refactor it later if it becomes more unwieldy.
Okay I can live with webrev.02 as well.
Thanks,
David
-----
Thanks
- Ioi
On 3/20/19 12:07 AM, Yasumasa Suenaga wrote:
Hi David,
I tried to fix as you suggested, but it was failed.
x86_64-w64-mingw32-gcc -o build/windows-amd64/hsdis-amd64.dll
-Ibuild/windows-amd64/include
-I/home/ysuenaga/rpmbuild/BUILD/binutils-2.31.1/include
-I/home/ysuenaga/rpmbuild/BUILD/binutils-2.31.1/bfd
-Ibuild/windows-amd64/bfd -DLIBARCH_amd64 -DLIBARCH=\"amd64\"
-DLIB_EXT=\".dll\" -O hsdis.c -shared build/windows-amd64/zlib/libz.a
build/windows-amd64/bfd/libbfd.a
build/windows-amd64/opcodes/libopcodes.a
build/windows-amd64/libiberty/libiberty.a
build/windows-amd64/bfd/libbfd.a(compress.o):compress.c:(.text+0x5b):
undefined reference to `inflateInit_'
I tried to compile with moving libz.a to the last argument, it was
succeeded.
x86_64-w64-mingw32-gcc -o build/windows-amd64/hsdis-amd64.dll
-Ibuild/windows-amd64/include
-I/home/ysuenaga/rpmbuild/BUILD/binutils-2.31.1/include
-I/home/ysuenaga/rpmbuild/BUILD/binutils-2.31.1/bfd
-Ibuild/windows-amd64/bfd -DLIBARCH_amd64 -DLIBARCH=\"amd64\"
-DLIB_EXT=\".dll\" -O hsdis.c -shared
build/windows-amd64/bfd/libbfd.a
build/windows-amd64/opcodes/libopcodes.a
build/windows-amd64/libiberty/libiberty.a
build/windows-amd64/zlib/libz.a
I'm not unclear why it is failed.
But order of libraries seems to be important.
Yasumasa
2019年3月20日(水) 15:18 David Holmes <david.hol...@oracle.com>:
On 20/03/2019 4:03 pm, Yasumasa Suenaga wrote:
Hi David,
2019年3月20日(水) 14:25 David Holmes <david.hol...@oracle.com>:
Hi Yasumasa,
On 20/03/2019 3:01 pm, Yasumasa Suenaga wrote:
Thanks David!
I uploaded new webrev.
This change affects Windows only, and I confirmed it works fine for
Linux x64 and MinGW64.
http://cr.openjdk.java.net/~ysuenaga/JDK-8220784/webrev.01/
I this is for MINGW only then shouldn't the change be made within this
section:
Did you say we should determine with $(MINGW) whether libz.a is added
to LDFLAGS?
webrev is here:
http://cr.openjdk.java.net/~ysuenaga/JDK-8220784/webrev.02/
The code is added to outside of the section you pointed.
$(LIBRARIES) is available after OS-specific code.
So we need to change like webrev.02 to minimize changes.
Or you could add
LIBRARIES = .../libzip.a
inside the existing MINGW section and then change the external
LIBRARIES = ...
to
LIBRARIES += ...
I'm unclear whether MINGW is only an issue on Linux when building for
Windows or whether also of use on Windows itself?
David
Yasumasa
## OS = Linux ##
ifeq ($(OS),Linux)
ifneq ($(MINGW),)
LIB_EXT = .dll
CPPFLAGS += -I$(TARGET_DIR)/include
LDFLAGS += -L$(TARGET_DIR)/lib
OS=windows
ifneq ($(findstring x86_64-,$(MINGW)),)
ARCH=amd64
else
ARCH=i386
endif
CC = $(MINGW)-gcc
CONFIGURE_ARGS= --host=$(MINGW) --target=$(MINGW)
else #linux
?
David
-----
Thanks,
Yasumasa
2019年3月20日(水) 11:25 David Holmes <david.hol...@oracle.com>:
On 20/03/2019 11:54 am, Yasumasa Suenaga wrote:
Hi David,
2019年3月20日(水) 10:38 David Holmes <david.hol...@oracle.com>:
Hi Yasumasa,
I'm not familar with building hsdis, but if the only currnet
problem is
on Windows, why is the fix not restricted to only building on
Windows?
For Solaris and Linux, -lz is not added to LDFLAGS. I think it
means
initial committer want to use zlib in binutils.
And zlib in binutils will be built implicitly in build phase in
binutils.
Otherwise we need people who build hsdis on other platforms to
comment
on the appropriateness of the fix.
If using different zlib is used by platforms is allowed, I will
upload
webrev which affects Windows only.
What do you think?
That seems preferable/simpler. Then you only need to know that your
change works for non-MinGW64 environments.
Thanks,
David
Thanks,
Yasumasa
Thanks,
David
On 20/03/2019 11:07 am, Yasumasa Suenaga wrote:
Hi Erik, David,
I checked this change on Linux x64 and MinGW for Windows.
According to hsdis README, we need to use MinGW cross compiler to
build hsdis [1]. So I think Cygwin is not required.
I do not have macOS and AIX. So I cannot check this change on
them.
BTW is hsdis included Java SE spec?
hsdis seems not to be included jtreg tests, and it is not
contained in
OpenJDK binaries.
I think it is allowed even if we lack some test for hsdis if
hsdis is
not required for Java SE.
If not so, I want sponsor(s) for this change.
Thanks,
Yasumasa
[1]
http://hg.openjdk.java.net/jdk/jdk/file/ddfb658c8ce3/src/utils/hsdis/README#l91
2019年3月20日(水) 6:54 David Holmes <david.hol...@oracle.com>:
CC'ing hotspot-dev. I agree this needs to be checked on every
platform
affected. I can't comment on the fix itself.
David
On 20/03/2019 2:36 am, Erik Joelsson wrote:
I think this needs to be reviewed by at least someone in
hotspot who
regularly builds hsdis. I can't really comment on the
validity of the
patch as I'm unfamiliar with both hsdis as well as this
makefile. Have
you at least verified the build on all the platforms which
you affect
with this change (which would be at least Macos, AIX and
Windows in a
normal Cygwin VS env)?
/Erik
On 2019-03-18 17:56, Yasumasa Suenaga wrote:
Hi all,
Please review this change:
JBS: https://bugs.openjdk.java.net/browse/JDK-8220784
webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8220784/webrev.00/
I attempt to build hsdis for Windows on WSL Ubuntu 18.04 with
gcc-mingw-w64-x86-64, but I saw error messages that some
functions
which are provided by zlib are unresolved.
We need to link to zlib.
Thanks,
Yasumasa
