RFC: 8229469 JEP 386: Alpine Linux/x64 Port

[David Holmes]

Hi Aleksei,

Overall this all seems okay. A few minor comments below.

On 1/09/2020 9:41 pm, Aleksei Voitylov wrote:
> Hi,
> 
> JEP 386 is now Candidate [1] and as we resolved all outstanding issues
> within the Portola project I'd like to ask for comments from HotSpot,
> Core Libs (changes in libjli/java_md.c), and Build groups before moving
> the JEP to Proposed to Target:
> 
> http://cr.openjdk.java.net/~avoitylov/webrev.8247589.01/

src/hotspot/os/linux/os_linux.cpp

  624   os::Linux::set_libc_version("unknown");
  625   os::Linux::set_libpthread_version("unknown");

Surely we can do better than "unknown"? Surely different releases of 
Alpine linux must identify different version of the libraries?

--

The PaX related error message belongs in release notes not the source 
code - the error message should be much more succint:

"Failed to mark memory page as executable - check if grsecurity/PaX is 
enabled"

--

src/hotspot/os/linux/os_linux.hpp

numa_node_to_cpus is too big to be in the header file now.

---

src/hotspot/share/prims/whitebox.cpp

I would expect this to use the version string set in os_linux.cpp.

---

src/hotspot/share/runtime/abstract_vm_version.cpp

Ideally LIBC_STR would come from the build system - as we do for 
FLOAT_ARCH. See flags.m4.

---

src/java.base/unix/native/libjli/java_md.c

The comment is way too long - see the AIX case below it. :)

---

src/jdk.hotspot.agent/linux/native/libsaproc/ps_proc.c

  282     // To improve portability across platforms and avoid conflicts
  283     // between GNU and XSI versions of strerror_r, plain strerror 
is used.
  284     // It's safe because this code is not used in any 
multithreaded environment.

It is not at all clear to me that this code is not used in a 
multi-threaded environment. The VM is always multi-threaded.  This usage 
was added here:

http://mail.openjdk.java.net/pipermail/serviceability-dev/2015-December/018419.html

But this code also uses strerror in another place (as does other code) 
so ...

---

test/hotspot/jtreg/runtime/StackGuardPages/exeinvoke.c

Why is this change needed?

---

In general the busybox changes are bit unpleasant in the tests. Pity 
Alpine didn't try to present a familiar binary environment. :(

---

test/jdk/java/lang/ProcessBuilder/RedirectWithLongFilename.java

@comment is not needed

That said I don't think that test should be using the Basic class.

---

Thanks,
David


> Testing:
> 
> JTReg, VM TestBase on all platforms (Linux x86_64, Linux x86, Linux Arm,
> Linux AArch64, Linux PPC, Windows x86_64, Windows x86, Mac) with no
> regressions. A downport of these changes to 14 passes JCK 14 on these
> platforms.
> 
> The port was tested on Alpine Linux 3.8 and 3.11 with JTReg, VM
> TestBase. There are no differences with Linux x86_64  with the exception
> of SA which is not supported as per the JEP. A downport of these changes
> to 14 passes JCK 14 on Alpine Linux.
> 
> Thanks,
> 
> -Aleksei
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8229469
> 
>