RFR: JDK-8199749: Debug symbols are not copied to exploded image on Mac

[Erik Joelsson]

Debug symbols are by default external and the idea is that we copy them 
to the exploded image together with the native libraries, to have them 
available for debugging. This logic fails on Windows and Mac due to some 
intricate make dependency complications. On Windows this fails on the 
initial build already, on Mac the problem only appears if you make a 
change to a source file and rebuild incrementally. On both, if you 
rebuild again, the files get copied.


On Solaris and Linux we symlink these files instead of copying, so after 
the first build, the symlinks do not need to be recreated, which is why 
this isn't an issue there.


The underlying cause for this is that we are trying to trick make into 
correctly tracking dependencies when a rule has multiple targets. The 
link rule creates both the native library as well as the debuginfo 
files. A rule like that is always causing trouble in some way but 
various workarounds exist. Currently we work around it by adding this 
declaration:


$$($1_DEBUGINFO_FILES): $$($1_TARGET)

This trips up make if DEBUGINFO_FILES already exists, since there is no 
recipe, make will not consider DEBUGINFO to have been updated just 
because TARGET was rebuilt. If we add a recipe, make will consider 
DEBUGINFO to have changed. The obvious recipe is a simple touch on 
DEBUGINFO. That way we also guarantee that the DEBUGINFO is newer than 
TARGET so the rule doesn't get executed again on the next run.


This whole type of workaround for dealing with multiple files created by 
the same rule has a flaw. If something external deletes a DEBUGINFO 
file, it would currently not be recreated. In the new solution it would 
be touched as an empty file, which is even worse. To fix this, I've 
added a parse time check on the DEBUGINFO files and if either of them 
are gone, the TARGET is explicitly deleted. This forces a rebuild of 
both TARGET and DEBUGINFO.


While at it, I noticed that the import library on Windows suffers from 
the same problem so applied the same fix there.


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

Webrev: http://cr.openjdk.java.net/~erikj/8199749/webrev.01/index.html

/Erik

Re: RFR: JDK-8199745: JDK-8199668 introduced a build race on macosx

[Tim Bell]

Erik:


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

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


Looks good.

Tim

RFR: JDK-8199745: JDK-8199668 introduced a build race on macosx

[Erik Joelsson]

Hello,

JDK-8199668 introduced a race in the macosx build. The library 
libawt_lawt links against libosxapp. Before the change, the declaration 
of libosxapp happened before libawt_lawt, but in that change the order 
reversed. This makes the dependency declaration between them no longer 
functioning.


To fix this I changed the relevant instances of $(BUILD_LIBFOO) to 
$(call FindLib, module, foo). The problem with using BUILD_LIBFOO is 
that it is only defined in the same makefile context and after the call 
to SetupNativeCompilation for it. The FindLib macro can be used anywhere.


While fixing this I found more similar broken dependency declarations 
and fixed those as well including some cleanup. I did not replace all 
instances. Maybe we should, but I assume this will get handled 
differently with the further cleanups in this area.


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

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

/Erik

Re: RFR: JDK-8199682 Clean up building the saproc library

[Magnus Ihse Bursie]

Hi Sundar,

I almost missed your mail, since you removed both me and build-dev from the cc 
list...

> 16 mars 2018 kl. 06:14 skrev Sundararajan Athijegannathan 
> :
> 
> Renaming sawindbg as saproc sounds odd. For Linux, Solaris/Unix, we either 
> use /proc & libproc, so calling saproc for those makes sense. But Windows? We 
> have a separate debugger class to load platform specific native library. What 
> is the reason for uniform naming?

This is the only library in the JDK that has a different name on different 
platform. This clashes with the design of the build system, and requires a 
clunky workaround. For the upcoming changes in the build system, this goes from 
an annoyance to a blocker.

No other components have their names based on the OS functionality they use, 
even if they use vastly different APIs on different platforms; rather they are 
named after the services they provide to the JDK.

My assumption was that ”saproc” meant ”serviceability agent process handling”, 
and that this was a reasonable name for all platforms. Also, the source code 
for all platforms reside in the ”libsaproc” directory, which is consistent with 
the JDK standard for matching source code to native library.

But if you believe this is an inappropriate name, let’s work together to find a 
name that works for all platforms. This of course will lead to new names for 
the current libsaproc.* libraries, and the source code directories.

