RFR(L): 8035936: SIGBUS in StubRoutines::aesencryptBlock, solaris-sparc

Vladimir Kozlov vladimir.kozlov at oracle.com
Mon Apr 28 22:25:58 UTC 2014


Hi Joshi,

In several places I see unconditional branch with nop in delay slots:

+     __ br(Assembler::always, true, Assembler::pt, L_load_expanded_key);
+     __ delayed()->nop();

Why you did not use cbcond (when distance is small) since AES will be 
used only on T4 and newer cpus?:

   __ ba_short(L_load_expanded_key);

There are other short branches variants.

In general changes are good.

Thanks,
Vladimir

On 4/25/14 5:45 PM, Shrinivas Joshi wrote:
> Hi,
>
> Requesting reviews for the following change. Thanks for your time.
>
> -Shrinivas
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8035936
> Webrev: http://cr.openjdk.java.net/~kvn/8035936/webrev/
>
> Summary: The intrinsified AES encryption and decryption JDK API methods
> take byte arrays and offsets into these arrays as input arguments. Thus
> the stub routines can potentially read from and write to arbitrarily
> aligned addresses. SPARC load/store instructions generate
> 'mem_address_not_aligned' exception when they operate on improperly
> aligned addresses. This change addresses the arbitrary alignment issue
> in case of SPARC AES crypto stub routines.
>
> Summary of source code changes:
>
> * src/cpu/sparc/vm/assembler_sparc.hpp:
>     - Added support for ALIGNADDR(Align Address), FALIGNDATA(Align Data
> using GSR.align), EDGE8N(Edge Handling Instructions), FSRC2(F Register
> Logical Operate) and STPARTIALF(Store Partial Floating-Point) instructions
>
> * src/cpu/sparc/vm/stubGenerator_sparc.cpp:
>     - Added assertion checks to test if first element of 'int' and
> 'byte' arrays are at 8-byte aligned address. HotSpot's current object
> layout guarantees this.
>     - Changed single-block encrypt and decrypt stub routine names to
> match with what is used on x86.
>     - Added code to address mis-aligned loads and stores. Added some
> details on the methodology used to address mis-aligned load/store
> instructions (in form of comments).
>     - Since we are now explicitly testing 8-byte alignment, we are now
> using 8-byte FP load/store instructions instead of the 2*4-bytes and a
> FMOV instruction as was done previously.
>     - Single-block decrypt routine now uses a lower latency FSRC2d
> instruction instead of the more expensive FMOVd instruction.
>     - Multi-block (CBC) encrypt routine now does a save-restore to ease
> register pressure due to mis-alignment related code.
>     - CBC encrypt routine uses G5 instead of G2 register.
>     - During mis-aligned stores in CBC encrypt, we also have to save and
> reload cipher text (in F60:F62) so that it is saved in the correct byte
> order in the initialization vector which is then fed back into
> encryption of the next block. This is done by MOVdTOx and MOVxTOd
> instructions.
>
> * src/cpu/sparc/vm/stubRoutines_sparc.hpp:
>     - Increased code buffer blob size for stub routines as the current
> size of 20000 is not sufficient after addition of mis-alignment related
> code.
>
> * src/cpu/sparc/vm/vm_version_sparc.cpp:
>     - Updated comments to reflect that AES instructions are available
> starting SPARC T4, previously it was incorrectly stated as T2.
>     - Updated AES intrinsics flag value sanity checks to include VIS3
> instruction capability instead of only VIS1 since VIS3
> instructions(MOVdTOx, MOVxTod) are also used by AES stubs.
>
> * src/share/vm/classfile/vmSymbols.hpp and src/share/vm/opto/runtime.cpp
>     - typo fixes
>
> * test/compiler/7184394/TestAESBase.java
>     - Added code to exercise mis-aligned loads and stores.
>     - Made padding string a property that can be passed on command-line
> since certain alignment issues are not triggered with padding enabled.
>     - If any of the alignment cases (encryption input, encryption
> output/decryption input and decryption output) are being tested then the
> test case now does multi-part crypto operations with an update()
> followed by doFinal(). We need to do this because certain alignment
> issues are not triggered when only doFinal() calls are used. If
> mis-alignment is not being tested then the test case excercises
> single-part operations as was the case previously.
>     - In case of multi-part operations, the last chunk of input passed
> to encryption/decryption routines is set to 2*blocks long. This is again
> because mis-aligned stores during decryption are not triggered if input
> is less than 2*blocks long.
>
> * test/compiler/7184394/TestAESDecode.java and
> test/compiler/7184394/TestAESEncode.java:
>     - Exercises mulit-part mis-aligned operations when mis-alignment is
> being tested
>
> * test/compiler/7184394/TestAESMain.java:
>     - Added new @run commands to include various mis-aligned cases.
>     - warm-up iterations count is now configurable.


More information about the hotspot-compiler-dev mailing list