6953477: review request for portability improvements to Hotspot

[Tom Rodriguez]

On Jul 28, 2010, at 11:46 AM, Bob Vandette wrote:

> 
> 
> On Jul 28, 2010, at 2:18 PM, Tom Rodriguez wrote:
> 
>> INTX_FORMAT isn't available in the SA anyway so it won't compile if you fix it.
> 
> I was about to find that out.
> 
>> As is the changes break the code so they clearly aren't necessary, so can they be dropped?  
> 
> We must have had problems with newer gcc versions on x86 or we wouldn't have changed this.
> 
> 
>> Same with the os_linux.cpp changes with UFM.
> 
> The only reason, I think, that someone in my team would have changed these files is if GCC complained.
> INTX_FORMAT is available in os_linux.cpp, so I should be able to just move the " characters around the
> macro uses.
> 
> So the only change I need to think about is the ps_proc.c one.
> 
> This is what prompted the change.  Using gcc 4.3.2 on x86 Linux produces this warning:
> 
> ps_proc.c:256: warning: format ‘%lx’ expects type ‘long unsigned int *’, but argument 3 has type ‘intptr_t *’
> 
> Since this is in a Linux specific source file, I wonder if %p would work in all cases?

Maybe.  The format of %p isn't standard.  Some platforms include 0x at the beginning and others don't and I'm not sure what linux does.  Maybe it should simply be ifdef'ed _LP64 with the type of base corrected to uintptr_t:

         uintptr_t base;
         lib_info* lib;
#ifdef _LP64
         sscanf(word[0], "%lx", &base);
#else
         sscanf(word[0], "%x", &base);
#endif
         if ((lib = add_lib_info(ph, word[5], base)) == NULL)

Whatever fix is done needs to be tested to make sure it doesn't break that code.  The nsk/sajdi tests would exercise the needed code I think.

tom

> 
> Bob.
> 
> 
>> 
>> tom
>> 
>> On Jul 28, 2010, at 9:13 AM, Bob Vandette wrote:
>> 
>>> Thanks for catching that Kelly.
>>> 
>>> On Jul 28, 2010, at 11:52 AM, Kelly O'Hair wrote:
>>> 
>>>> Just did a very quick scan.
>>>> 
>>>> Something doesn't look right with the INTX_FORMAT, _UFM, and _DFM uses.
>>>> Macros will not be expanded inside string literals, and these macros are string literals.
>>>> 
>>>> Files agent/src/os/linux/ps_proc.c and src/os/linux/vm/os_linux.cpp
>>>> 
>>>> -kto
>>>> 
>>>> On Jul 28, 2010, at 7:40 AM, Bob Vandette wrote:
>>>> 
>>>>> Here are a set of changes that we're planning on pushing into the openjdk hotspot repository.
>>>>> 
>>>>> http://cr.openjdk.java.net/~bobv/6953477/webrev.00/
>>>>> 
>>>>> They include:
>>>>> 
>>>>> 1. Cross Compilation improvements
>>>>> 2. Shared code changes for ARM and PPC
>>>>> 3. Software floating point support
>>>>> 4. Improvements to the hotspot crash log
>>>>> 
>>>>> There are a few areas that could use additional factoring but these will be done under a seperate
>>>>> CR.
>>>>> 
>>>>> 
>>>>> Bob Vandette
>>>>> Oracle
>>>>> 
>>>> 
>>> 
>> 
>