RFR: JDK-8270308: Amalloc aligns size but not return value [v2]

[Thomas Stuefe]

> Hi,
> 
> may I have reviews for this fix to Arena alignment handling in hotspot. This affects the old Arena classes as well as child classes (ResourceArea, HandleArea etc). This is a followup to https://bugs.openjdk.java.net/browse/JDK-8270179 and other recent Arena fixes.
> 
> This makes arenas more flexible, allowing to allocate with different alignments to achieve tighter packing. Theoretically arenas should be able to do that today, but it is actually broken. We did not notice any errors though, probably because the only platforms affected are 32-bit platforms other than x86 (x86 allows unaligned access). Also, today in the VM nobody mixes different alignments - you either allocate only with Amalloc, so with 64bit alignment, or only with AmallocWords (former Amalloc_4), which is 32bit on 32bit.
> 
> I redid this patch twice. My first attempts rewrote a lot of the code, but I scratched that and in this patch kept it focused only on the alignment issue. Defer cleanups to separate RFEs.
> 
> This patch establishes new alignment rules, but actual behavioral changes to current Arena users should be minimal and should only affect 32-bit anyway.
> 
> Before this patch, Arena allocation checked (or auto-aligned) the *allocation size* in an attempt to guarantee alignment of the allocation start address. That was neither needed nor sufficient - see JBS text.
> 
> The new code just allows any allocation size, aligned or unaligned. However, it now clearly guarantees alignment of the *allocation start address*. E.g., theoretically, you could allocate a 16bit word at a 64bit boundary.
> 
> The arena does this by aligning, if needed, before an allocation. Alignment gaps are filled (debug build + ZapResourceArea) with a special zap pattern.
> 
> 
> ---
> 
> Notes:
> - Chunk start, payload start, and payload end have to be aligned to max arena alignment. I wondered whether just statically assert `sizeof(Chunk)` to be aligned; but I did not find a pragma for this for all platforms, and did not want to play whack-the-mole with 32bit and strange compilers like Xlc. Therefore we align payload start separately.
> - To simplify implementation, I added the rule that the four standard chunk sizes (those cached in the pools) have to be aligned to max arena alignment. They mostly were already, just had to tweak numbers a bit for 32-bit.
> - Chunks are preceded with a canary now for debugging purposes and potential sanity tests in gtests (not used yet)
> - Arena::realloc():
> 	- Commented in header that the original alignment is preserved (which it is since Arealloc either changes the allocation in-place, so start address does not change, or reallocates with max possible alignment.
> 	- I fixed another pointer calculation to use pointer_delta when an allocation is grown in-place
> 	- I added code to zap the leftover memory if an allocation shrinks
> 
> Tests:
> - I wrote a large number of gtests for the Arena class. These test proper behavior on alloc, realloc, and free, including overwrite tests and gap pattern tests.
> - wrote a new gtests jtreg wrapper to run the Arena gtests with +UseMallocOnly - I think as long as we support it we should test it.
> 
> Ran all tests on 32bit and 64bit Linux on my box.
> 
> Nightlies at SAP are in progress.
> 
> Manual tests:
> - I also tested a higher arena max alignment (16 bytes on 64 bit), ran gtests, worked fine
> - I also tested an unaligned Chunk size on 64bit to test padding after header is handled correctly, works fine

Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:

 - Merge
 - Arena alignment fixes

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4784/files
  - new: https://git.openjdk.java.net/jdk/pull/4784/files/14e7c086..0e11110a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4784&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4784&range=00-01

  Stats: 2716 lines in 96 files changed: 2050 ins; 234 del; 432 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4784.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4784/head:pull/4784

PR: https://git.openjdk.java.net/jdk/pull/4784