Hi Magnus,

I've uploaded new webrev:
  http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.05/

If we do implement this flag, it should work as expected. If you believe this 
is too hard to get right, I'm alright with fixing this as a separate bug.

Okay, I removed --enable-java-debug-symbols in this webrev.
I will file to JBS and try to create patch for it after this issue.


That is, have you tried building with the four different values of 
--with-native-debug-symbols and verified that the result is indeed as you 
specified?

Of course, I've checked all pattern of --with-native-debug-symbols.
I ran readelf -S for libjvm, libnio, libsaproc, and java command.
I checked debug section (.debug_info, and more) in ELF binaries.


Just by my quick glance I can already spot what I believe is a mistake: for 
"none" you set STRIP_POLICY to min_strip, but it should reasonably be no_strip.

I've fixed in this webrev.


Thanks,

Yasumasa


On 2015/11/30 21:26, Magnus Ihse Bursie wrote:


On 2015-11-26 15:19, Yasumasa Suenaga wrote:
I uploaded a new webrev:
  http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.04/

I implemented --with-native-debug-symbols and --enable-java-debug-symbols in it.
And I deprecated --enable-debug-symbols and --enable-zip-debug-info .

Hi Yasumasa,

I apologize I have not been able to spend more time on this. I still only had 
time to take a short glimpse, but I found a few issues anyway.

First, the new -enable-java-debug-symbols is problematic. If you build with 
"--enable-debug --disable-java-debug-symbols", then this implies that you would 
get a build without java debug symbols. However, this will not happen, since changing 
debug level wil enable java debug symbols, but the new java-debug-symbols flag will not 
manage to disable this.

If we do implement this flag, it should work as expected. If you believe this 
is too hard to get right, I'm alright with fixing this as a separate bug.

Second, all the flags we use to manage this internally makes me nervous. :) Are 
you sure you're getting them right? That is, have you tried building with the 
four different values of --with-native-debug-symbols and verified that the 
result is indeed as you specified? You'd need to check both libjvm.so and at 
least a jdk library, like libjava.so or so, since they use different build 
system and flags.

Just by my quick glance I can already spot what I believe is a mistake: for 
"none" you set STRIP_POLICY to min_strip, but it should reasonably be no_strip. 
(I'd be surprised if strip was run if I compiled without debug symbols).

Other than that, I believe this patch is starting to get ready.

/Magnus


Thanks,

Yasumasa


On 2015/11/25 23:52, Yasumasa Suenaga wrote:
Hi Magnus,

--with-java-debug-symbols=none,all
(or "none,internal" for consistency).

Can I change this option to --enable-java-debug-symbols ?
This value set to none or all. So I think that we should use AC_ARG_ENABLE for 
it.


Thanks,

Yasumasa


On 2015/11/25 23:35, Magnus Ihse Bursie wrote:
On 2015-11-21 14:12, Yasumasa Suenaga wrote:
My point with current proposal is that either it doesn't touch the zipping 
because we already have an option for that; or it should deprecate the existing 
option (now Erik has pointed out that capability).

I'm using BASIC_DEPRECATED_ARG_ENABLE for debug-symbols and zip-debug-info in 
new webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/webrev.03/

I had thought this new option as a wrapper for existing options.
However, if current options should be deprecated, I will follow it.

Hi Yasumasa,

Sorry for the delay in my response.

I had to go back and re-read the old discussions and my personal notes about 
this. It turned out that some of the complications that existed then is not 
relevant from Oracle's point of view anymore (more specifically, stripping to 
several distinct levels). With that complication gone, the basic case boils 
down to the three ways of doing this:
1) Compile without debug information ("none")
2) Compile with debug information and keep it ("internal")
3) Compile with debug information, copy it to an external file, and strip the binary 
("external")

So far, so good. I agree with David Holmes that the zipping complicates things somewhat. 
The problem here is two-fold: Firstly, I believe that the "external" choice 
will be relevant to nobody (as in, neither the community nor Oracle), and secondly, there 
have been discussions of handling the external debuginfo differently (namely, to move all 
debug info, unzipped, into a separate image, and zip them all at once). It's a bit 
unfortunate if we create an interface to configure that cannot grow properly with future 
changes.

On the other hand, it does seem convenient to express the end result for the Oracle build as 
"zipped" (or possibly "external-zipped", to emphasize  the connection to 
"external"?).

So I'm not sure what to think myself. But if David agrees that adding "zipped" (or 
"external-zipped") to the new argument, and deprecating the old, then I agree with it as 
well. :-)

The second discussion here is the connection between java and native debug 
symbols.

Fact: We do not currently have a story for setting java debug symbols. That's 
bad, and a regression as pointed out.
Fact: That does not mean it has to be fixed in this patch.

I do think that java and native debug symbols are different things and should 
be treated differently. But I also think we should have configure arguments 
that show their similarity. So, I propose that we have a
--with-native-debug-symbols=none,internal,external,zipped
as you propose, and then also a
--with-java-debug-symbols=none,all
(or "none,internal" for consistency).

I think it would be nice if that flag could be implemented in the same patch, 
but since it is a different thing I understand if you want to leave it out. 
There is some added complication here, since we already set -g for java 
compilation depending on debug level, and the native debug symbol patch is 
obviously tricky as it is.

/Magnus

Reply via email to