[11] Review Request: 8200198 javah man pages were not removed by JDK-8191054

[Sergey Bylokhov]

Hello.
Please review fix for jdk11.

Bug: https://bugs.openjdk.java.net/browse/JDK-8200198
Webrev can be found at: http://cr.openjdk.java.net/~serb/8200198/webrev.00

The man pages related to javah tool were removed from the ws in the same 
way as it was done for policytool and for tools related to CORBA.


The usage of these files were removed in JDK-8193512[1]
http://hg.openjdk.java.net/jdk/jdk10/rev/26b47ea4c77d

--
Best regards, Sergey.

Re: RFR: JDK-8200083: Bump bootjdk requirement for JDK 11 to JDK 10

[John Paul Adrian Glaubitz]

On 03/24/2018 01:07 AM, Magnus Ihse Bursie wrote:
> But if javac developers are seriously hindered in their effort to enhance 
> Java due to this, then maybe developer convenience is not as important.

But is using the latest Java features really so important for OpenJDK
development? I mean, do people seriously say "Oh, we have type inference
for variables now. We have to use it immediately, it's making the code so
much better and faster!!!" (sorry, couldn't resist the hyperbole).

I mean, in the end, OpenJDK is developed for users and not for the OpenJDK
developers sake to use Java, isn't it? So, I think the project should always
keep users and downstream interests in mind.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [NEW BUG]: Configure broken on MIPS

[John Paul Adrian Glaubitz]

On Mar 23, 2018, at 7:04 PM, Ao Qi  wrote:

>>> 
>>> It might be some time still. I'm working on a complete overhaul of all
>>> CFLAGS and LDFLAGS, where this is a part of that picture, but I was not
>>> planning on addressing just this thing urgently.
>>> 
>>> So, I think this patch will do for now. It solves the immediate problem
>>> for MIPS, and I can come back and make a cleaner solution later on.
>> 
>> 
>> Isn't the best quick fix one that only adds -m64 for x86? I recall  a report
>> that arm32 is similarly broken.
> 
> I had a look at man gcc. It seems that at least -m64 is for S/390 and
> zSeries, SPARC , RS/6000 and PowerPC, and x86.

I hope you don’t break architectures like m68k which don’t support that flag.

Adrian

Re: [NEW BUG]: Configure broken on MIPS

[Magnus Ihse Bursie]

On 2018-03-24 01:00, David Holmes wrote:

On 24/03/2018 1:58 AM, Magnus Ihse Bursie wrote:


On 2018-03-23 11:05, David Holmes wrote:

On 23/03/2018 7:54 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 09:55, David Holmes wrote:

On 23/03/2018 6:46 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 06:22, David Holmes wrote:

Hi Thomas,

On 23/03/2018 2:55 PM, Thomas Stüfe wrote:

Hi David,

would it not be pragmatic to accept Ao's patch - it looks fine 
to me - since it certainly would not make matters worse. And 
let Magnus follow up with a cleanup change later?


Well I hope Magnus's change is forthcoming. 
It might be some time still. I'm working on a complete overhaul 
of all CFLAGS and LDFLAGS, where this is a part of that picture, 
but I was not planning on addressing just this thing urgently.


So, I think this patch will do for now. It solves the immediate 
problem for MIPS, and I can come back and make a cleaner solution 
later on.


Isn't the best quick fix one that only adds -m64 for x86? I recall 
a report that arm32 is similarly broken.
Not really, because this is also needed on some other platforms, at 
least s390x, as I recall. (This was the reason it was originally 
added.)


According to gcc docs there are 4 archs that use m64 and we only 
care about 2 of them:


m64: SPARC Options
m64: S/390 and zSeries Options
m64: RS/6000 and PowerPC Options
m64: i386 and x86-64 Options

Note that this logic also applies to the xlc compiler, not only gcc. 
I also believe that solstudio requires it on sparcv9, but I'm not 
100% sure.


But you need to know whether you are dealing with S390 or S390x as 
m64 implies zSeries.

We only support s390x (64-bit), not s390.

And OpenJDK still support ppc64, even if Oracle does not test it. So 
I'd say that we need -m64 on all those platforms listed. So then the 
question is, is it easier to check if it's one of four, or not one of 
tree known CPU architectures. (For mips, we shouldn't test 
OPENJDK_TARGET_CPU for a lot of mips*, but OPENJDK_TARGET_CPU_ARCH 
for mips). For me, it's like, whatever. I'll probably rewrite this 
from the ground up anywhat. What we're trying to express here is that 
some flags are needed as CFLAGS always, even when running configure 
checks. It has really nothing to do with the number of CPU bits.


My recollection for x86 was we only needed to specify -m64 to ensure a 
64-bit build if the compiler was not configured to do that by default. 
Very much tied to the number of CPU bits we wanted to build for.


In any case isn't the whole point of configure that you can check if a 
compiler has a flag before you use it ??


Good point, the correct long term fix is probably to just check if 
-m is a supported option, instead of hardcoding platforms. I'll 
make a mental note of it.


/Magnus



David


/Magnus



Ao will need a sponsor to create a bug etc regardless of which way 
this goes.


My week is over. :)

Cheers,
David


/Magnus



David
-

AFAICS it's as easy to write this only for x86 as it is to 
exclude it for non x86. Honestly I don't know why the Aarch64 
patch was done the way it was - there must be some subtlety here 
that I'm not aware of.
I think it was just the smallest patch that worked for the 
aarch64 platform. I didn't spend time arguing about the fix, 
since it is supposed to be short-lived anyway.


/Magnus

Re: [NEW BUG]: Configure broken on MIPS

[David Holmes]

On 24/03/2018 1:58 AM, Magnus Ihse Bursie wrote:


On 2018-03-23 11:05, David Holmes wrote:

On 23/03/2018 7:54 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 09:55, David Holmes wrote:

On 23/03/2018 6:46 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 06:22, David Holmes wrote:

Hi Thomas,

On 23/03/2018 2:55 PM, Thomas Stüfe wrote:

Hi David,

would it not be pragmatic to accept Ao's patch - it looks fine to 
me - since it certainly would not make matters worse. And let 
Magnus follow up with a cleanup change later?


Well I hope Magnus's change is forthcoming. 
It might be some time still. I'm working on a complete overhaul of 
all CFLAGS and LDFLAGS, where this is a part of that picture, but I 
was not planning on addressing just this thing urgently.


So, I think this patch will do for now. It solves the immediate 
problem for MIPS, and I can come back and make a cleaner solution 
later on.


Isn't the best quick fix one that only adds -m64 for x86? I recall a 
report that arm32 is similarly broken.
Not really, because this is also needed on some other platforms, at 
least s390x, as I recall. (This was the reason it was originally added.)


According to gcc docs there are 4 archs that use m64 and we only care 
about 2 of them:


m64: SPARC Options
m64: S/390 and zSeries Options
m64: RS/6000 and PowerPC Options
m64: i386 and x86-64 Options

Note that this logic also applies to the xlc compiler, not only gcc. I 
also believe that solstudio requires it on sparcv9, but I'm not 100% sure.


But you need to know whether you are dealing with S390 or S390x as m64 
implies zSeries.

We only support s390x (64-bit), not s390.

And OpenJDK still support ppc64, even if Oracle does not test it. So I'd 
say that we need -m64 on all those platforms listed. So then the 
question is, is it easier to check if it's one of four, or not one of 
tree known CPU architectures. (For mips, we shouldn't test 
OPENJDK_TARGET_CPU for a lot of mips*, but OPENJDK_TARGET_CPU_ARCH for 
mips). For me, it's like, whatever. I'll probably rewrite this from the 
ground up anywhat. What we're trying to express here is that some flags 
are needed as CFLAGS always, even when running configure checks. It has 
really nothing to do with the number of CPU bits.


My recollection for x86 was we only needed to specify -m64 to ensure a 
64-bit build if the compiler was not configured to do that by default. 
Very much tied to the number of CPU bits we wanted to build for.


In any case isn't the whole point of configure that you can check if a 
compiler has a flag before you use it ??


David


/Magnus



Ao will need a sponsor to create a bug etc regardless of which way 
this goes.


My week is over. :)

Cheers,
David


/Magnus



David
-

AFAICS it's as easy to write this only for x86 as it is to exclude 
it for non x86. Honestly I don't know why the Aarch64 patch was 
done the way it was - there must be some subtlety here that I'm 
not aware of.
I think it was just the smallest patch that worked for the aarch64 
platform. I didn't spend time arguing about the fix, since it is 
supposed to be short-lived anyway.


/Magnus

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

On 2018-03-23 17:05, mandy chung wrote:

This is a very good change and no more mapfile to maintain!!

Thank you!



Please do file JBS issues for the component teams to clean up their 
exports.

I have now filed:

https://bugs.openjdk.java.net/browse/JDK-8200191 -- for java.base
https://bugs.openjdk.java.net/browse/JDK-8200192 -- for java.desktop
https://bugs.openjdk.java.net/browse/JDK-8200193 -- for jdk.security.auth

/Magnus



Mandy

On 3/23/18 7:30 AM, Erik Joelsson wrote:

I have looked at the build changes and they look good.

Will you file followups for each component team to look over their 
exported symbols, at least for the libraries with 
$(EXPORT_ALL_SYMBOLS)? It sure looks like there is some technical 
debt laying around here.