/Magnus

> 
> -Sundar
> 
> On 16/03/18, 12:19 AM, Magnus Ihse Bursie wrote:
>> 
>> 
>> On 2018-03-15 19:39, Erik Joelsson wrote:
>>> Looks good to me.
>>> 
>>> The removed source files, are those some kind of tests?
>> I don't really know; they have been excluded from the build for all time. My 
>> guess is that the Bsd* stuff is, like in the case of the sound libraries, 
>> bsd-based stuff that arrived with the mac port (but disabled). The test.c is 
>> a trivial main() method which looks more like a left-over adhoc testing from 
>> the initial developer. Perhaps someone wants to turn it into a proper test, 
>> but it seems like it's not much even to start with. (And hopefully we have 
>> much better real test coverage of this now.)
>> 
>> /Magnus
>>> 
>>> /Erik
>>> 
>>> 
>>> On 2018-03-15 11:22, Magnus Ihse Bursie wrote:
 The saproc library has historically been built in quite odd ways on almost 
 all platforms. When the old build system was converted, this was not 
 changed.
 
 However, now the time has come to streamline this and build this library 
 just as any other.
 
 The most visible change, perhaps, is that the library is now named saproc 
 on all platforms, even Windows. Other changes include:
 * Don't set flags that is already set by the default flags.
 * Don't set flags that do not have anny effect.
 * Don't subst away the WIN32_LEAN_AND_MEAN definition, it's perfectly okay 
 to have it.
 * Don't set CXX linker on solaris -- this was not needed so no reason to 
 do it.
 * Cleaned up some old hooks for closed code that is no longer needed.
 
 I have verified this using COMPARE_BUILD. This shows only the expected 
 differences:
 * On all platforms: class file changes for WindbgDebuggerLocal.java.
 * On solaris: some minor symbol differences, since the linker now uses C 
 framework functions instead of C++. (And with symbol changes always comes 
 disasm changes.)
 * On linux: a binary difference for libsaproc.so, but no 
 size/symbol/deps/disasm change.
 * On macosx: no changes at all.
 * On windows: sawindbg.dll is renamed to saproc.dll. When I made a manual 
 comparison between the two files, I found no significant differences.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8199682
 WebRev: 
 http://cr.openjdk.java.net/~ihse/JDK-8199682-clean-up-saproc/webrev.01
 
 /Magnus
 
>>> 
>> 

Re: [OpenJDK 2D-Dev] RFR: JDK-8071469 Cleanup include and exclude of sound native libraries after source code restructure

[Alex Menkov]

On 03/15/2018 13:09, Magnus Ihse Bursie wrote:



15 mars 2018 kl. 20:13 skrev Phil Race :



As far as I know the split was made to dynamically load ALSA/DirectSound stuff


Yes, I think it is something like that for Linux
ie if at runtime a dependent-but-not-essential .so was not
installed it was not fatal. I don't know to what extent this is no longer a
possible issue, or one that matters.


I have not heard of any mainstream Linux distro in years that was lacking ALSA.

If ALSA was not present, will the libraries fall back to OSS, or will there be 
just no sound available?


No sound.
OSS support was dropped many years ago (IIRC in jdk7)


In any case, I think that whatever Linux distros we're targeting as supported, 
ALSA will be present.

Alex, did I understand you correctly that in any case, a separate Windows 
library is always unnecessary, since we can rely on DirectAudio always being 
present in our supported versions of Windows?


Yes, that's right.
Windows always has DirectSound pre-installed and its version is greater 
than required (IIRC javasoundds requires DirectX 5).


For now failure of libjsound loading is fatal (see 
com.sun.media.sound.Platform.loadLibraries()), loading of extra libs is 
non-fatal.
I believe libjsound loading failure should be made non-fatal, then all 
the functionality will remain the same as we have now.


--alex



/Magnus



-phil.


On 03/15/2018 12:06 PM, Alex Menkov wrote:



On 03/15/2018 11:44, Magnus Ihse Bursie wrote:

On 2018-03-15 18:23, Phil Race wrote:
I wondered if that might be the case since it was a "BSD" port .. using X11 ..

Maybe we should be getting rid of them ?

I agree, we should delete them. I just shuffled them around in the hope that 
they would be useful for a potential future bsd port, but if/when that happens, 
we can dig them out from mercurial.

