RFR: 8206454: [8u] os::current_stack_pointer() fails to compile on later Windows compilers (warning C4172: returning address of local variable)
Kevin Walls
kevin.walls at oracle.com
Mon Jul 9 11:00:38 UTC 2018
Hi David, sure no problem we can do the "default" after the code we want
to affect here.
I only wanted to put the #pragma within the #if for compiler level, not
to change what the older compiler does. (I have (now) run the existing
VS2010 build always doing the #pragma, and also using the push/pop, and
they are all OK too. I'd still like to go with with the condition and
not change anything about existing builds.)
Updated diff below.
Thanks!
Kevin
bash-4.2$ hg diff
diff -r ad057f2e3211 src/os_cpu/windows_x86/vm/os_windows_x86.cpp
--- a/src/os_cpu/windows_x86/vm/os_windows_x86.cpp Wed Jul 04
03:02:43 2018 -0400
+++ b/src/os_cpu/windows_x86/vm/os_windows_x86.cpp Mon Jul 09
01:10:18 2018 -0700
@@ -454,11 +454,18 @@
// 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(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(default: 4172)
+#endif
#else
// Returns the current stack pointer. Accurate value needed for
// os::verify_stack_alignment().
On 09/07/2018 03:38, David Holmes wrote:
> Hi Kevin,
>
> I note we currently never use push/pop on Windows, rather we use the
> pattern:
>
> #pragma warning( disable : NNN )
> <code>
> #pragma warning( default : NNN )
>
> Do older 8u compilers support the push/pop pragma warning capability?
>
> Thanks,
> David
>
> On 7/07/2018 5:22 PM, Kevin Walls wrote:
>> No problem! Thanks for bringing this up for clarification.
>>
>>
>> On 06/07/2018 23:32, Kim Barrett wrote:
>>>> 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().
>>>
>>
More information about the hotspot-dev
mailing list