/Erik


On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort 
to using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility 
on Windows, for most of the shared code, we already have proper 
JNIEXPORT decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead 
export all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. 
The only reason Windows is impacted at all, is that some changes 
needed by Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual 
generated code. Unfortunately, this was not possible to fully 
achieve. I do not believe that these changes will have any actual 
impact on the product, though. I will present the differences more 
in detail further down. Those who are not interested can probably 
skip that.


The patch has passed tier1 testing and is currently running tier2 
and tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base 
and java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the 
long-standing convention of adding JNICALL. This is a no-op on 
non-Windows platforms, so for most of the changes this is purely 
cosmetic (and possibly adding in robustness, should the function 
ever be used on Windows in the future). I have also followed the 
stylistic convention of putting "JNIEXPORT  JNICALL" on 
a separate line. For some functions, however, this might cause a 
change in calling convention on Windows. Since this can not apply to 
exported functions on Windows (otherwise they would already have had 
JNIEXPORT), I do not think this matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. 
This actually meant that all symbols were exported. It is highly 
unclear if this was known and intended by the original make rule 
writer. I have emulated this by adding the flag 
$(EXPORT_ALL_SYMBOLS) to these libraries. Hopefully, we can remove 
this flag and fix proper exported symbols in the future.


I have run the complete build using COMPARE_BUILD, and made a 
thourough analysis of the differences for Linux and Solaris. All 
native libraries have symbol differences, but most of them are 
trivial and/or harmless. As a result, most libraries have disasm 
differences as well, but these too seem trivial and harmless. The 
differences in symbols that are common to all libraries include:
 * Internal symbols such as __bss_start, _edata, _end and _fini are 
now global. (They are imported as such from the compiler 
libraries/archives, and we have no linker script to override this 
behavior).
 * The versioning tag SUNWprivate_1.1 is not included, and thus 
neither the .gnu.version_d symbol.
 * There are a few differences in the symbol and/or mangling of some 
local functions. I'm not sure what's causing this,

but it's unlikely to have any effect on the product.

Another common source for change in symbols is due to previous 
platform 

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

On 2018-03-23 18:24, Volker Simonis wrote:

Hi Magnus,

thanks for addressing this long standing issue! I haven't looked at
the changes, but just want to share some general and historical notes:

- Compiling with "-fvisibility=hidden" which hides all symbols expect
the ones explicitly exported with
"__attribute__((visibility("default")))" has been requested by SAP
back in 2007 even before we had OpenJDK (see "Use -fvisibility=hidden
for gcc compiles" https://bugs.openjdk.java.net/browse/JDK-6588413)
and finally pushed into the OpenJKD around 2010.
- "-fvisibility=hidden" gave us performance improvements of about 5%
(JBB2005) and 2% (JVM98) on Linux/IA64 and 1,5% (JBB2005) and 0,5%
(JVM98) on Linux/PPC64 because the compiler could use faster calls for
non exported symbols. This improvement was only very small on x86
tough.
That's a nice side effect! Although my main purpose here is 
maintainability, gaining performance is nothing I say no to. :)

- "-fvisibility=hidden"/"__attribute__((visibility("default")))"
applies BEFORE using the map files in the linking step (i.e. hidden
symbols can't be exported any more even if mentioned in the map file)
- because of the performance improvements we got by using
"-fvisibility=hidden" it was worth while using it even though we had
the mapfiles at the end of the process.

Then we had several mail threads (which you probably remember because
you were involved :) where we discussed to either remove the map files
completely or instead generate them automatically during the build:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12412
http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12628

The main arguments against removing the map files at that time were:

1. the danger to re-export all symbols of statically linked libraries
(notably libstdc++ at that time)
2. loosing exports of compiler generated symbols like vtables which
are required by the Serviceability Agent

Point 1 is not a problem today, because I don't think we do any static
linking any more. If we still do it under some circumstances, this
problem should be re-evaluated.
Well, we do static linking with libstdc++ on linux, in certain 
circumstances. See "--with-stdc++lib=,,". 
Fortunately, this is not a problem. The linker can be told not to 
include symbols from statically linked libraries, which is exactly what 
I do with LDFLAGS_JDKLIB += -Wl,--exclude-libs,ALL.


The corresponding feature does not exist for the solstudio linker, but 
fortunately we do not use statically linked libraries there.



Point 2 is only relevant for HotSpot. But because of "8034065: GCC 4.3
and later doesn't export vtable symbols any more which seem to be
needed by SA" (https://bugs.openjdk.java.net/browse/JDK-8034065),
exporting such symbols trough a map files doesn't work any more
anyway. So this isn't a problem either.
In any case, that's a question for another day. :) There were reasons I 
left Hotspot out of this fix, and the question about the SA agent is one 
of them. :) As you say, I think they do not apply anymore, but I'll 
return to consider Hotspot later on.



So to cut a long story short - I think the time is ripe to get rid of
the map files. Thumbs up from me (meant as moral support, not as a
concrete review :)

Thanks for the kind words!

/Magnus



Regards,
Volker

On Fri, Mar 23, 2018 at 5:05 PM, mandy chung  wrote:

This is a very good change and no more mapfile to maintain!!

Please do file JBS issues for the component teams to clean up their exports.

Mandy


On 3/23/18 7:30 AM, Erik Joelsson wrote:

I have looked at the build changes and they look good.

Will you file followups for each component team to look over their
exported symbols, at least for the libraries with $(EXPORT_ALL_SYMBOLS)? It
sure looks like there is some technical debt laying around here.

/Erik


On 2018-03-23 06:56, Magnus Ihse Bursie wrote:

With modern compilers, we can use compiler directives (such as
_attribute__((visibility("default"))), or __declspec(dllexport)) to control
symbol visibility, directly in the source code. This has historically not
been present on all compilers, so we had to resort to using mapfiles (also
known as linker scripts).

This is no longer the case. Now all compilers we use support symbol
visibility directives, in one form or another. We should start using this.
Since this has been the only way to control symbol visibility on Windows,
for most of the shared code, we already have proper JNIEXPORT decorations in
place.

If we fix the remaining platform-specific files to have proper JNIEXPORT
tagging, then we can finally get rid of mapfiles.