A short explanation of how the files moved. The sound library is apparently 
composed of either a single library (solaris and macosx) or two libraries 
(linux and windows). Two building blocks, MIDI + ports and DirectAudio is used 
for all platforms, but they go into either the main library (libjsound) or the 
helper library.

For Windows, MIDI+Ports go into libjsound, and DirectAudio go into libjsoundds. 
On Linux, MIDI+Ports and DirectAudio go into libjsoundalsa. On Macosx and 
Solaris, MIDI+Ports and DirectAudio go into the main libjsound.

I have no idea why this split is necessary, but this is how the libraries de 
facto is compiled, and the code needs to match that. If it would be possible to 
move libjsoundds and libjsoundalsa into libjsound directly, things would be 
greatly simplified.


As far as I know the split was made to dynamically load ALSA/DirectSound stuff. 
If it's not available (or old unsupported version is installed), libjsound 
stuff continues to work (in pre-OpenJDK libjsound supported WaveIn/WaveOut on 
Windows and OSS on Linux).
For now Windows (DirectSound) libjsoundds stuff can be merged into libjsound, 
but I'm not sure we can rely on ALSA is always available on Linux (but most 
likely if ALSA is not available, libjsound does not provide any functionality, 
so I suppose libjsoundalsa stuff can be moved to libjsound as well)

--alex



/Magnus



-phil.


On 03/15/2018 10:21 AM, Erik Joelsson wrote:
Digging a bit, those files came with the initial Macosx support. It doesn't 
look like they were ever used.

/Erik



On 2018-03-15 09:53, Phil Race wrote:
It is very hard to follow all the moved around files, but one thing
that sticks out is there is a "bsd" directory created and I can't
work out how the files in there are used.
If they are for a BSD port of OpenJDK where is rest of the support for that ?


On 03/15/2018 07:20 AM, Erik Joelsson wrote:
Looks good to me. I tried cleaning this up before but failed to find a 
reasonable split, but this seems like a good split between common and library 
specific.

/Erik



On 2018-03-14 18:12, Magnus Ihse Bursie wrote:
I forgot to add the client mailing lists as recipients. Sorry. (Not sure if "sounds" belong to 
"awt" or "2d".)


In fact, there is a sound-specific list, which I've added.

-phil.


/Magnus


On 2018-03-15 02:07, Magnus Ihse Bursie wrote:
 From the bug description:

Moving this to a separate bug from JDK-8055190. In SoundLibraries.gmk, the 
source code splitting is not complete. The directory libjsound is used to build 
not only libjsound but libjsoundalsa and libjsoundds, and thus needs a complex 
include/exclude system like before.

I have tested this using COMPARE_BUILD. Windows and solaris are completely 
clean. On macosx, there's a binary diff (but nothing else) on libjsound.dylib. 
On linux, some offset seems to have changed, which caused a slight change in 
disass and fulldump for libjsound.so. I'm not quite sure what's causing it, but 
I'm convinced it's harmless.

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

Re: RFR: build pragma error with gcc 4.4.7

[Michal Vala]

On 03/16/2018 12:36 PM, Magnus Ihse Bursie wrote:


On 2018-03-16 12:05, David Holmes wrote:

Hi Michal,

On 16/03/2018 8:48 PM, Michal Vala wrote:

Hi,

I've been trying to build latest jdk with gcc 4.4.7 and I hit compile error 
due to pragma used in function:
I don't think gcc 4.4.7 is likely to work at all. Configure will complain (but 
continue) if you use a gcc prior to 4.7 (very recently raised to 4.8).


You can try getting past this error, but you are likely to hit more issues down 
the road.


Do you have any specific reasons for using such an old compiler?


Yes, I'm targeting RHEL6 where 4.4.7 is base gcc. With patch I've posted I'm 
able to compile. Configure is complaining with warning, but otherwise it's ok. 
Few more warnings during the build but no errors. We'd like to keep it 
'compilable' in RHEL6 with code as close as possible to upstream.


--
Michal Vala
OpenJDK QE
Red Hat Czech

Re: RFR: JDK-8199681 Remove boilerplate code from creating native jtreg tests

[David Holmes]

On 16/03/2018 9:43 PM, Magnus Ihse Bursie wrote:

On 2018-03-16 03:48, David Holmes wrote:

Hi Magnus,

Removing boilerplate is good but the exclude mechanism seems rather 
awkward compared to the previous include. It's much more obvious when 
writing a platform specific native test to include it for that 
platform, rather than exclude it for all other platforms. You run the 
risk of having excludes spread all over the place, or not knowing 
where you should put them. I thought the opt-in approach of just 
adding the test directory to the list of native test directories was 
simple and worked well. (The need to add the extra link instructions 
wasn't good though, :) )


Previously you had to write like:
ifeq ($(OPENJDK_TARGET_OS), windows)
SRC += mytest
endif

Now you have to write
ifneq ($(OPENJDK_TARGET_OS), windows)
EXCLUDE += mytest
endif

Seriously, that's not much harder.


True. It just doesn't look that clean in the actual file.

But the latter has the extremely important benefit that for all 
multi-platform tests (which comprise the absolute majority), you don't 
have to modify the makefiles at all.


True.

Also having to exclude on the file level is more awkward than using 
the directory level.

I can of course change that. I chose the file level due to these reasons:
* We have a requirement of filename uniqueness across all tests (since 
they compile to object files in a single directory); however we do not 
have a requirement of directory name uniqueness.


Given we used to include directories it seems natural to me that we now 
exclude directories. Though I suppose files does give more fine grained 
control if we were to need it.


* Basing this on filenames allows you to single out individual tests 
that are, for logical reasons, grouped together in a single directory.

* And it was easier to implement in make scripts. :-)

As a suggestion, I can add a PATTERN_EXCLUDE. This way it will work 
about the same as the Setup*Compilation functions, were you can exclude 
based on file name or pattern matching. Do you want me to do that? I 
think with the current number of excludes, it's not really worth it, but 
if it is important to you, I can fix it.


Let's just see how it goes, as you've already pushed it.

Thanks,
David




49 BUILD_HOTSPOT_JTREG_IMAGE_DIR := $(TEST_IMAGE_DIR)/hotspot/jtreg

I thought you'd changed something here then noticed that we end up doing:

 94 DEST := $(TEST_IMAGE_DIR)/hotspot/jtreg/native, \

and BUILD_HOTSPOT_JTREG_IMAGE_DIR is actually unused.


*duh*

I have already pushed the patch. I will fix this in a follow-up bug. Let 
me know if you want to have a directory-based or pattern- based 
exclusion mechanism in that as well.


/Magnus



Thanks,
David

On 15/03/2018 11:01 PM, Magnus Ihse Bursie wrote:
There's currently a lot of boilerplate code needed to setup a new 
native jtreg test. This was never the way it was intended to work -- 
the idea was that you basically should just add the file and then 
things should work automatically, unless you had special requirements.


This patch will make it so once more.

I have tested this using COMPARE_BUILD. I get some spurious binary 
differences on macosx. I can't really say why, but then again, we 
have never verified the test image by a clean "back-to-back" null 
comparison either, so I'm not worried. Apart from that, it looks green.


Bug: https://bugs.openjdk.java.net/browse/JDK-8199681
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8199681-remote-native-test-boilerplate/webrev.01 



/Magnus

Re: RFR: JDK-8199682 Clean up building the saproc library

[David Holmes]

On 16/03/2018 9:49 PM, Magnus Ihse Bursie wrote:

On 2018-03-16 04:13, David Holmes wrote:

Hi Magnus,

Overall this seems okay.

Thanks!



On 16/03/2018 4:22 AM, Magnus Ihse Bursie wrote:
The saproc library has historically been built in quite odd ways on 
almost all platforms. When the old build system was converted, this 
was not changed.


However, now the time has come to streamline this and build this 
library just as any other.


The most visible change, perhaps, is that the library is now named 
saproc on all platforms, even Windows. Other changes include:


That could have repercussions elsewhere. sawindbg.dll is probably a 
well known name for deployment systems.
You mean other classes than WindbgDebuggerLocal.java, out in the wild, 
might load sawindbg.dll directly and call into it? If they do so, they 
must also be prepared that this is not an exported interface and can 
change at any time.


No I mean deployment systems, like an upstream RPM manager, or Oracle's 
own installer process, may know the name of the file and have to be 
modified if the name changes.


Though as Sundar said "proc" isn't really the right name.

