RFR: 8234185: Cleanup usage of canonicalize function between libjava, hotspot and libinstrument

David Holmes david.holmes at oracle.com
Tue Dec 3 22:51:51 UTC 2019 

Hi Christoph,

On 3/12/2019 7:26 pm, Langer, Christoph wrote:
> Hi David,
> 
> thanks for taking care of this.
> 
> This would be my updated patch that could hopefully be enabled by just adding the include directory where "jdk_util.h" is located. It would be really great if that'd work. I think it would open up a path to automatically include io_util_md.h by including io_util.h.
> 
> http://cr.openjdk.java.net/~clanger/webrevs/8234185.1/

I'm afraid I cannot get this to work at our end. I get the following errors:

t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): 
error C2143: syntax error: missing ')' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): 
error C2143: syntax error: missing '{' before '*'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): 
warning C4142: 'size_t': benign redefinition of type
C:\ade\mesos\work_dir\jib-ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\include\vcruntime.h(184): 
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): 
error C2370: 'size_t': redefinition; different storage class
C:\ade\mesos\work_dir\jib-ma~1\install\jpg\infra\buildd~1\devkit~1\vs2017~3.0\devkit~1.gz\VC\include\vcruntime.h(184): 
note: see declaration of 'size_t'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): 
error C2146: syntax error: missing ';' before identifier 'info_size'
t:\workspace\open\src\java.base\share\native\libjava\jdk_util.h(46): 
error C2059: syntax error: ')'

This pertains to the line:

JDK_GetVersionInfo0(jdk_version_info* info, size_t info_size);

There is also a second problem in our closed code that will take more 
effort from someone familiar with that code to resolve. I will file an 
issue for our install team to pick up.

I would ask that this not be pushed for the moment while others figure 
out how best to handle this.

Thanks,
David
-----


