[PATCH] 8006508 : Wrong frame constructor is called in os_linux_x86.cpp

[David Holmes]

Hi Coleen,

On 18/01/2013 1:25 PM, Coleen Phillimore wrote:
>
> It makes sense to call the default constructor in this case. But there
> are other cases just like this. Do they need to be changed also?
>
> ./os_cpu/bsd_x86/vm/os_bsd_x86.cpp: return frame(NULL, NULL, NULL);
> ./os_cpu/linux_sparc/vm/os_linux_sparc.cpp: return frame(NULL,
> frame::unpatchable, NULL);
> ./os_cpu/linux_x86/vm/os_linux_x86.cpp: return frame(NULL, NULL, NULL);
> ./os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp: return frame(NULL, NULL,
> NULL);
> ./os_cpu/windows_x86/vm/os_windows_x86.cpp: if (func == NULL) return
> frame(NULL, NULL, NULL);
> ./os_cpu/windows_x86/vm/os_windows_x86.cpp: return frame(NULL, NULL, NULL);

Good catch. All the <os>_x86 files will need the same fix as they will 
call the frame_x86.inline.hpp frame::frame(intptr_t* sp, intptr_t* fp, 
address pc) constructor.

For sparc there is no problem. The three arg constructors called above 
call the four arg constructor:

   frame(intptr_t* sp, unpatchable_t, address pc = NULL, CodeBlob* cb = 
NULL);

which as you can see defaults pc and cb to NULL. The constructor body 
then requires that both be NULL if either is, hence the calls you 
flagged are okay: one explicitly NULL the other defaults to NULL.

Thanks,
David

> Thanks,
> Coleen
>
> On 1/17/2013 1:32 PM, Jeremy Manson wrote:
>> As discussed with David last month. The frame constructor that is
>> currently getting called in this case fails if NULL is passed as a PC.
>> The fix is to call a constructor that does not expect a PC.
>>
>> Apologies for the lack of a reproducible test case, but it's basically
>> impossible to reproduce without a very peculiar and platform-dependent
>> compilation strategy.
>>
>> # HG changeset patch
>> # User jeremymanson
>> # Date 1358447322 28800
>> # Node ID b155e0a0e6f030aea5fbdf8cd341efcb2f097a76
>> # Parent 1a3e54283c54aaa8b3437813e8507fbdc966e5b6
>> 8006508 : Wrong frame constructor is called in os_linux_x86.cpp
>>
>> diff --git a/src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>> b/src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>> --- a/src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>> +++ b/src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>> @@ -189,7 +189,7 @@
>> CAST_FROM_FN_PTR(address, os::current_frame));
>> if (os::is_first_C_frame(&myframe)) {
>> // stack is not walkable
>> - return frame(NULL, NULL, NULL);
>> + return frame();
>> } else {
>> return os::get_sender_for_C_frame(&myframe);
>> }
>>
>