David
-




* Don't set flags that is already set by the default flags.
* Don't set flags that do not have anny effect.
* Don't subst away the WIN32_LEAN_AND_MEAN definition, it's perfectly 
okay to have it.
* Don't set CXX linker on solaris -- this was not needed so no reason 
to do it.

* Cleaned up some old hooks for closed code that is no longer needed.


Right - we could have deleted that when our ARM ports went open.

I have verified this using COMPARE_BUILD. This shows only the 
expected differences:

* On all platforms: class file changes for WindbgDebuggerLocal.java.
* On solaris: some minor symbol differences, since the linker now 
uses C framework functions instead of C++. (And with symbol changes 
always comes disasm changes.)
* On linux: a binary difference for libsaproc.so, but no 
size/symbol/deps/disasm change.

* On macosx: no changes at all.
* On windows: sawindbg.dll is renamed to saproc.dll. When I made a 
manual comparison between the two files, I found no significant 
differences.


Bug: https://bugs.openjdk.java.net/browse/JDK-8199682
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8199682-clean-up-saproc/webrev.01


The deleted OSX files seem okay. This just seems like another case 
where the original port copied every Linux file across to the bsd 
directory.


Not sure about the Solaris saproc_audit.cpp or the test.c files ??
I don't know either. :) As I said to Erik, the test files looked like 
stupid adhoc testing just left in place. The saproc_audit.cpp looks 
legit, but has not been compiled for years. Someone must have "removed" 
the file by excluding it from compilation, rather than deleting it. 
Could have happened back in the bad old days when "solaris" didn't mean 
solaris but "unix", and nobody understood the consequences of deleting 
files there.


As always, the file is still in the repository, if someone wants to 
revive it.


/Magnus



Thanks,
David


/Magnus

Re: RFR: JDK-8199682 Clean up building the saproc library

[Magnus Ihse Bursie]

On 2018-03-16 04:13, David Holmes wrote:

Hi Magnus,

Overall this seems okay.

Thanks!



On 16/03/2018 4:22 AM, Magnus Ihse Bursie wrote:
The saproc library has historically been built in quite odd ways on 
almost all platforms. When the old build system was converted, this 
was not changed.


However, now the time has come to streamline this and build this 
library just as any other.


The most visible change, perhaps, is that the library is now named 
saproc on all platforms, even Windows. Other changes include:


That could have repercussions elsewhere. sawindbg.dll is probably a 
well known name for deployment systems.
You mean other classes than WindbgDebuggerLocal.java, out in the wild, 
might load sawindbg.dll directly and call into it? If they do so, they 
must also be prepared that this is not an exported interface and can 
change at any time.



* Don't set flags that is already set by the default flags.
* Don't set flags that do not have anny effect.
* Don't subst away the WIN32_LEAN_AND_MEAN definition, it's perfectly 
okay to have it.
* Don't set CXX linker on solaris -- this was not needed so no reason 
to do it.

* Cleaned up some old hooks for closed code that is no longer needed.


Right - we could have deleted that when our ARM ports went open.

I have verified this using COMPARE_BUILD. This shows only the 
expected differences:

* On all platforms: class file changes for WindbgDebuggerLocal.java.
* On solaris: some minor symbol differences, since the linker now 
uses C framework functions instead of C++. (And with symbol changes 
always comes disasm changes.)
* On linux: a binary difference for libsaproc.so, but no 
size/symbol/deps/disasm change.

* On macosx: no changes at all.
* On windows: sawindbg.dll is renamed to saproc.dll. When I made a 
manual comparison between the two files, I found no significant 
differences.


Bug: https://bugs.openjdk.java.net/browse/JDK-8199682
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8199682-clean-up-saproc/webrev.01


The deleted OSX files seem okay. This just seems like another case 
where the original port copied every Linux file across to the bsd 
directory.


Not sure about the Solaris saproc_audit.cpp or the test.c files ??
I don't know either. :) As I said to Erik, the test files looked like 
stupid adhoc testing just left in place. The saproc_audit.cpp looks 
legit, but has not been compiled for years. Someone must have "removed" 
the file by excluding it from compilation, rather than deleting it. 
Could have happened back in the bad old days when "solaris" didn't mean 
solaris but "unix", and nobody understood the consequences of deleting 
files there.


