On 5/24/12 8:28 AM, Karen Kinnear wrote:
Dan,
Code looks good.
Thanks!
Thank you for the work-around.
So is this HDR_FLAGS workaround also needed in all the other jdk repositories?
SHF_ALLOC flag?
We know that gobjcopy crashes when running into empty ELF sections
where the SHF_ALLOC flag is set. We have only seen the loader (ld)
generate empty ELF sections with the SHF_ALLOC flag set when compiling
for Solaris X64. However, it seems safer to me to always apply the
work around when building on Solaris just to be sure.
The fix_empty_sec_hdr_flags prints a diagnostic message when it
modifies an ELF object so we can check build logs periodically to
verify which Solaris builds are affected.
And I assume the windows/makefiles/defs.make change is because you determined
that the failing tests actually pre-dated the zipped debuginfo.
Correct. ZIP'ing the .pdb and .map files was not the cause of library
loading failures.
Did you find the root cause for that
yet, or is that an orthogonal topic?
Evgeny's team found that several Windows machines were missing the
msvcr71.dll library. That version of the MS runtime is needed by
the affected testbase(s) (just VM/NSK I think, not sure).
And this also appears to have additional modifications for the earlier
workaround add_gnu_debuglink.
Yes, after getting hit by the M&M test failures on Solaris 11, I
decided to apply the add_gnu_debuglink work around to every FDS
use of gobjcopy.
I has assumed in reviewing 7165060 that you only used that for makefiles that
built with DOF sections,
i.e. libjvm.so and that you had chosen not to use the simpler add_gnu_debuglink
in all cases, even though
it is generally less risky. Did you change your approach here?
Yes, I've changed approaches due to the unexpected M&M failures.
I was originally balancing the risk of changing more Makefiles
to use the add_gnu_debuglink work around versus the risk that
gobjcopy would screw up some other ELF section in ways that we
couldn't easily detect.
In hindsight, I should have gone with using the add_gnu_debuglink
because the changes that it makes to an ELF object are rather easy
to verify as correct by comparing two elfdump output files.
Your comment was changed from SUNW_DOF to
SUNW_*. That makes sense to me.
Yes, Dean's guess is that the SUNW_cap section is what the M&M tests
were relying on and when gobjcopy munged it, the tests started to
fail on Solaris 11.
Interesting data points: the tests don't fail on Solaris 10 or on a
recent Solaris 11 Update 1 build (I used jurassic for a quick test).
So whatever gobjcopy did to the SUNW_cap section is properly handled
by newer Solaris 11 versions...
And are you also changing that for
the other jdk repositories?
Yes, please search for 7170449 in your InBox and you'll see that
review thread. I would love to have your review there also.
Dan
thanks,
Karen
On May 23, 2012, at 3:59 PM, Daniel D. Daugherty wrote:
Greetings,
This is a hotspot code review request for the second of a pair of
Full Debug Symbols gobjcopy work arounds on Solaris. The first
hotspot FDS gobjcopy work around was reviewed using bug 7165060
and that fixed the dtrace test failures.
The gobjcopy utility also crashes due to empty sections with the
SHF_ALLOC flagset on Solaris X64 objects. This causes build
failures.
The first new temporary work around tool is add_gnu_debuglink
and it was added by 7165060.The second new temporary work around
tool is:
fix_empty_sec_hdr_flags - removes the SHF_ALLOC flag from empty
sections in ELF objects.
These temporary work arounds are only needed until the proper
Solaris 10 Update 6 patches are made available. The two patches
are independent of one another which is why there are two
separate temporary work arounds. However, we're putting the
temporary work arounds in place because the 7u6/HSX-23.2 project
window is closing fast.
Here is the webrev URL for the HSX-24 version:
http://cr.openjdk.java.net/~dcubed/fds_revamp/7165598-webrev/0/
This fix will also be backported to 7u6/HSX-23.2 and I expect the
changes to virtually identical.
Thanks, in advance, for any reviews!
Dan