This fix removed mapfiles for all JDK libraries. It does not touch
hotspot libraries nor JDK executables; they will have to wait for a future
fix -- this was complex enough. This change will not have any impact on
macosx, since we do not use mapfiles there, but instead export all symbols.
(This is not a good idea, bu

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

On 2018-03-23 18:33, Phil Race wrote:

There are a lot of changes in the desktop libraries.
Well, yes and no. While there are multiple touched files, the resulting 
native shared libraries that are built have very minimal changes in 
them. (That's the view point from the build guy, you know :))


Doing mach5 tier1/2/3 testing is not nearly sufficient to cover those 
since
only tier3 has any UI tests and it barely uses anything that's touched 
here.

So since testing seems to be wise, then I think you should do a
jtreg desktop group run on Linux & Windows.
There is next to *no* difference for java.desktop on Windows. The only, 
very subtle, difference, is that awt.dll now exports 18 more functions 
(totalling 800, instead of 782). I can't even begin to imagine how 
anything could fail due to this additional exporting. Not even the 
disassembly of the machine code of awt.dll is different from before, not 
a single byte. So I don't buy it that I need to do extensive client 
testing on Windows.


You can probably skip Mac since it is unaffected and I think Linux 
will cover Solaris here.

You should also do some headless testing.
I agree that it seems prudent to do some Linux/Solaris testing, since 
changes there are more wide spread. Could you please point me to some 
guidance on how to run these tests? (You can do it off list)
It could take some time to review this properly and decide what 
changes are OK,
what changes are something we should clean up later, and what changes 
are something

that ought to be addressed now ..
As I said, I am going to file follow-up bugs for suspicious handling of 
exported symbols. These follow-up bugs will be separated per component 
team, unlike this fix, which by necessity addresses all JDK libraries at 
once. So you will get plenty of time to consider ways of cleaning up any 
exports handling that you do not like. It would be a pity if this entire 
checkin was delayed since the client team could not accept the changes 
needed in client libraries. :-(


And frankly, I believe the java.desktop libs needs some serious 
refactoring to get to grip with the exported symbols situation. The 
major cause of problems is, I believe, rooted in a non-optimal split of 
functionality between libawt, libawt_xawt and libawt_headless. This is 
not likely something that can be addressed in this change.


I think I'd be mainly concerned that something fails due to a missing 
symbol, or
that for newly exported symbols if we ended up with duplicate symbols 
as a result.


Once again, I've run the COMPARE_BUILD script on this patch. Let me 
explain a bit more in detail what it does, since that might be known 
only to us in the build team. This script analyses the build result, the 
jmods, the lib*.so files, etc. The basic idea here is that a change in 
the build system, which does not produce a change in the build result, 
is "transparent" to the product. There is e.g. no reason to run any 
further testing, since we're in effect testing the same bits. For many 
changes in the build system, we hold this as the gold standard.


For this particular change, to achieve this kind of fidelity would have 
come with a too high price in code complexity, so I have allowed certain 
small deviations. These are really minimal, and should in most cases be 
undetectable by the product. The changes in Linux and Solaris that have 
occured, is those that I listed in my review mail. Basically, for some 
libraries, additional symbols are exported. I could fix this, but only 
at the expense of more complex code. While it's a good thing to minimize 
the functions exported, a handful extra symbols is not a disaster. (We 
have more important issues to address in our native libraries.)


For the AWT libraries, most of the duplicates are coming from the source 
code that are shared between libraries, in 
java.desktop/share/native/common. This means that the same function is 
compiled into -- and now also exported from -- multiple libraries. This 
is not a big deal. Even if we were to link with two libraries defining 
the same symbol, the dynamic linker will arbitrarily chose one of them, 
but since they are identical, it does not matter. (It's another thing if 
they implement different functions, as you noted yourself in the bugs 
about linking with awt_xawt vs doing a runtime linking to awt_headless).


Also, I guarantee you that in no way are there missing symbols in the 
refactored build. I've checked, double-checked and triple-checked that.



The results of a test run will add confidence here.

BTW I don't think you are right that
java.desktop:/libawt_headless: The following symbols are now also 
exported due to JNIEXPORT (they were previously

..
 X11SurfaceData_GetOps

It looks to me like it was previously exported.
You are correct, it was previously exported in libawt_headless. I meant 
that it is now also exported for libawt_xawt due to the JNIEXPORT. Sorry 
for mixing this up.


/Magnus




-phil.



On 03/23/2

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

On 2018-03-23 19:01, Phil Race wrote:


http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01/src/java.desktop/share/native/libmlib_image/mlib_image_proto.h.udiff.html 



The variable definitions here are now misaligned.
No, they are not. That's just an artifact of webrev, which filters out 
whitespace changes in the diff view. :-( To see the proper changes, 
including whitespace, you need to download the patch file.


I've gone to great pains to mimick the existing style in the source 
codes I've made changes to.


/Magnus



..and added 2d-dev since many of these native changes are in 2d.

-phil.

On 03/23/2018 10:33 AM, Phil Race wrote:

There are a lot of changes in the desktop libraries.
Doing mach5 tier1/2/3 testing is not nearly sufficient to cover those 
since
only tier3 has any UI tests and it barely uses anything that's 
touched here.

So since testing seems to be wise, then I think you should do a
jtreg desktop group run on Linux & Windows.
You can probably skip Mac since it is unaffected and I think Linux 
will cover Solaris here.

You should also do some headless testing.

It could take some time to review this properly and decide what 
changes are OK,
what changes are something we should clean up later, and what changes 
are something

that ought to be addressed now ..

I think I'd be mainly concerned that something fails due to a missing 
symbol, or
that for newly exported symbols if we ended up with duplicate symbols 
as a result.


The results of a test run will add confidence here.

BTW I don't think you are right that
java.desktop:/libawt_headless: The following symbols are now also 
exported due to JNIEXPORT (they were previously

..
 X11SurfaceData_GetOps

It looks to me like it was previously exported.


-phil.



On 03/23/2018 06:56 AM, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort 
to using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility 
on Windows, for most of the shared code, we already have proper 
JNIEXPORT decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead 
export all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. 
The only reason Windows is impacted at all, is that some changes 
needed by Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual 
generated code. Unfortunately, this was not possible to fully 
achieve. I do not believe that these changes will have any actual 
impact on the product, though. I will present the differences more 
in detail further down. Those who are not interested can probably 
skip that.


The patch has passed tier1 testing and is currently running tier2 
and tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base 
and java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the 
long-standing convention of adding JNICALL. This is a no-op on 
non-Windows platforms, so for most of the changes this is purely 
cosmetic (and possibly adding in robustness, should the function 
ever be used on Windows in the future). I have also followed the 
stylistic convention of putting "JNIEXPORT  JNICALL" on 
a separate line. For some functions, however, this might cause a 
change in calling convention on Windows. Since this can not apply to 
exported functions on Windows (otherwise they would already have had 
JNIEXPORT), I do not think this matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. 
This actually meant that all symbols were exported. It is highly 
unclear if this was known and intended by the original make rule 
writer. I have emulated this by adding the flag 
$(EXPORT_ALL_SYMBOLS) to these libraries. Hopef

Re: RFR: JDK-8200083: Bump bootjdk requirement for JDK 11 to JDK 10

[Martin Buchholz]

On Thu, Mar 22, 2018 at 8:13 AM, Jonathan Gibbons <
jonathan.gibb...@oracle.com> wrote:

>
> The interim JDK relies on javac and related tools being compilable by the
> boot JDK.  This imposes a restriction that the source code of those tools
> must be conformant to the source version supported by the boot JDK, meaning
> no use of any newer features. The javac team have always lived with and
> accepted the N-1 restriction that this imposes. With a more rapid cadence,
> it might be appropriate to revisit the N-1 rule. But since a "last LTS"
> rule may imply N-5 or N-6 or so, that seems like too much.
>

Historically, major java releases came out about once every 3 years, which
aligns pretty well with a "last LTS" rule.

Non-LTS releases such as jdk9 see cascading lack of support and hence lack
of adoption - your OS vendor may be reluctant to ship such a jdk.

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Phil Race]

http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01/src/java.desktop/share/native/libmlib_image/mlib_image_proto.h.udiff.html

The variable definitions here are now misaligned.

..and added 2d-dev since many of these native changes are in 2d.

-phil.

On 03/23/2018 10:33 AM, Phil Race wrote:

There are a lot of changes in the desktop libraries.
Doing mach5 tier1/2/3 testing is not nearly sufficient to cover those 
since
only tier3 has any UI tests and it barely uses anything that's touched 
here.

So since testing seems to be wise, then I think you should do a
jtreg desktop group run on Linux & Windows.
You can probably skip Mac since it is unaffected and I think Linux 
will cover Solaris here.

You should also do some headless testing.

It could take some time to review this properly and decide what 
changes are OK,
what changes are something we should clean up later, and what changes 
are something

that ought to be addressed now ..

I think I'd be mainly concerned that something fails due to a missing 
symbol, or
that for newly exported symbols if we ended up with duplicate symbols 
as a result.


The results of a test run will add confidence here.

BTW I don't think you are right that
java.desktop:/libawt_headless: The following symbols are now also 
exported due to JNIEXPORT (they were previously

..
 X11SurfaceData_GetOps

It looks to me like it was previously exported.


-phil.



On 03/23/2018 06:56 AM, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort 
to using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility 
on Windows, for most of the shared code, we already have proper 
JNIEXPORT decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead 
export all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. 
The only reason Windows is impacted at all, is that some changes 
needed by Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual 
generated code. Unfortunately, this was not possible to fully 
achieve. I do not believe that these changes will have any actual 
impact on the product, though. I will present the differences more in 
detail further down. Those who are not interested can probably skip 
that.


The patch has passed tier1 testing and is currently running tier2 and 
tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base and 
java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the 
long-standing convention of adding JNICALL. This is a no-op on 
non-Windows platforms, so for most of the changes this is purely 
cosmetic (and possibly adding in robustness, should the function ever 
be used on Windows in the future). I have also followed the stylistic 
convention of putting "JNIEXPORT  JNICALL" on a separate 
line. For some functions, however, this might cause a change in 
calling convention on Windows. Since this can not apply to exported 
functions on Windows (otherwise they would already have had 
JNIEXPORT), I do not think this matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. This 
actually meant that all symbols were exported. It is highly unclear 
if this was known and intended by the original make rule writer. I 
have emulated this by adding the flag $(EXPORT_ALL_SYMBOLS) to these 
libraries. Hopefully, we can remove this flag and fix proper exported 
symbols in the future.


I have run the complete build using COMPARE_BUILD, and made a 
thourough analysis of the differences for Linux and Solaris. All 
native libraries have symbol differences, but most of them are 
trivial and/or harmless. As a result, most libraries have disasm 
differences as 

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Phil Race]

There are a lot of changes in the desktop libraries.
Doing mach5 tier1/2/3 testing is not nearly sufficient to cover those since
only tier3 has any UI tests and it barely uses anything that's touched here.
So since testing seems to be wise, then I think you should do a
jtreg desktop group run on Linux & Windows.
You can probably skip Mac since it is unaffected and I think Linux will 
cover Solaris here.

You should also do some headless testing.

It could take some time to review this properly and decide what changes 
are OK,
what changes are something we should clean up later, and what changes 
are something

that ought to be addressed now ..

I think I'd be mainly concerned that something fails due to a missing 
symbol, or
that for newly exported symbols if we ended up with duplicate symbols as 
a result.


The results of a test run will add confidence here.

BTW I don't think you are right that
java.desktop:/libawt_headless: The following symbols are now also 
exported due to JNIEXPORT (they were previously

..
 X11SurfaceData_GetOps

It looks to me like it was previously exported.


-phil.



On 03/23/2018 06:56 AM, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort to 
using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility on 
Windows, for most of the shared code, we already have proper JNIEXPORT 
decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead 
export all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. 
The only reason Windows is impacted at all, is that some changes 
needed by Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual 
generated code. Unfortunately, this was not possible to fully achieve. 
I do not believe that these changes will have any actual impact on the 
product, though. I will present the differences more in detail further 
down. Those who are not interested can probably skip that.


The patch has passed tier1 testing and is currently running tier2 and 
tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base and 
java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the 
long-standing convention of adding JNICALL. This is a no-op on 
non-Windows platforms, so for most of the changes this is purely 
cosmetic (and possibly adding in robustness, should the function ever 
be used on Windows in the future). I have also followed the stylistic 
convention of putting "JNIEXPORT  JNICALL" on a separate 
line. For some functions, however, this might cause a change in 
calling convention on Windows. Since this can not apply to exported 
functions on Windows (otherwise they would already have had 
JNIEXPORT), I do not think this matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. This 
actually meant that all symbols were exported. It is highly unclear if 
this was known and intended by the original make rule writer. I have 
emulated this by adding the flag $(EXPORT_ALL_SYMBOLS) to these 
libraries. Hopefully, we can remove this flag and fix proper exported 
symbols in the future.


I have run the complete build using COMPARE_BUILD, and made a 
thourough analysis of the differences for Linux and Solaris. All 
native libraries have symbol differences, but most of them are trivial 
and/or harmless. As a result, most libraries have disasm differences 
as well, but these too seem trivial and harmless. The differences in 
symbols that are common to all libraries include:
 * Internal symbols such as __bss_start, _edata, _end and _fini are 
now global. (They are imported as such from the compiler 
libraries/archives, and we have no linker script to override this 
behavio

Re: RFR: 8199138: Add RISC-V support to Zero

[Magnus Ihse Bursie]

On 2018-03-20 14:54, Edward Nevill wrote:

On Tue, 2018-03-20 at 08:39 +0100, Erik Helin wrote:

Please review the following webrev

Bugid: https://bugs.openjdk.java.net/browse/JDK-8199138
Webrev: http://cr.openjdk.java.net/~enevill/8199138/webrev.00

32 # First, filter out everything that doesn't begin with "aarch64-"
33 if ! echo $* | grep '^aarch64-\|^riscv64-' >/dev/null ; then

Could you please update the comment on line 32 to say the same thing as
the code?


Hi Eirk,

Thanks for this. I have updated the webrev with the above comment.

http://cr.openjdk.java.net/~enevill/8199138/webrev.01
I note that in platform.m4 (sorry I didn't say this earlier), you set 
the CPU_ARCH to riscv64 as well, and not just riscv. Now I don't know 
how likely it is that OpenJDK will ever support the 32-bit version of 
riscv, but it seems like it would make more sense to define the CPU_ARCH 
as "riscv", and the CPU as "riscv64".


It's just a minor thing, if you like it the way it is, keep it.

/Magnus



I have also fixed a problem encountered with the submit-hs repo where the build 
machine had older headers which did not define EM_RISCV.

The solution is to define EM_RISCV if not already defined as is done for 
aarch64.

IE.

  #ifndef EM_AARCH64
#define EM_AARCH64183   /* ARM AARCH64 */
  #endif
+#ifndef EM_RISCV
+  #define EM_RISCV  243
+#endif

This now passes the submit-hs tests.

Does this look OK to push now?

Thanks,
Ed.

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Volker Simonis]

Hi Magnus,

thanks for addressing this long standing issue! I haven't looked at
the changes, but just want to share some general and historical notes:

- Compiling with "-fvisibility=hidden" which hides all symbols expect
the ones explicitly exported with
"__attribute__((visibility("default")))" has been requested by SAP
back in 2007 even before we had OpenJDK (see "Use -fvisibility=hidden
for gcc compiles" https://bugs.openjdk.java.net/browse/JDK-6588413)
and finally pushed into the OpenJKD around 2010.
- "-fvisibility=hidden" gave us performance improvements of about 5%
(JBB2005) and 2% (JVM98) on Linux/IA64 and 1,5% (JBB2005) and 0,5%
(JVM98) on Linux/PPC64 because the compiler could use faster calls for
non exported symbols. This improvement was only very small on x86
tough.
- "-fvisibility=hidden"/"__attribute__((visibility("default")))"
applies BEFORE using the map files in the linking step (i.e. hidden
symbols can't be exported any more even if mentioned in the map file)
- because of the performance improvements we got by using
"-fvisibility=hidden" it was worth while using it even though we had
the mapfiles at the end of the process.

Then we had several mail threads (which you probably remember because
you were involved :) where we discussed to either remove the map files
completely or instead generate them automatically during the build:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12412
http://mail.openjdk.java.net/pipermail/hotspot-dev/2014-February/thread.html#12628

The main arguments against removing the map files at that time were:

1. the danger to re-export all symbols of statically linked libraries
(notably libstdc++ at that time)
2. loosing exports of compiler generated symbols like vtables which
are required by the Serviceability Agent

Point 1 is not a problem today, because I don't think we do any static
linking any more. If we still do it under some circumstances, this
problem should be re-evaluated.

Point 2 is only relevant for HotSpot. But because of "8034065: GCC 4.3
and later doesn't export vtable symbols any more which seem to be
needed by SA" (https://bugs.openjdk.java.net/browse/JDK-8034065),
exporting such symbols trough a map files doesn't work any more
anyway. So this isn't a problem either.

So to cut a long story short - I think the time is ripe to get rid of
the map files. Thumbs up from me (meant as moral support, not as a
concrete review :)

Regards,
Volker

On Fri, Mar 23, 2018 at 5:05 PM, mandy chung  wrote:
> This is a very good change and no more mapfile to maintain!!
>
> Please do file JBS issues for the component teams to clean up their exports.
>
> Mandy
>
>
> On 3/23/18 7:30 AM, Erik Joelsson wrote:
>>
>> I have looked at the build changes and they look good.
>>
>> Will you file followups for each component team to look over their
>> exported symbols, at least for the libraries with $(EXPORT_ALL_SYMBOLS)? It
>> sure looks like there is some technical debt laying around here.
>>
>> /Erik
>>
>>
>> On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
>>>
>>> With modern compilers, we can use compiler directives (such as
>>> _attribute__((visibility("default"))), or __declspec(dllexport)) to control
>>> symbol visibility, directly in the source code. This has historically not
>>> been present on all compilers, so we had to resort to using mapfiles (also
>>> known as linker scripts).
>>>
>>> This is no longer the case. Now all compilers we use support symbol
>>> visibility directives, in one form or another. We should start using this.
>>> Since this has been the only way to control symbol visibility on Windows,
>>> for most of the shared code, we already have proper JNIEXPORT decorations in
>>> place.
>>>
>>> If we fix the remaining platform-specific files to have proper JNIEXPORT
>>> tagging, then we can finally get rid of mapfiles.
>>>
>>> This fix removed mapfiles for all JDK libraries. It does not touch
>>> hotspot libraries nor JDK executables; they will have to wait for a future
>>> fix -- this was complex enough. This change will not have any impact on
>>> macosx, since we do not use mapfiles there, but instead export all symbols.
>>> (This is not a good idea, but I'll address that separately.) This change
>>> will also have a minimal impact on Windows. The only reason Windows is
>>> impacted at all, is that some changes needed by Solaris and Linux were
>>> simpler to fix for all platforms.
>>>
>>> I have strived for this change to have no impact on the actual generated
>>> code. Unfortunately, this was not possible to fully achieve. I do not
>>> believe that these changes will have any actual impact on the product,
>>> though. I will present the differences more in detail further down. Those
>>> who are not interested can probably skip that.
>>>
>>> The patch has passed tier1 testing and is currently running tier2 and
>>> tier3. Since the running code is more or less (see caveat below) unmodified,
>>> I don't expect any testing 

Re: RFR: JDK-8200083: Bump bootjdk requirement for JDK 11 to JDK 10

[joe darcy]

In addition, the APIs in the java.compiler module are bootstrapped along 
with javac. Therefore, these APIs also have to abide by the same (N-k)  
language feature policy as javac. If the bootstrap is older than 
necessary, maintenance of these APIs would be overly constrained.


-Joe


On 3/23/2018 9:07 AM, Magnus Ihse Bursie wrote:

On 2018-03-22 16:13, Jonathan Gibbons wrote:

Magnus,

There has always been a desire that most of JDK is free to use the 
latest language and API features, meaning we must use the latest 
javac to compile most most of JDK.   That is where the "interim 
javac" comes in.


The interim JDK relies on javac and related tools being compilable by 
the boot JDK.  This imposes a restriction that the source code of 
those tools must be conformant to the source version supported by the 
boot JDK, meaning no use of any newer features. The javac team have 
always lived with and accepted the N-1 restriction that this imposes. 
With a more rapid cadence, it might be appropriate to revisit the N-1 
rule. But since a "last LTS" rule may imply N-5 or N-6 or so, that 
seems like too much.


I note that this is not just an issue for javac source code.  If we 
were currently on a "last LTS" rule, that implies JDK 8, which means 
the build would still have to be bimodal and support both 
(boot)classpath and modular builds. OK, that window is closing, but 
the general point is that supporting older versions may affect more 
than just the javac team.


I would not propose a "last LTS" scheme, that seems far to infrequent 
to be reasonable. But maybe "N-2" might be an acceptable compromise?


Just to be clear: As far as I understand, the balance to strike here 
is between:
1) on one hand, giving developers more time to adjust their build 
system to a newly available boot JDK.
2) on the other hand, allowing developers of javac access to new JDK 
features.


In keeping with the N-1 rule, we are nominally doing the same thing, 
but practically shifting things towards 2). That might still be the 
right thing to do, but we will then need to acknowledge that this is 
what we're doing.


Personally, I don't have any strong opinion either way. I agree with 
Erik's idea to keep the test matrix minimal, but on the other hand, 
there's a huge number of possible configurations to build OpenJDK on 
which is not regularly tested by Oracle's build system, and that works 
fine in most cases. If someone from the community needs it and it is 
broken, they can submit a patch. I could live with stating that "N-1" 
is officially supported and is guaranteed to work, but we will also 
support "N-2" but you'll have to test it yourself and submit bug fixes 
if it does not work.


But if javac developers are seriously hindered in their effort to 
enhance Java due to this, then maybe developer convenience is not as 
important.


/Magnus




-- Jon


On 3/21/18 11:10 PM, Magnus Ihse Bursie wrote:

Jon,

21 mars 2018 kl. 23:20 skrev Jonathan Gibbons 
:


Holding javac and related tools back to the latest LTS would indeed 
be somewhat onerous.
Can we use the interim JDK build to get around this? Something like, 
if we can build a interim JDK with somewhat older tools, it can then 
be used to compile the javac proper?


I can see that how with the increased release cadence, the 
assumptions behind the old N-1 scheme might not be valid anymore.


/Magnus


-- Jon


On 03/21/2018 03:07 PM, Martin Buchholz wrote:
Now that we are releasing jdks an order of magnitude faster than 
before, we

should reconsider the N-1 boot jdk policy.

The primary beneficiaries of this are compiler-dev, who might like 
to code

using the very features they are implementing.

But for users, being able to bootstrap with an ancient jdk is 
definitely

convenient.

A good compromise might be to be able to bootstrap with the most 
recent LTS

release (jdk 8) but it might already be too late for that.

On Wed, Mar 21, 2018 at 2:51 PM, Erik Joelsson 


wrote:

Now that JDK 10 has been officially released we can update the 
boot jdk
requirement for JDK 11. Cross posting this to jdk-dev to raise 
awareness of

this rather disruptive change.

This patch changes the requirement on boot jdk version in 
configure (and
updates the configuration that controls what JDK to use as boot 
in Oracle's

internal build system).

Webrev: http://cr.openjdk.java.net/~erikj/8200083/webrev.01/

Bug: https://bugs.openjdk.java.net/browse/JDK-8200083

/Erik

Re: RFR(XXS) : 8200180 : fix a typo in run-test framework documentation

[Igor Ignatyev]

Magnus, Erik,

thanks for your review.

-- Igor

> On Mar 23, 2018, at 8:30 AM, Magnus Ihse Bursie 
>  wrote:
> 
> 
>> 23 mars 2018 kl. 16:25 skrev Erik Joelsson :
>> 
>> Hello Igor,
>> 
>> This looks good, but please also run "make update-build-docs" so that the 
>> html file also gets regenerated before pushing.
> 
> Looks good to me too. 
> 
> /Magnus
> 
>> 
>> /Erik
>> 
>> 
>>> On 2018-03-23 08:13, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html
 3 lines changed: 0 ins; 0 del; 3 mod;
>>> 
>>> Hi all,
>>> 
>>> could you please review this small fix for run-test framework 
>>> documentation? VM_OTIONS was used instead of VM_OPTIONS at several places, 
>>> the fix is obvious s/VM_OTIONS/VM_OPTIONS/ .
>>> 
>>> webrev: http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8200180
>>> 
>>> Thanks,
>>> -- Igor
>>> 
>> 
> 

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[mandy chung]

This is a very good change and no more mapfile to maintain!!

Please do file JBS issues for the component teams to clean up their 
exports.


Mandy

On 3/23/18 7:30 AM, Erik Joelsson wrote:

I have looked at the build changes and they look good.

Will you file followups for each component team to look over their 
exported symbols, at least for the libraries with 
$(EXPORT_ALL_SYMBOLS)? It sure looks like there is some technical debt 
laying around here.


/Erik


On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort 
to using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility 
on Windows, for most of the shared code, we already have proper 
JNIEXPORT decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead 
export all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. 
The only reason Windows is impacted at all, is that some changes 
needed by Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual 
generated code. Unfortunately, this was not possible to fully 
achieve. I do not believe that these changes will have any actual 
impact on the product, though. I will present the differences more in 
detail further down. Those who are not interested can probably skip 
that.


The patch has passed tier1 testing and is currently running tier2 and 
tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base and 
java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the 
long-standing convention of adding JNICALL. This is a no-op on 
non-Windows platforms, so for most of the changes this is purely 
cosmetic (and possibly adding in robustness, should the function ever 
be used on Windows in the future). I have also followed the stylistic 
convention of putting "JNIEXPORT  JNICALL" on a separate 
line. For some functions, however, this might cause a change in 
calling convention on Windows. Since this can not apply to exported 
functions on Windows (otherwise they would already have had 
JNIEXPORT), I do not think this matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. This 
actually meant that all symbols were exported. It is highly unclear 
if this was known and intended by the original make rule writer. I 
have emulated this by adding the flag $(EXPORT_ALL_SYMBOLS) to these 
libraries. Hopefully, we can remove this flag and fix proper exported 
symbols in the future.


I have run the complete build using COMPARE_BUILD, and made a 
thourough analysis of the differences for Linux and Solaris. All 
native libraries have symbol differences, but most of them are 
trivial and/or harmless. As a result, most libraries have disasm 
differences as well, but these too seem trivial and harmless. The 
differences in symbols that are common to all libraries include:
 * Internal symbols such as __bss_start, _edata, _end and _fini are 
now global. (They are imported as such from the compiler 
libraries/archives, and we have no linker script to override this 
behavior).
 * The versioning tag SUNWprivate_1.1 is not included, and thus 
neither the .gnu.version_d symbol.
 * There are a few differences in the symbol and/or mangling of some 
local functions. I'm not sure what's causing this,

but it's unlikely to have any effect on the product.

Another common source for change in symbols is due to previous 
platform differences. For instance, if we had "JNIEXPORT int JNICALL 
do_foo() { ... }", but do_foo was not in the mapfile, the symbol was 
exported on Windows but not on Linux and Solaris. (Presumable since 
it was not needed there, even though it was compiled for those 
platforms as well.) Now, with t

Re: RFR: JDK-8200083: Bump bootjdk requirement for JDK 11 to JDK 10

[Magnus Ihse Bursie]

On 2018-03-22 16:13, Jonathan Gibbons wrote:

Magnus,

There has always been a desire that most of JDK is free to use the 
latest language and API features, meaning we must use the latest javac 
to compile most most of JDK.   That is where the "interim javac" comes 
in.


The interim JDK relies on javac and related tools being compilable by 
the boot JDK.  This imposes a restriction that the source code of 
those tools must be conformant to the source version supported by the 
boot JDK, meaning no use of any newer features. The javac team have 
always lived with and accepted the N-1 restriction that this imposes. 
With a more rapid cadence, it might be appropriate to revisit the N-1 
rule. But since a "last LTS" rule may imply N-5 or N-6 or so, that 
seems like too much.


I note that this is not just an issue for javac source code.  If we 
were currently on a "last LTS" rule, that implies JDK 8, which means 
the build would still have to be bimodal and support both 
(boot)classpath and modular builds. OK, that window is closing, but 
the general point is that supporting older versions may affect more 
than just the javac team.


I would not propose a "last LTS" scheme, that seems far to infrequent to 
be reasonable. But maybe "N-2" might be an acceptable compromise?


Just to be clear: As far as I understand, the balance to strike here is 
between:
1) on one hand, giving developers more time to adjust their build system 
to a newly available boot JDK.
2) on the other hand, allowing developers of javac access to new JDK 
features.


In keeping with the N-1 rule, we are nominally doing the same thing, but 
practically shifting things towards 2). That might still be the right 
thing to do, but we will then need to acknowledge that this is what 
we're doing.


Personally, I don't have any strong opinion either way. I agree with 
Erik's idea to keep the test matrix minimal, but on the other hand, 
there's a huge number of possible configurations to build OpenJDK on 
which is not regularly tested by Oracle's build system, and that works 
fine in most cases. If someone from the community needs it and it is 
broken, they can submit a patch. I could live with stating that "N-1" is 
officially supported and is guaranteed to work, but we will also support 
"N-2" but you'll have to test it yourself and submit bug fixes if it 
does not work.


But if javac developers are seriously hindered in their effort to 
enhance Java due to this, then maybe developer convenience is not as 
important.


/Magnus




-- Jon


On 3/21/18 11:10 PM, Magnus Ihse Bursie wrote:

Jon,

21 mars 2018 kl. 23:20 skrev Jonathan Gibbons 
:


Holding javac and related tools back to the latest LTS would indeed 
be somewhat onerous.
Can we use the interim JDK build to get around this? Something like, 
if we can build a interim JDK with somewhat older tools, it can then 
be used to compile the javac proper?


I can see that how with the increased release cadence, the 
assumptions behind the old N-1 scheme might not be valid anymore.


/Magnus


-- Jon


On 03/21/2018 03:07 PM, Martin Buchholz wrote:
Now that we are releasing jdks an order of magnitude faster than 
before, we

should reconsider the N-1 boot jdk policy.

The primary beneficiaries of this are compiler-dev, who might like 
to code

using the very features they are implementing.

But for users, being able to bootstrap with an ancient jdk is 
definitely

convenient.

A good compromise might be to be able to bootstrap with the most 
recent LTS

release (jdk 8) but it might already be too late for that.

On Wed, Mar 21, 2018 at 2:51 PM, Erik Joelsson 


wrote:

Now that JDK 10 has been officially released we can update the 
boot jdk
requirement for JDK 11. Cross posting this to jdk-dev to raise 
awareness of

this rather disruptive change.

This patch changes the requirement on boot jdk version in 
configure (and
updates the configuration that controls what JDK to use as boot in 
Oracle's

internal build system).

Webrev: http://cr.openjdk.java.net/~erikj/8200083/webrev.01/

Bug: https://bugs.openjdk.java.net/browse/JDK-8200083

/Erik

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Alan Bateman]

On 23/03/2018 15:15, Magnus Ihse Bursie wrote:

:
Very much so, yes. I've found a lot of dubious exports, everything from global 
variables (yuck!) to functions that does not seem to be used anymore, to lots 
of strange exports.
The changes looks good to me and I think we should follow this up with a 
few JIRA issues (as you suggested) for the symbols that don't make sense 
to export.


-Alan

Re: [NEW BUG]: Configure broken on MIPS

[Magnus Ihse Bursie]

On 2018-03-23 11:05, David Holmes wrote:

On 23/03/2018 7:54 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 09:55, David Holmes wrote:

On 23/03/2018 6:46 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 06:22, David Holmes wrote:

Hi Thomas,

On 23/03/2018 2:55 PM, Thomas Stüfe wrote:

Hi David,

would it not be pragmatic to accept Ao's patch - it looks fine to 
me - since it certainly would not make matters worse. And let 
Magnus follow up with a cleanup change later?


Well I hope Magnus's change is forthcoming. 
It might be some time still. I'm working on a complete overhaul of 
all CFLAGS and LDFLAGS, where this is a part of that picture, but I 
was not planning on addressing just this thing urgently.


So, I think this patch will do for now. It solves the immediate 
problem for MIPS, and I can come back and make a cleaner solution 
later on.


Isn't the best quick fix one that only adds -m64 for x86? I recall  
a report that arm32 is similarly broken.
Not really, because this is also needed on some other platforms, at 
least s390x, as I recall. (This was the reason it was originally added.)


According to gcc docs there are 4 archs that use m64 and we only care 
about 2 of them:


m64: SPARC Options
m64: S/390 and zSeries Options
m64: RS/6000 and PowerPC Options
m64: i386 and x86-64 Options

Note that this logic also applies to the xlc compiler, not only gcc. I 
also believe that solstudio requires it on sparcv9, but I'm not 100% sure.


But you need to know whether you are dealing with S390 or S390x as m64 
implies zSeries.

We only support s390x (64-bit), not s390.

And OpenJDK still support ppc64, even if Oracle does not test it. So I'd 
say that we need -m64 on all those platforms listed. So then the 
question is, is it easier to check if it's one of four, or not one of 
tree known CPU architectures. (For mips, we shouldn't test 
OPENJDK_TARGET_CPU for a lot of mips*, but OPENJDK_TARGET_CPU_ARCH for 
mips). For me, it's like, whatever. I'll probably rewrite this from the 
ground up anywhat. What we're trying to express here is that some flags 
are needed as CFLAGS always, even when running configure checks. It has 
really nothing to do with the number of CPU bits.


/Magnus



Ao will need a sponsor to create a bug etc regardless of which way 
this goes.


My week is over. :)

Cheers,
David


/Magnus



David
-

AFAICS it's as easy to write this only for x86 as it is to exclude 
it for non x86. Honestly I don't know why the Aarch64 patch was 
done the way it was - there must be some subtlety here that I'm 
not aware of.
I think it was just the smallest patch that worked for the aarch64 
platform. I didn't spend time arguing about the fix, since it is 
supposed to be short-lived anyway.


/Magnus

Re: RFR: 8199736: Define WIN32_LEAN_AND_MEAN before including windows.h

[Magnus Ihse Bursie]

This looks good to me.

/Magnus

> 23 mars 2018 kl. 14:58 skrev Erik Joelsson :
> 
> I think this looks good, but Magnus is currently refactoring the flags 
> handling in configure so better get his input as well. (adding build-dev)
> 
> /Erik
> 
> 
>> On 2018-03-23 05:37, Robin Westberg wrote:
>> Hi Kim & Erik,
>> 
>> Certainly makes sense to define it from the build system, I’ve updated the 
>> patch accordingly:
>> 
>> Full: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.01/ 
>> 
>> Incremental: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00-01/ 
>> 
>> 
>> (Not quite sure if the definition belongs where I put it or a bit later 
>> where most other windows-specific JVM flags are defined, but seemed 
>> reasonable to put it close to where it is defined for the JDK libraries).
>> 
>> Best regards,
>> Robin
>> 
> On 22 Mar 2018, at 16:52, Kim Barrett  > wrote:
 
 On Mar 22, 2018, at 10:34 AM, Robin Westberg >>> > wrote:
 
 Hi all,
 
 Please review the following change that defines WIN32_LEAN_AND_MEAN [1] 
 before including windows.h. This marginally improves build times, and 
 makes it possible to include winsock2.h.
 
 Issue: https://bugs.openjdk.java.net/browse/JDK-8199736 
 
 Webrev: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00/ 
  
 >
 Testing: hs-tier1
 
 Best regards,
 Robin
 
 [1] 
 https://msdn.microsoft.com/en-us/library/windows/desktop/aa383745%28v=vs.85%29.aspx#faster_builds_with_smaller_header_files
  
 >
>>> 
>>> I think the addition of the WIN32_LEAN_AND_MEAN definition should be done 
>>> through the build
>>> system, so that it applies everywhere.
>>> 
>> 
> 

Re: RFR(XXS) : 8200180 : fix a typo in run-test framework documentation

[Magnus Ihse Bursie]

> 23 mars 2018 kl. 16:25 skrev Erik Joelsson :
> 
> Hello Igor,
> 
> This looks good, but please also run "make update-build-docs" so that the 
> html file also gets regenerated before pushing.

Looks good to me too. 

/Magnus

> 
> /Erik
> 
> 
>> On 2018-03-23 08:13, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html
>>> 3 lines changed: 0 ins; 0 del; 3 mod;
>> 
>> Hi all,
>> 
>> could you please review this small fix for run-test framework documentation? 
>> VM_OTIONS was used instead of VM_OPTIONS at several places, the fix is 
>> obvious s/VM_OTIONS/VM_OPTIONS/ .
>> 
>> webrev: http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8200180
>> 
>> Thanks,
>> -- Igor
>>  
> 

Re: RFR(XXS) : 8200180 : fix a typo in run-test framework documentation

[Erik Joelsson]

Hello Igor,

This looks good, but please also run "make update-build-docs" so that 
the html file also gets regenerated before pushing.


/Erik


On 2018-03-23 08:13, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html

3 lines changed: 0 ins; 0 del; 3 mod;


Hi all,

could you please review this small fix for run-test framework documentation? 
VM_OTIONS was used instead of VM_OPTIONS at several places, the fix is obvious 
s/VM_OTIONS/VM_OPTIONS/ .

webrev: http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html
JBS: https://bugs.openjdk.java.net/browse/JDK-8200180

Thanks,
-- Igor
  

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

> 23 mars 2018 kl. 15:45 skrev Alan Bateman :
> 
>> On 23/03/2018 13:56, Magnus Ihse Bursie wrote:
>> With modern compilers, we can use compiler directives (such as 
>> _attribute__((visibility("default"))), or __declspec(dllexport)) to control 
>> symbol visibility, directly in the source code. This has historically not 
>> been present on all compilers, so we had to resort to using mapfiles (also 
>> known as linker scripts).
>> 
>> This is no longer the case. Now all compilers we use support symbol 
>> visibility directives, in one form or another. We should start using this. 
>> Since this has been the only way to control symbol visibility on Windows, 
>> for most of the shared code, we already have proper JNIEXPORT decorations in 
>> place.
>> 
>> If we fix the remaining platform-specific files to have proper JNIEXPORT 
>> tagging, then we can finally get rid of mapfiles.
> This seems like a great cleanup as the mapfile have always been a pain to 
> maintain. Also shines a light on some technical debt too.

Very much so, yes. I've found a lot of dubious exports, everything from global 
variables (yuck!) to functions that does not seem to be used anymore, to lots 
of strange exports. 

> handleSocketError in libnio is a surprise, this should not be exported and 
> should not have been in the map file.  I suspect the issue is that jdk.sctp 
> is missing a function prototype from its header file (it has its own 
> handleSocketError in SctpNet.c).

That might be so, yes. 

> NET_Wait in libnet is another one, I can't tell why this was listed in the 
> map file.

Neither can I. :-) Once again, my goal with this patch was to keep the produced 
binaries as similar to before with the mapfiles. I'll be happy to file 
follow-up bugs listing all suspicious symbol handling I've encountered, but I'd 
rather not change anything about that in this patch. 

> I'm also surprised with java.dll exporting handleRead, winHandleRead, and 
> handleLSeek. I didn't see them mentioned in your mail so I'm curious what 
> might be using those.

They were previously exported using -export: on the command line for the 
Microsoft linker. This was the case for a couple other libraries as well. Yeah, 
I forgot to write about that, sorry. :( Been a lot to keep track of, and it 
went away when I cleaned up my notes. 

Can I consider this a review?

/Magnus

> 
> -Alan

RFR(XXS) : 8200180 : fix a typo in run-test framework documentation

[Igor Ignatyev]

http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html
> 3 lines changed: 0 ins; 0 del; 3 mod; 


Hi all,

could you please review this small fix for run-test framework documentation? 
VM_OTIONS was used instead of VM_OPTIONS at several places, the fix is obvious 
s/VM_OTIONS/VM_OPTIONS/ .

webrev: http://cr.openjdk.java.net/~iignatyev//8200180/webrev.00/index.html
JBS: https://bugs.openjdk.java.net/browse/JDK-8200180

Thanks,
-- Igor
 

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Alan Bateman]

On 23/03/2018 13:56, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort to 
using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility on 
Windows, for most of the shared code, we already have proper JNIEXPORT 
decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.
This seems like a great cleanup as the mapfile have always been a pain 
to maintain. Also shines a light on some technical debt too.


handleSocketError in libnio is a surprise, this should not be exported 
and should not have been in the map file.  I suspect the issue is that 
jdk.sctp is missing a function prototype from its header file (it has 
its own handleSocketError in SctpNet.c).


NET_Wait in libnet is another one, I can't tell why this was listed in 
the map file.


I'm also surprised with java.dll exporting handleRead, winHandleRead, 
and handleLSeek. I didn't see them mentioned in your mail so I'm curious 
what might be using those.


-Alan

Re: RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Erik Joelsson]