As always, the file is still in the repository, if someone wants to 
revive it.


/Magnus



Thanks,
David


/Magnus

Re: RFR: JDK-8199681 Remove boilerplate code from creating native jtreg tests

[Magnus Ihse Bursie]

On 2018-03-16 03:48, David Holmes wrote:

Hi Magnus,

Removing boilerplate is good but the exclude mechanism seems rather 
awkward compared to the previous include. It's much more obvious when 
writing a platform specific native test to include it for that 
platform, rather than exclude it for all other platforms. You run the 
risk of having excludes spread all over the place, or not knowing 
where you should put them. I thought the opt-in approach of just 
adding the test directory to the list of native test directories was 
simple and worked well. (The need to add the extra link instructions 
wasn't good though, :) )


Previously you had to write like:
ifeq ($(OPENJDK_TARGET_OS), windows)
SRC += mytest
endif

Now you have to write
ifneq ($(OPENJDK_TARGET_OS), windows)
EXCLUDE += mytest
endif

Seriously, that's not much harder.

But the latter has the extremely important benefit that for all 
multi-platform tests (which comprise the absolute majority), you don't 
have to modify the makefiles at all.


Also having to exclude on the file level is more awkward than using 
the directory level.

I can of course change that. I chose the file level due to these reasons:
* We have a requirement of filename uniqueness across all tests (since 
they compile to object files in a single directory); however we do not 
have a requirement of directory name uniqueness.
* Basing this on filenames allows you to single out individual tests 
that are, for logical reasons, grouped together in a single directory.

* And it was easier to implement in make scripts. :-)

As a suggestion, I can add a PATTERN_EXCLUDE. This way it will work 
about the same as the Setup*Compilation functions, were you can exclude 
based on file name or pattern matching. Do you want me to do that? I 
think with the current number of excludes, it's not really worth it, but 
if it is important to you, I can fix it.





49 BUILD_HOTSPOT_JTREG_IMAGE_DIR := $(TEST_IMAGE_DIR)/hotspot/jtreg

I thought you'd changed something here then noticed that we end up doing:

 94 DEST := $(TEST_IMAGE_DIR)/hotspot/jtreg/native, \

and BUILD_HOTSPOT_JTREG_IMAGE_DIR is actually unused.


*duh*

I have already pushed the patch. I will fix this in a follow-up bug. Let 
me know if you want to have a directory-based or pattern- based 
exclusion mechanism in that as well.


/Magnus



Thanks,
David

On 15/03/2018 11:01 PM, Magnus Ihse Bursie wrote:
There's currently a lot of boilerplate code needed to setup a new 
native jtreg test. This was never the way it was intended to work -- 
the idea was that you basically should just add the file and then 
things should work automatically, unless you had special requirements.


This patch will make it so once more.

I have tested this using COMPARE_BUILD. I get some spurious binary 
differences on macosx. I can't really say why, but then again, we 
have never verified the test image by a clean "back-to-back" null 
comparison either, so I'm not worried. Apart from that, it looks green.


Bug: https://bugs.openjdk.java.net/browse/JDK-8199681
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8199681-remote-native-test-boilerplate/webrev.01 



/Magnus

Re: RFR: build pragma error with gcc 4.4.7

[Magnus Ihse Bursie]

On 2018-03-16 12:05, David Holmes wrote:

Hi Michal,

On 16/03/2018 8:48 PM, Michal Vala wrote:

Hi,

I've been trying to build latest jdk with gcc 4.4.7 and I hit compile 
error due to pragma used in function:
I don't think gcc 4.4.7 is likely to work at all. Configure will 
complain (but continue) if you use a gcc prior to 4.7 (very recently 
raised to 4.8).


You can try getting past this error, but you are likely to hit more 
issues down the road.


Do you have any specific reasons for using such an old compiler?

/Magnus


That's a very old gcc. Our "official" version is 4.9.2 but we're 
working on getting gcc 7.x working as well. This code causes no 
problem on 4.9.2+ so to make any change we'd have to know it will 
continue to work on later versions.


Also a google search indicates the "pragma diagnostic push" and pop 
weren't added until gcc 4.6 ??


David
-

/mnt/ramdisk/openjdk/src/hotspot/os/linux/os_linux.inline.hpp:103: 
error: #pragma GCC diagnostic not allowed inside functions