> Cheers
> Christoph
> 
> 
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Dienstag, 3. Dezember 2019 03:13
>> To: Langer, Christoph <christoph.langer at sap.com>; Alan Bateman
>> <Alan.Bateman at oracle.com>; gerard ziemski <gerard.ziemski at oracle.com>
>> Cc: core-libs-dev at openjdk.java.net; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function between
>> libjava, hotspot and libinstrument
>>
>> Hi Christoph,
>>
>> Can you please post your updated patch for review and I will use it to
>> check the fix for our internal build. Once you have your fix reviewed I
>> will push both fixes together.
>>
>> Thanks,
>> David
>>
>> On 25/11/2019 10:41 pm, David Holmes wrote:
>>> Hi Christoph,
>>>
>>> On 25/11/2019 10:38 pm, Langer, Christoph wrote:
>>>> Hi David,
>>>>
>>>> thanks for your investigation. I'll prepare a fix to move back
>>>> getPrefixed into canonicalize_md.c. However, could you please still
>>>> fix your internal build in terms that it would have 'jdk_util.h' in
>>>> the include path?
>>>
>>> That should be simple enough to do.
>>>
>>> David
>>>
>>>> Thanks
>>>> Christoph
>>>>
>>>>> -----Original Message-----
>>>>> From: David Holmes <david.holmes at oracle.com>
>>>>> Sent: Montag, 25. November 2019 01:02
>>>>> To: Langer, Christoph <christoph.langer at sap.com>; Alan Bateman
>>>>> <Alan.Bateman at oracle.com>; gerard ziemski
>> <gerard.ziemski at oracle.com>
>>>>> Cc: core-libs-dev at openjdk.java.net; hotspot-runtime-
>>>>> dev at openjdk.java.net
>>>>> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize function
>>>>> between
>>>>> libjava, hotspot and libinstrument
>>>>>
>>>>>
>>>>>
>>>>> On 25/11/2019 8:45 am, David Holmes wrote:
>>>>>> On 25/11/2019 7:49 am, David Holmes wrote:
>>>>>>> On 25/11/2019 7:33 am, David Holmes wrote:
>>>>>>>> Hi Christoph,
>>>>>>>>
>>>>>>>> On 23/11/2019 12:04 am, Langer, Christoph wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> I'd like to push this change. However, running it through jdk-submit
>>>>>>>>> shows reproducible errors:
>>>>>>>>>
>>>>>>>>> Job: mach5-one-clanger-JDK-8234185-1-20191122-0927-6913189
>>>>>>>>> BuildId: 2019-11-22-0926373.christoph.langer.source
>>>>>>>>> No failed tests
>>>>>>>>> Tasks Summary
>>>>>>>>> •    NA: 0
>>>>>>>>> •    NOTHING_TO_RUN: 0
>>>>>>>>> •    KILLED: 0
>>>>>>>>> •    PASSED: 76
>>>>>>>>> •    UNABLE_TO_RUN: 0
>>>>>>>>> •    EXECUTED_WITH_FAILURE: 1
>>>>>>>>> •    FAILED: 0
>>>>>>>>> •    HARNESS_ERROR: 0
>>>>>>>>> Build
>>>>>>>>> 1 Executed with failure
>>>>>>>>> o    windows-x64-install-windows-x64-build-19 error while building,
>>>>>>>>> return value: 2
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Job: mach5-one-clanger-JDK-8234185-20191121-2313-6898791
>>>>>>>>> BuildId: 2019-11-21-2311357.christoph.langer.source
>>>>>>>>> No failed tests
>>>>>>>>> Tasks Summary
>>>>>>>>> •    NA: 0
>>>>>>>>> •    NOTHING_TO_RUN: 0
>>>>>>>>> •    KILLED: 0
>>>>>>>>> •    PASSED: 76
>>>>>>>>> •    UNABLE_TO_RUN: 0
>>>>>>>>> •    EXECUTED_WITH_FAILURE: 1
>>>>>>>>> •    FAILED: 0
>>>>>>>>> •    HARNESS_ERROR: 0
>>>>>>>>> Build
>>>>>>>>> 1 Executed with failure
>>>>>>>>> o    windows-x64-install-windows-x64-build-19 error while building,
>>>>>>>>> return value: 2
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> David already had a look and let me know that the following was
>> the
>>>>>>>>> reason:
>>>>>>>>>
>>>>>>>>>
>>>>>
>> t:/workspace/open/src/java.base/windows/native/libjava/canonicalize_md.
>>>>> c(41):
>>>>>>>>> fatal error C1083: Cannot open include file: 'jdk_util.h': No such
>>>>>>>>> file or directory
>>>>>>>>>
>>>>>>>>> This is not explainable to me as I see this running through my local
>>>>>>>>> build and our nightly builds without problems. I also can't explain
>>>>>>>>> jdk_util.h can't be opened at this place - it should be there and
>>>>>>>>> part of the include directories...
>>>>>>>>>
>>>>>>>>> I'd appreciate any help...
>>>>>>>>
>>>>>>>> I just dug a little deeper and this is failing in part of our closed
>>>>>>>> build for the install repo. There is a library there that is using
>>>>>>>> canonicalize_md.c directly - i.e. it adds that file to its source
>>>>>>>> files list. The build instructions don't include that directory on
>>>>>>>> the include directory list - hence the failure. But it will also fail
>>>>>>>> due to the name change you made.
>>>>>>>
>>>>>>> Actually it appears that the other source code doesn't actually refer
>>>>>>> to the canonicalize function at all, so a simple fix may be possible
>>>>>>> at the build level on our side. I'm testing that now.
>>>>>>
>>>>>> It isn't the canonicalize function that is used, it is getPrefixed,
>>>>>> which has now been moved to the io_util_md.c file. So a fix will be a
>>>>>> bit more involved.
>>>>>
>>>>> I tried adding io_util_md.c to the library source list instead of
>>>>> canonicalize_md.c but that just caused a slew of other compilation
>>>>> failures, so I don't see any quick fix for us here.
>>>>>
>>>>> David
>>>>>
>>>>>>
>>>>>> David
>>>>>>
>>>>>>>
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>> Someone will need to work with you to make the necessary changes
>> to
>>>>>>>> our code.
>>>>>>>>
>>>>>>>> David
>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Christoph
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Langer, Christoph
>>>>>>>>>> Sent: Donnerstag, 21. November 2019 14:19
>>>>>>>>>> To: Alan Bateman <Alan.Bateman at oracle.com>; core-libs-
>>>>>>>>>> dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
>>>>>>>>>> Subject: RE: RFR: 8234185: Cleanup usage of canonicalize function
>>>>>>>>>> between
>>>>>>>>>> libjava, hotspot and libinstrument
>>>>>>>>>>
>>>>>>>>>> Hi Alan,
>>>>>>>>>>
>>>>>>>>>> thanks for the review. I'll push it then after running through
>>>>>>>>>> jdk-submit.
>>>>>>>>>>
>>>>>>>>>> /Christoph
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: Alan Bateman <Alan.Bateman at oracle.com>
>>>>>>>>>>> Sent: Donnerstag, 21. November 2019 09:51
>>>>>>>>>>> To: Langer, Christoph <christoph.langer at sap.com>; core-libs-
>>>>>>>>>>> dev at openjdk.java.net; hotspot-runtime-
>> dev at openjdk.java.net
>>>>>>>>>>> Subject: Re: RFR: 8234185: Cleanup usage of canonicalize
>> function
>>>>>>>>>>> between
>>>>>>>>>>> libjava, hotspot and libinstrument
>>>>>>>>>>>
>>>>>>>>>>> On 14/11/2019 15:37, Langer, Christoph wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> please review this cleanup change regarding function
>>>>>>>>>>>> "canonicalize" of
>>>>>>>>>>> libjava.
>>>>>>>>>>>>
>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234185
>>>>>>>>>>>> Webrev:
>> http://cr.openjdk.java.net/~clanger/webrevs/8234185.0/
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> The goal is to cleanup how this function is defined and used.
>> One
>>>>>>>>>>>> thing is,
>>>>>>>>>>> that there was an unnecessary wrapper function "Canonicalize"
>> in
>>>>>>>>>>> jni_util.c.
>>>>>>>>>>> It wrapped the call to "canonicalize". We can get rid of this
>>>>>>>>>>> wrapper.
>>>>>>>>>>> Unfortunately, it is not possible to just export "canonicalize"
>>>>>>>>>>> since this will
>>>>>>>>>>> conflict with a method signature from the math library, at least
>>>>>>>>>>> on modern
>>>>>>>>>>> Linuxes. So I decided to call the method JDK_Canonicalize and
>> will
>>>>>>>>>>> correctly
>>>>>>>>>>> define it in jdk_util.h which can be included everywhere.
>>>>>>>>>>>>
>>>>>>>>>>> I think this change is okay. My main concern when initially seeing
>>>>>>>>>>> this
>>>>>>>>>>> go by was that it would leak the \\?\ or \\?\UNC\ prefix into the
>>>>>>>>>>> canonical File when it wasn't there previously, this would of
>>>>>>>>>>> course
>>>>>>>>>>> have several implications. But I think you have it right and this
>>>>>>>>>>> is, as
>>>>>>>>>>> you position, just refactoring/cleanup.
>>>>>>>>>>>
>>>>>>>>>>> -Alan

More information about the core-libs-dev mailing list