RFR: 8239001: Hotspot build broken on linux-sparc after 8238281

Kim Barrett kim.barrett at oracle.com
Fri Feb 14 01:10:42 UTC 2020


> On Feb 13, 2020, at 5:21 PM, Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:
> 
> I searched the code base for mentions of old gcc workarounds, and tried to remedy them now that they were not needed. 
> 
> If it turns out that they still were needed, by all means, revert that part of the patch. 

I think the offset_of definition in question was not an "old gcc
workaround".

offsetof with a non-POD type is undefined behavior in C++98.  C++11
relaxed the restriction to allow standard-layout types.  C++17 changed
offsetof with a non-standard-layout type to be "conditionally
supported", which is more or less effectively UB for portable code.

Contrary to what was said in the RFR for JDK-8238281, we should not be
considering eliminating offset_of and just using offsetof directly.
offset_of is the HotSpot portability shim to let us knowingly do
something that is potentially problematic but we claim we know what
we're doing.  The fact that gcc warns about problematic offsetof uses
(unless -Wno-invalid-offsetof) is a good thing, because it helps keep
us honest about using that portability shim.  (This is similar in
concept to JDK-8214976.)  (There are a couple of direct uses of
offsetof in os_perf_solaris.cpp.)

Then there's the separate issue that the failing code in the
RegistersForDebugging class is using offset_of / offsetof in an
invalid way, since the result is supposed to be a constant expression
and that's impossible because these uses involve the non-constant "j"
argument.

I think that can be fixed by something like

/* Add this in RegistersForDebugging */
private:
  static const RegistersForDebugging& _dummy; // not ODR-used so not defined

/* Change offset functions based on this model. */
static int i_offset(int j) {
  return offset_of(RegistersForDebugging, i) + j * (sizeof _dummy.i[0]);
}

Of course, reverting to the gcc implementation of offset_of also
"fixes" this problem, because that version of offset_of happens to
"accidentally" be more lenient.




More information about the build-dev mailing list