RFR: 8253375: OSX build fails with Xcode 12.0 (12A7209)

[Kim Barrett]

> On Sep 29, 2020, at 3:59 AM, Matthias Baesken <mbaesken at openjdk.java.net> wrote:
> 
> On Fri, 25 Sep 2020 02:23:07 GMT, David Holmes <dholmes at openjdk.org> wrote:
> 
>>> Please review this small patch to enable the OSX build using Xcode 12.0.
>>> 
>>> Thanks,
>>> Paul
>> 
>> src/hotspot/share/runtime/sharedRuntime.cpp line 2851:
>> 
>>> 2849:     if (buf != NULL) {
>>> 2850:       CodeBuffer buffer(buf);
>>> 2851:       short locs_buf[80];
>> 
>> This code is just weird. Why is the locs_buf array not declared to be the desired type? If the compiler rejects double
>> because it isn't relocInfo* then why does it accept short? And if it accepts short will it accept int or long long or
>> int64_t? Using int64_t would be a clearer change. Though semantically this code is awful. :( Should it be intptr_t ?
> 
> Currently we have in the existing code  various kind of buffers passed into initialize_shared_locs that compile nicely
> on all supported compilers and on Xcode 12 as well ,see
> 
> c1_Compilation.cpp
> 
> 326  char* locs_buffer = NEW_RESOURCE_ARRAY(char, locs_buffer_size);
> 327  code->insts()->initialize_shared_locs((relocInfo*)locs_buffer,
> 
> sharedRuntime.cpp
> 
> 2681      CodeBuffer buffer(buf);
> 2682      short buffer_locs[20];
> 2683      buffer.insts()->initialize_shared_locs((relocInfo*)buffer_locs,
> 2684                                             sizeof(buffer_locs)/sizeof(relocInfo));
> 
> So probably using short would be consistent to what we do already in the coding at another place (looking into
> relocInfo it would be probably better  to use unsigned short  or to   typedef  unsigned short in the relocInfo class
> and use the typedef).

That’s *not* consistent, because of buffer alignment.  The call to NEW_RESOURCE_ARRAY is going
to return a pointer to memory that is 2*word aligned.  (Interesting, possibly a 32/64 bit “bug” here.)
The existing code in sharedRuntime.cpp is allocating double-aligned.  iniitalize_shared_locs wants a
HeapWordSize-aligned buffer, and adjusts the pointer it uses accordingly.  (I think with existing code
it could just make the alignment of the buffer a precondition, but I haven’t looked at all callers.)
Changing the declaration in sharedRuntime.cpp to short[] reduces the alignment requirement for the
array, possibly requiring alignment adjustment (and hence size reduction) by initialize_shared_locs.

Since initialize_shared_locs specifically adjusts the alignment, some downstream use of the buffer
probably actually cares.


> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/348