[hs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[David Holmes]

Hi Andrew,

On 24/05/2018 12:32 AM, Andrew Dinn wrote:
> Hi David,
> 
> There is no need to worry about nestmates here -- the problem is (very)
> real but lies elsewhere.

Thanks - that's a relief for me. :)

I'm curious though why Aarch64 testing has not caught this before now? 
The update to SpecialInterfaceCall.java (which I believe caught this) 
was pushed over 3 weeks ago.

David

> First things first: the disparity between my test results and Aleksey's
> is because I tested a release build and he tested a fastdebug build. Now
> as to that:
> 
>    arguably, Aleksey is right to test with debug enabled so as to catch
> conditions innocuous to the test but still unexpected
> 
>    arguably, I am right to test what will end up being released because,
> err ... well, that's what actually needs to work
> 
> I guess we really need to test both.
> 
> Secondly, test SpecialInterfaceCall only failed because debug code was
> exercised (in code guarded by "if (VerifyfMethodHandles)") yet the
> failure was actually in non-debug code called under that branch. So, it
> is quite an important error to have found. To be more sepcific:
> 
> The AArch64 version of MacroAssembler::check_klass_subtype_slow_path
> has had this error in it from when it was created. The default
> assumption is that r0 holds a pointer to the super class -- on x86 the
> equivalent register is rax. If a different register is passed in then it
> needs to be moved into r0, after saving the value stored there. The x86
> implementation plants a push of rax onto followed by a mov from the
> input register to rax. On AArch64 the equivalent code always did the
> push but also always omitted the mov.
> 
> It is probably worth providing a post mortem to understand why has this
> not been caught up until now. Firstly, subclass checks planted in other
> stubs seem always to have the super in r0 so the error does't show
> elsewhere. The method handle invocation stub uses r11 for the super but
> the debug code is only planted there and only executes the super check
> if VerifyfMethodHandles is set. So, it won't show in a release build.
> Also, the error only trips you up if the fast super check fails and a
> super search is needed. So, it won't show unless you can foil the super
> caching code by trying an invoke via one interface and then another.
> 
> I guess SpecialInterfaceCall is the first indy test where this
> combination of events has arisen in a fastdbeug build. I am left
> wondering how many other 5 year old bugs we still have in the AArch64
> code base?
> 
> regards,
> 
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
>