RFR: 8206454: [8u] os::current_stack_pointer() fails to compile on later Windows compilers (warning C4172: returning address of local variable)

[Kim Barrett]

> On Jul 6, 2018, at 4:37 PM, Kevin Walls <kevin.walls at oracle.com> wrote:
> 
> Hi Kim -
> 
> Good question. 8-)  Yes the same code is there in the latest version when I looked, but address os::current_stack_pointer() is within an #ifndef AMD64, so I suspect have we never built 32-bit with the later VS2017 compiler…

Oh, you are right.  I mis-read that as #ifdef AMD64.

Yeah, we (Oracle) don’t seem to be doing 32bit Windows builds anymore.  Presumably
nobody else is doing so with recent versions of VS and the JDK, else the problem would
have already been known (and probably dealt with).  Since I’m not a big fan of adding
untested code, I’m okay with just addressing this in jdk8u, and leaving the current code
alone until someone complains about it.

Unfortunately, while this change looks okay to me, I’m not even a committer for jdk8,
so you still need a Reviewer.

> More file context around the 8u suggestion below..
> 
> Thanks
> Kevin
> 
> src/os_cpu/windows_x86/vm/os_windows_x86.cpp
> 
>    452
>    453  #ifndef AMD64
>    454  // Returns an estimate of the current stack pointer. Result must be guaranteed
>    455  // to point into the calling threads stack, and be no lower than the current
>    456  // stack pointer.
>    457  #if defined(_MSC_VER) && _MSC_VER >= 1900
>    458  // warning C4172: returning address of local variable or temporary: dummy
>    459  #pragma warning( push )
>    460  #pragma warning( disable: 4172 )
>    461  #endif
>    462  address os::current_stack_pointer() {
>    463    int dummy;
>    464    address sp = (address)&dummy;
>    465    return sp;
>    466  }
>    467  #if defined(_MSC_VER) && _MSC_VER >= 1900
>    468  #pragma warning( pop )
>    469  #endif
>    470  #else
>    471  // Returns the current stack pointer. Accurate value needed for
>    472  // os::verify_stack_alignment().
>    473  address os::current_stack_pointer() {
>    474    typedef address get_sp_func();
>    475    get_sp_func* func = CAST_TO_FN_PTR(get_sp_func*,
>    476 StubRoutines::x86::get_previous_sp_entry());
>    477    return (*func)();
>    478  }
>    479  #endif
>    480
> 
> 
> 
> 
> On 06/07/2018 21:09, Kim Barrett wrote:
>>> On Jul 6, 2018, at 5:54 AM, Kevin Walls <kevin.walls at oracle.com> wrote:
>>> 
>>> Hi,
>>> 
>>> I'd like to get a review of this change for 8u:
>>> 
>>> 8206454: [8u] os::current_stack_pointer() fails to compile on later Windows compilers (warning C4172: returning address of local variable)
>>> https://bugs.openjdk.java.net/browse/JDK-8206454
>>> 
>>> This is part of getting jdk8u to accept later Windows compilers. Not changing the code, but adding a conditional  #pragma to let the compiler accept the code.
>> Why is this only being done for jdk8u, and why isn’t it a problem in (say) jdk11 or jdk12?
>> The code being patched below is still the same.
>> 
>>> I'll include a diff in text below (can do a webrev if anybody would like one...).
>>> 
>>> Many thanks!
>>> Kevin
>>> 
>>> 
>>> bash-4.2$ hg diff src/os_cpu/windows_x86/vm/os_windows_x86.cpp
>>> diff -r 5792d995ed26 src/os_cpu/windows_x86/vm/os_windows_x86.cpp
>>> --- a/src/os_cpu/windows_x86/vm/os_windows_x86.cpp      Wed Jun 27 03:04:33 2018 -0700
>>> +++ b/src/os_cpu/windows_x86/vm/os_windows_x86.cpp      Fri Jul 06 02:46:48 2018 -0700
>>> @@ -454,11 +454,19 @@
>>>  // Returns an estimate of the current stack pointer. Result must be guaranteed
>>>  // to point into the calling threads stack, and be no lower than the current
>>>  // stack pointer.
>>> +#if defined(_MSC_VER) && _MSC_VER >= 1900
>>> +// warning C4172: returning address of local variable or temporary: dummy
>>> +#pragma warning( push )
>>> +#pragma warning( disable: 4172 )
>>> +#endif
>>>  address os::current_stack_pointer() {
>>>    int dummy;
>>>    address sp = (address)&dummy;
>>>    return sp;
>>>  }
>>> +#if defined(_MSC_VER) && _MSC_VER >= 1900
>>> +#pragma warning( pop )
>>> +#endif
>>>  #else
>>>  // Returns the current stack pointer. Accurate value needed for
>>>  // os::verify_stack_alignment().