I have looked at the build changes and they look good.

Will you file followups for each component team to look over their 
exported symbols, at least for the libraries with $(EXPORT_ALL_SYMBOLS)? 
It sure looks like there is some technical debt laying around here.


/Erik


On 2018-03-23 06:56, Magnus Ihse Bursie wrote:
With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort to 
using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility on 
Windows, for most of the shared code, we already have proper JNIEXPORT 
decorations in place.


If we fix the remaining platform-specific files to have proper 
JNIEXPORT tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead 
export all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. 
The only reason Windows is impacted at all, is that some changes 
needed by Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual 
generated code. Unfortunately, this was not possible to fully achieve. 
I do not believe that these changes will have any actual impact on the 
product, though. I will present the differences more in detail further 
down. Those who are not interested can probably skip that.


The patch has passed tier1 testing and is currently running tier2 and 
tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base and 
java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the 
long-standing convention of adding JNICALL. This is a no-op on 
non-Windows platforms, so for most of the changes this is purely 
cosmetic (and possibly adding in robustness, should the function ever 
be used on Windows in the future). I have also followed the stylistic 
convention of putting "JNIEXPORT  JNICALL" on a separate 
line. For some functions, however, this might cause a change in 
calling convention on Windows. Since this can not apply to exported 
functions on Windows (otherwise they would already have had 
JNIEXPORT), I do not think this matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. This 
actually meant that all symbols were exported. It is highly unclear if 
this was known and intended by the original make rule writer. I have 
emulated this by adding the flag $(EXPORT_ALL_SYMBOLS) to these 
libraries. Hopefully, we can remove this flag and fix proper exported 
symbols in the future.


