RFR (S) 8237591: Mac: include OS X version in hs_err_pid crash log file

[Daniel D. Daugherty]

> open webrev at http://cr.openjdk.java.net/~gziemski/8237591_rev1 

src/hotspot/os/bsd/os_bsd.cpp
     No comments.

Thumbs up. Thanks for including the before and after snippets
in the review request.

Dan


On 7/30/20 12:54 PM, gerard ziemski wrote:
> Thank you for the review.
>
> May I please have a 2nd reviewer? (or would this be considered trivial?)
>
>
> cheers
>
> On 7/30/20 1:10 AM, David Holmes wrote:
>> On 30/07/2020 4:51 am, gerard ziemski wrote:
>>> On 7/27/20 6:21 PM, David Holmes wrote:
>>>> On 28/07/2020 2:12 am, gerard ziemski wrote:
>>>>> Thank you David for taking a look.
>>>>>
>>>>>
>>>>> On 7/19/20 11:37 PM, David Holmes wrote:
>>>>>> Hi Gerard,
>>>>>>
>>>>>> On 18/07/2020 5:19 am, gerard ziemski wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> Please review this small fix that adds the OS version and the OS 
>>>>>>> build number to the hs_err_pidXXX.log output in the “Summary” 
>>>>>>> section for Mac platform (it’s easier to use for developers than 
>>>>>>> the Darwin kernel version that we display right now).
>>>>>>>
>>>>>>> This is how things used to look:
>>>>>>>
>>>>>>>
>>>>>>> --------------- S U M M A R Y ------------
>>>>>>>
>>>>>>> Command Line: Crasher
>>>>>>>
>>>>>>> Host: Gerards-MBP-16, MacBookPro16,1 x86_64 2600 MHz, 12 cores, 
>>>>>>> 32G, Darwin 19.5.0
>>>>>>> Time: Thu Jul 16 14:01:46 2020 CDT elapsed time: 1.089465 
>>>>>>> seconds (0d 0h 0m 1s)
>>>>>>>
>>>>>>>
>>>>>>> And this is how the “Summary” section looks like with the 
>>>>>>> proposed change:
>>>>>>>
>>>>>>>
>>>>>>> --------------- S U M M A R Y ------------
>>>>>>>
>>>>>>> Command Line: Crasher
>>>>>>>
>>>>>>> Host: Gerards-MBP-16, MacBookPro16,1 x86_64 2600 MHz, 12 cores, 
>>>>>>> 32G, Darwin 19.5.0, macOS 10.15.5 (19F101)
>>>>>>> Time: Thu Jul 16 14:02:29 2020 CDT elapsed time: 0.360881 
>>>>>>> seconds (0d 0h 0m 0s)
>>>>>>>
>>>>>>>
>>>>>>> bug link at https://bugs.openjdk.java.net/browse/JDK-8237591
>>>>>>> open webrev at http://cr.openjdk.java.net/~gziemski/8237591_rev1
>>>>>>> testing Mach5 hs_tier1,2,3,4,5 in progress
>>>>>>
>>>>>> Just to be clear, the changes prior to:
>>>>>>
>>>>>> 1555 #ifdef __APPLE__
>>>>>>
>>>>>> are just fixing up existing indentation errors - correct?
>>>>>
>>>>> Yes, hope that's OK, as this was the only spot in the function 
>>>>> that stood out with inconsistent indentation.
>>>>
>>>> Yes that is fine.
>>>>
>>>>>>
>>>>>> The actual change seems okay, just one query:
>>>>>>
>>>>>> 1562     int mib_build[] = { CTL_KERN, KERN_OSVERSION };
>>>>>>
>>>>>> I couldn't find KERN_OSVERSION documented for sysctl - is it a 
>>>>>> "recent" addition?
>>>>>
>>>>> Yes it is. Apple added it back in 2018 (see bug comments or this 
>>>>> link 
>>>>> https://github.com/apple/darwin-xnu/commit/5bbb823c13f3ab1ab58878f96b35433a29882676?diff=split#diff-6651b0c84a045f400bc45faa9f61c9e1 
>>>>> )
>>>>
>>>> That link shows the addition of sysctl_osproductversion which I 
>>>> assume underpins "kern.osproductversion". But my question was on 
>>>> KERN_OSVERSION. That definition seems to already exist prior to the 
>>>> change you link. My concern is whether it was also fairly recently 
>>>> introduced and so referring to it would require a minimum macOS 
>>>> version on the build machine?
>>>
>>> Sorry, I thought you meant "kern.osproductversion", not 
>>> KERN_OSVERSION, but that's a valid question.
>>>
>>> I found Apple using KERN_OSVERSION in its own code since macOS 10.7, 
>>> i.e. 
>>> https://opensource.apple.com/source/Libc/Libc-763.11/gen/assumes.c.auto.html 
>>> , though I could not find any documentation of it either.
>>
>> I found it in sysctl.h from 10.5 dev kit as well, so that looks fine 
>> to use.
>>
>> Thanks for checking.
>> David
>> -----
>