I'm sending little patch that fixes the issue by wrapping whole 
function. I've also created a macro for ignoring deprecated 
declaration inside compilerWarnings.hpp to line up with others.


Can someone please review? If it's ok, I would also need a sponsor.


diff -r 422615764e12 src/hotspot/os/linux/os_linux.inline.hpp
--- a/src/hotspot/os/linux/os_linux.inline.hpp    Thu Mar 15 14:54:10 
2018 -0700
+++ b/src/hotspot/os/linux/os_linux.inline.hpp    Fri Mar 16 10:50:24 
2018 +0100

@@ -96,13 +96,12 @@
    return ::ftruncate64(fd, length);
  }

-inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
-{
  // readdir_r has been deprecated since glibc 2.24.
  // See https://sourceware.org/bugzilla/show_bug.cgi?id=19056 for 
more details.

-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
+PRAGMA_DIAG_PUSH
+PRAGMA_DEPRECATED_IGNORED
+inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
+{
    dirent* p;
    int status;
    assert(dirp != NULL, "just checking");
@@ -114,11 +113,11 @@
    if((status = ::readdir_r(dirp, dbuf, )) != 0) {
  errno = status;
  return NULL;
-  } else
+  } else {
  return p;
-
-#pragma GCC diagnostic pop
+  }
  }
+PRAGMA_DIAG_POP

  inline int os::closedir(DIR *dirp) {
    assert(dirp != NULL, "argument is NULL");
diff -r 422615764e12 src/hotspot/share/utilities/compilerWarnings.hpp
--- a/src/hotspot/share/utilities/compilerWarnings.hpp    Thu Mar 15 
14:54:10 2018 -0700
+++ b/src/hotspot/share/utilities/compilerWarnings.hpp    Fri Mar 16 
10:50:24 2018 +0100

@@ -48,6 +48,7 @@
  #define PRAGMA_FORMAT_NONLITERAL_IGNORED _Pragma("GCC diagnostic 
ignored \"-Wformat-nonliteral\"") \
   _Pragma("GCC diagnostic 
ignored \"-Wformat-security\"")
  #define PRAGMA_FORMAT_IGNORED _Pragma("GCC diagnostic ignored 
\"-Wformat\"")
+#define PRAGMA_DEPRECATED_IGNORED _Pragma("GCC diagnostic ignored 
\"-Wdeprecated-declarations\"")


  #if defined(__clang_major__) && \
    (__clang_major__ >= 4 || \


Thanks!

Re: RFR: build pragma error with gcc 4.4.7

[David Holmes]

Hi Michal,

On 16/03/2018 8:48 PM, Michal Vala wrote:

Hi,

I've been trying to build latest jdk with gcc 4.4.7 and I hit compile 
error due to pragma used in function:


That's a very old gcc. Our "official" version is 4.9.2 but we're working 
on getting gcc 7.x working as well. This code causes no problem on 
4.9.2+ so to make any change we'd have to know it will continue to work 
on later versions.


Also a google search indicates the "pragma diagnostic push" and pop 
weren't added until gcc 4.6 ??


David
-

/mnt/ramdisk/openjdk/src/hotspot/os/linux/os_linux.inline.hpp:103: 
error: #pragma GCC diagnostic not allowed inside functions



I'm sending little patch that fixes the issue by wrapping whole 
function. I've also created a macro for ignoring deprecated declaration 
inside compilerWarnings.hpp to line up with others.


Can someone please review? If it's ok, I would also need a sponsor.


diff -r 422615764e12 src/hotspot/os/linux/os_linux.inline.hpp
--- a/src/hotspot/os/linux/os_linux.inline.hpp    Thu Mar 15 14:54:10 
2018 -0700
+++ b/src/hotspot/os/linux/os_linux.inline.hpp    Fri Mar 16 10:50:24 
2018 +0100

@@ -96,13 +96,12 @@
    return ::ftruncate64(fd, length);
  }

-inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
-{
  // readdir_r has been deprecated since glibc 2.24.
  // See https://sourceware.org/bugzilla/show_bug.cgi?id=19056 for more 
details.

-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
+PRAGMA_DIAG_PUSH
+PRAGMA_DEPRECATED_IGNORED
+inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
+{
    dirent* p;
    int status;
    assert(dirp != NULL, "just checking");
@@ -114,11 +113,11 @@
    if((status = ::readdir_r(dirp, dbuf, )) != 0) {
  errno = status;
  return NULL;
-  } else
+  } else {
  return p;
-
-#pragma GCC diagnostic pop
+  }
  }