I have run the complete build using COMPARE_BUILD, and made a 
thourough analysis of the differences for Linux and Solaris. All 
native libraries have symbol differences, but most of them are trivial 
and/or harmless. As a result, most libraries have disasm differences 
as well, but these too seem trivial and harmless. The differences in 
symbols that are common to all libraries include:
 * Internal symbols such as __bss_start, _edata, _end and _fini are 
now global. (They are imported as such from the compiler 
libraries/archives, and we have no linker script to override this 
behavior).
 * The versioning tag SUNWprivate_1.1 is not included, and thus 
neither the .gnu.version_d symbol.
 * There are a few differences in the symbol and/or mangling of some 
local functions. I'm not sure what's causing this,

but it's unlikely to have any effect on the product.

Another common source for change in symbols is due to previous 
platform differences. For instance, if we had "JNIEXPORT int JNICALL 
do_foo() { ... }", but do_foo was not in the mapfile, the symbol was 
exported on Windows but not on Linux and Solaris. (Presumable since it 
was not needed there, even though it was compiled for those platforms 
as well.) Now, with the mapfiles gone, do_foo() will be exported on 
all platforms. And contrary, functions that are compiled on all 
platforms, and were exported in mapfiles, but now have gotten an 
JNIEXPORT deco

Re: RFR: JDK-8200174 compare.sh improvements

