code review for second hotspot FDS gobjcopy work around (7165598)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Thu May 24 16:01:55 UTC 2012
On 5/24/12 8:28 AM, Karen Kinnear wrote:
> Code looks good.
> Thank you for the work-around.
> So is this HDR_FLAGS workaround also needed in all the other jdk repositories?
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
> 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.
> On May 23, 2012, at 3:59 PM, Daniel D. Daugherty wrote:
>> 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
>> 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:
>> 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!
More information about the build-dev