+PRAGMA_DIAG_POP

  inline int os::closedir(DIR *dirp) {
    assert(dirp != NULL, "argument is NULL");
diff -r 422615764e12 src/hotspot/share/utilities/compilerWarnings.hpp
--- a/src/hotspot/share/utilities/compilerWarnings.hpp    Thu Mar 15 
14:54:10 2018 -0700
+++ b/src/hotspot/share/utilities/compilerWarnings.hpp    Fri Mar 16 
10:50:24 2018 +0100

@@ -48,6 +48,7 @@
  #define PRAGMA_FORMAT_NONLITERAL_IGNORED _Pragma("GCC diagnostic 
ignored \"-Wformat-nonliteral\"") \
   _Pragma("GCC diagnostic 
ignored \"-Wformat-security\"")
  #define PRAGMA_FORMAT_IGNORED _Pragma("GCC diagnostic ignored 
\"-Wformat\"")
+#define PRAGMA_DEPRECATED_IGNORED _Pragma("GCC diagnostic ignored 
\"-Wdeprecated-declarations\"")


  #if defined(__clang_major__) && \
    (__clang_major__ >= 4 || \


Thanks!

RFR: build pragma error with gcc 4.4.7

[Michal Vala]

Hi,

I've been trying to build latest jdk with gcc 4.4.7 and I hit compile error due 
to pragma used in function:


/mnt/ramdisk/openjdk/src/hotspot/os/linux/os_linux.inline.hpp:103: error: 
#pragma GCC diagnostic not allowed inside functions



I'm sending little patch that fixes the issue by wrapping whole function. I've 
also created a macro for ignoring deprecated declaration inside 
compilerWarnings.hpp to line up with others.


Can someone please review? If it's ok, I would also need a sponsor.


diff -r 422615764e12 src/hotspot/os/linux/os_linux.inline.hpp
--- a/src/hotspot/os/linux/os_linux.inline.hpp  Thu Mar 15 14:54:10 2018 -0700
+++ b/src/hotspot/os/linux/os_linux.inline.hpp  Fri Mar 16 10:50:24 2018 +0100
@@ -96,13 +96,12 @@
   return ::ftruncate64(fd, length);
 }

-inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
-{
 // readdir_r has been deprecated since glibc 2.24.
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19056 for more details.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
+PRAGMA_DIAG_PUSH
+PRAGMA_DEPRECATED_IGNORED
+inline struct dirent* os::readdir(DIR* dirp, dirent *dbuf)
+{
   dirent* p;
   int status;
   assert(dirp != NULL, "just checking");
@@ -114,11 +113,11 @@
   if((status = ::readdir_r(dirp, dbuf, )) != 0) {
 errno = status;
 return NULL;
-  } else
+  } else {
 return p;
-
-#pragma GCC diagnostic pop
+  }
 }
+PRAGMA_DIAG_POP

 inline int os::closedir(DIR *dirp) {
   assert(dirp != NULL, "argument is NULL");
diff -r 422615764e12 src/hotspot/share/utilities/compilerWarnings.hpp
--- a/src/hotspot/share/utilities/compilerWarnings.hpp	Thu Mar 15 14:54:10 2018 
-0700
+++ b/src/hotspot/share/utilities/compilerWarnings.hpp	Fri Mar 16 10:50:24 2018 
+0100

@@ -48,6 +48,7 @@
 #define PRAGMA_FORMAT_NONLITERAL_IGNORED _Pragma("GCC diagnostic ignored 
\"-Wformat-nonliteral\"") \
  _Pragma("GCC diagnostic ignored 
\"-Wformat-security\"")

 #define PRAGMA_FORMAT_IGNORED _Pragma("GCC diagnostic ignored \"-Wformat\"")
+#define PRAGMA_DEPRECATED_IGNORED _Pragma("GCC diagnostic ignored 
\"-Wdeprecated-declarations\"")


 #if defined(__clang_major__) && \
   (__clang_major__ >= 4 || \


Thanks!

--
Michal Vala
OpenJDK QE
Red Hat Czech