[Erik Joelsson]

Looks ok.

Note that you can filter what checks are done with command line options. 
I usually add -libs when working specifically with native library 
differences.


/Erik


On 2018-03-23 04:59, Magnus Ihse Bursie wrote:

This patch contains two larger improvements to compare.sh:
* Start by doing the binary comparison. This is often the most 
interesting part, and for interactive sessions, much time is saved is 
this is done first (after the quick sanity check of the file tree).
* Allow not all parts (like the JRE image and sec-bin.zip) to be 
present. This also speeds up tests by making "make jdk-image" as the 
prerequisite for running comparison.


A fix for the hex disasm filter on solstudio is also included.

Bug: https://bugs.openjdk.java.net/browse/JDK-8200174
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200174-compare-sh-improvements/webrev.01


/Magnus

Re: RFR: 8199736: Define WIN32_LEAN_AND_MEAN before including windows.h

[Erik Joelsson]

I think this looks good, but Magnus is currently refactoring the flags 
handling in configure so better get his input as well. (adding build-dev)


/Erik


On 2018-03-23 05:37, Robin Westberg wrote:

Hi Kim & Erik,

Certainly makes sense to define it from the build system, I’ve updated 
the patch accordingly:


Full: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.01/ 

Incremental: 
http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00-01/ 



(Not quite sure if the definition belongs where I put it or a bit 
later where most other windows-specific JVM flags are defined, but 
seemed reasonable to put it close to where it is defined for the JDK 
libraries).


Best regards,
Robin

On 22 Mar 2018, at 16:52, Kim Barrett > wrote:


On Mar 22, 2018, at 10:34 AM, Robin Westberg 
mailto:robin.westb...@oracle.com>> wrote:


Hi all,

Please review the following change that defines WIN32_LEAN_AND_MEAN 
[1] before including windows.h. This marginally improves build 
times, and makes it possible to include winsock2.h.


Issue: https://bugs.openjdk.java.net/browse/JDK-8199736 

Webrev: http://cr.openjdk.java.net/~rwestberg/8199736/webrev.00/ 
 
>

Testing: hs-tier1

Best regards,
Robin

[1] 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa383745%28v=vs.85%29.aspx#faster_builds_with_smaller_header_files 
>


I think the addition of the WIN32_LEAN_AND_MEAN definition should be 
done through the build

system, so that it applies everywhere.

RFR: JDK-8200178 Remove mapfiles for JDK native libraries

[Magnus Ihse Bursie]

With modern compilers, we can use compiler directives (such as 
_attribute__((visibility("default"))), or __declspec(dllexport)) to 
control symbol visibility, directly in the source code. This has 
historically not been present on all compilers, so we had to resort to 
using mapfiles (also known as linker scripts).


This is no longer the case. Now all compilers we use support symbol 
visibility directives, in one form or another. We should start using 
this. Since this has been the only way to control symbol visibility on 
Windows, for most of the shared code, we already have proper JNIEXPORT 
decorations in place.


If we fix the remaining platform-specific files to have proper JNIEXPORT 
tagging, then we can finally get rid of mapfiles.


This fix removed mapfiles for all JDK libraries. It does not touch 
hotspot libraries nor JDK executables; they will have to wait for a 
future fix -- this was complex enough. This change will not have any 
impact on macosx, since we do not use mapfiles there, but instead export 
all symbols. (This is not a good idea, but I'll address that 
separately.) This change will also have a minimal impact on Windows. The 
only reason Windows is impacted at all, is that some changes needed by 
Solaris and Linux were simpler to fix for all platforms.


I have strived for this change to have no impact on the actual generated 
code. Unfortunately, this was not possible to fully achieve. I do not 
believe that these changes will have any actual impact on the product, 
though. I will present the differences more in detail further down. 
Those who are not interested can probably skip that.


The patch has passed tier1 testing and is currently running tier2 and 
tier3. Since the running code is more or less (see caveat below) 
unmodified, I don't expect any testing issues.


Bug: https://bugs.openjdk.java.net/browse/JDK-8200178
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200178-remove-mapfiles/webrev.01


Details on changes:
Most of the source code changes are (unsurprisingly) in java.base and 
java.desktop. Remaining changes are in jdk.crypto.ucrypto, 
jdk.hotspot.agent, jdk.jdi and jdk.jdwp.agent.


Source code changes does almost to 100% consists in decorating an 
exported function with JNIEXPORT. I have also followed the long-standing 
convention of adding JNICALL. This is a no-op on non-Windows platforms, 
so for most of the changes this is purely cosmetic (and possibly adding 
in robustness, should the function ever be used on Windows in the 
future). I have also followed the stylistic convention of putting 
"JNIEXPORT  JNICALL" on a separate line. For some 
functions, however, this might cause a change in calling convention on 
Windows. Since this can not apply to exported functions on Windows 
(otherwise they would already have had JNIEXPORT), I do not think this 
matters anything.


A few libraries did not have a mapfile, on Linux and/or Solaris. This 
actually meant that all symbols were exported. It is highly unclear if 
this was known and intended by the original make rule writer. I have 
emulated this by adding the flag $(EXPORT_ALL_SYMBOLS) to these 
libraries. Hopefully, we can remove this flag and fix proper exported 
symbols in the future.


I have run the complete build using COMPARE_BUILD, and made a thourough 
analysis of the differences for Linux and Solaris. All native libraries 
have symbol differences, but most of them are trivial and/or harmless. 
As a result, most libraries have disasm differences as well, but these 
too seem trivial and harmless. The differences in symbols that are 
common to all libraries include:
 * Internal symbols such as __bss_start, _edata, _end and _fini are now 
global. (They are imported as such from the compiler libraries/archives, 
and we have no linker script to override this behavior).
 * The versioning tag SUNWprivate_1.1 is not included, and thus neither 
the .gnu.version_d symbol.
 * There are a few differences in the symbol and/or mangling of some 
local functions. I'm not sure what's causing this,

but it's unlikely to have any effect on the product.

Another common source for change in symbols is due to previous platform 
differences. For instance, if we had "JNIEXPORT int JNICALL do_foo() { 
... }", but do_foo was not in the mapfile, the symbol was exported on 
Windows but not on Linux and Solaris. (Presumable since it was not 
needed there, even though it was compiled for those platforms as well.) 
Now, with the mapfiles gone, do_foo() will be exported on all platforms. 
And contrary, functions that are compiled on all platforms, and were 
exported in mapfiles, but now have gotten an JNIEXPORT decoration, will 
now be visible even on Windows. (This accounts for half of the noticed 
symbol differences on Windows.) I could have made the JNIEXPORT 
conditional on OS, but I didn't think the mess in source code were worth 
the keeping of binary confidence with the old build.


A third common source for change in symbols is

RFR: JDK-8200174 compare.sh improvements

[Magnus Ihse Bursie]

This patch contains two larger improvements to compare.sh:
* Start by doing the binary comparison. This is often the most 
interesting part, and for interactive sessions, much time is saved is 
this is done first (after the quick sanity check of the file tree).
* Allow not all parts (like the JRE image and sec-bin.zip) to be 
present. This also speeds up tests by making "make jdk-image" as the 
prerequisite for running comparison.


A fix for the hex disasm filter on solstudio is also included.

Bug: https://bugs.openjdk.java.net/browse/JDK-8200174
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8200174-compare-sh-improvements/webrev.01


/Magnus

Re: [NEW BUG]: Configure broken on MIPS

[David Holmes]

On 23/03/2018 7:54 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 09:55, David Holmes wrote:

On 23/03/2018 6:46 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 06:22, David Holmes wrote:

Hi Thomas,

On 23/03/2018 2:55 PM, Thomas Stüfe wrote:

Hi David,

would it not be pragmatic to accept Ao's patch - it looks fine to 
me - since it certainly would not make matters worse. And let 
Magnus follow up with a cleanup change later?


Well I hope Magnus's change is forthcoming. 
It might be some time still. I'm working on a complete overhaul of 
all CFLAGS and LDFLAGS, where this is a part of that picture, but I 
was not planning on addressing just this thing urgently.


So, I think this patch will do for now. It solves the immediate 
problem for MIPS, and I can come back and make a cleaner solution 
later on.


Isn't the best quick fix one that only adds -m64 for x86? I recall  a 
report that arm32 is similarly broken.
Not really, because this is also needed on some other platforms, at 
least s390x, as I recall. (This was the reason it was originally added.)


According to gcc docs there are 4 archs that use m64 and we only care 
about 2 of them:


m64: SPARC Options
m64: S/390 and zSeries Options
m64: RS/6000 and PowerPC Options
m64: i386 and x86-64 Options

But you need to know whether you are dealing with S390 or S390x as m64 
implies zSeries.


Ao will need a sponsor to create a bug etc regardless of which way this 
goes.


My week is over. :)

Cheers,
David


/Magnus



David
-

AFAICS it's as easy to write this only for x86 as it is to exclude 
it for non x86. Honestly I don't know why the Aarch64 patch was done 
the way it was - there must be some subtlety here that I'm not aware 
of.
I think it was just the smallest patch that worked for the aarch64 
platform. I didn't spend time arguing about the fix, since it is 
supposed to be short-lived anyway.


/Magnus

Re: [NEW BUG]: Configure broken on MIPS

[Ao Qi]

>>
>> It might be some time still. I'm working on a complete overhaul of all
>> CFLAGS and LDFLAGS, where this is a part of that picture, but I was not
>> planning on addressing just this thing urgently.
>>
>> So, I think this patch will do for now. It solves the immediate problem
>> for MIPS, and I can come back and make a cleaner solution later on.
>
>
> Isn't the best quick fix one that only adds -m64 for x86? I recall  a report
> that arm32 is similarly broken.

I had a look at man gcc. It seems that at least -m64 is for S/390 and
zSeries, SPARC , RS/6000 and PowerPC, and x86.

>
> David
> -
>
>

Re: [NEW BUG]: Configure broken on MIPS

[Magnus Ihse Bursie]

On 2018-03-23 09:55, David Holmes wrote:

On 23/03/2018 6:46 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 06:22, David Holmes wrote:

Hi Thomas,

On 23/03/2018 2:55 PM, Thomas Stüfe wrote:

Hi David,

would it not be pragmatic to accept Ao's patch - it looks fine to 
me - since it certainly would not make matters worse. And let 
Magnus follow up with a cleanup change later?


Well I hope Magnus's change is forthcoming. 
It might be some time still. I'm working on a complete overhaul of 
all CFLAGS and LDFLAGS, where this is a part of that picture, but I 
was not planning on addressing just this thing urgently.


So, I think this patch will do for now. It solves the immediate 
problem for MIPS, and I can come back and make a cleaner solution 
later on.


Isn't the best quick fix one that only adds -m64 for x86? I recall  a 
report that arm32 is similarly broken.
Not really, because this is also needed on some other platforms, at 
least s390x, as I recall. (This was the reason it was originally added.)


/Magnus



David
-

AFAICS it's as easy to write this only for x86 as it is to exclude 
it for non x86. Honestly I don't know why the Aarch64 patch was done 
the way it was - there must be some subtlety here that I'm not aware 
of.
I think it was just the smallest patch that worked for the aarch64 
platform. I didn't spend time arguing about the fix, since it is 
supposed to be short-lived anyway.


/Magnus

Re: [NEW BUG]: Configure broken on MIPS

[David Holmes]

On 23/03/2018 6:46 PM, Magnus Ihse Bursie wrote:


On 2018-03-23 06:22, David Holmes wrote:

Hi Thomas,

On 23/03/2018 2:55 PM, Thomas Stüfe wrote:

Hi David,

would it not be pragmatic to accept Ao's patch - it looks fine to me 
- since it certainly would not make matters worse. And let Magnus 
follow up with a cleanup change later?


Well I hope Magnus's change is forthcoming. 
It might be some time still. I'm working on a complete overhaul of all 
CFLAGS and LDFLAGS, where this is a part of that picture, but I was not 
planning on addressing just this thing urgently.


So, I think this patch will do for now. It solves the immediate problem 
for MIPS, and I can come back and make a cleaner solution later on.


Isn't the best quick fix one that only adds -m64 for x86? I recall  a 
report that arm32 is similarly broken.


David
-

AFAICS it's as easy to write this only for x86 as it is to exclude it 
for non x86. Honestly I don't know why the Aarch64 patch was done the 
way it was - there must be some subtlety here that I'm not aware of.
I think it was just the smallest patch that worked for the aarch64 
platform. I didn't spend time arguing about the fix, since it is 
supposed to be short-lived anyway.


/Magnus

Re: [NEW BUG]: Configure broken on MIPS

[Magnus Ihse Bursie]

On 2018-03-23 06:22, David Holmes wrote:

Hi Thomas,

On 23/03/2018 2:55 PM, Thomas Stüfe wrote:

Hi David,

would it not be pragmatic to accept Ao's patch - it looks fine to me 
- since it certainly would not make matters worse. And let Magnus 
follow up with a cleanup change later?


Well I hope Magnus's change is forthcoming. 
It might be some time still. I'm working on a complete overhaul of all 
CFLAGS and LDFLAGS, where this is a part of that picture, but I was not 
planning on addressing just this thing urgently.


So, I think this patch will do for now. It solves the immediate problem 
for MIPS, and I can come back and make a cleaner solution later on.


AFAICS it's as easy to write this only for x86 as it is to exclude it 
for non x86. Honestly I don't know why the Aarch64 patch was done the 
way it was - there must be some subtlety here that I'm not aware of.
I think it was just the smallest patch that worked for the aarch64 
platform. I didn't spend time arguing about the fix, since it is 
supposed to be short-lived anyway.


/Magnus