RFR: 8336529: (fs) UnixFileAttributeViews setTimes() failing on armhf, Ubuntu noble [v2]

[Alan Bateman]

On Wed, 17 Jul 2024 22:43:47 GMT, Vladimir Petko <vpetko at openjdk.org> wrote:

>> time_t transition in Debian/Ubuntu left 32 bit time_t  symbols in glibc. 
>> 
>> Looking up 'futimens' via dlsym returns 32 bit version of the function.  
>> This is causing failure to set last modified time (e.g. instead of year 2017 we get 1976 in the test).
>> Using the function directly correctly calls 64 bit versions. 
>> 
>> When we lookup functions with time_t arguments through dlsym() calls we should use 64 bit versions.
>> 
>> This MR adds a function to lookup 64 bit functions when time_t size is greater than 32 bit in line with the existing [time_t patch](https://evolvis.org/plugins/scmgit/cgi-bin/gitweb.cgi?p=alioth/openjdk-8.git;a=blob;f=debian/patches/t64-dlsyms.diff;h=b2cc8f8a00244bc44900366ed08d7f246ad09904;hb=4c5085cd49fcdc04405e9039705935fde08644a7)
>> 
>> Testing:
>> `` sh configure --with-debug-level=fastdebug --enable-fallback-linker --with-jtreg=../jtreg/build/images/jtreg``
>> 
>> 
>> ==============================
>> Test summary
>> ==============================
>>    TEST                                              TOTAL  PASS  FAIL ERROR   
>>    jtreg:test/jdk/jdk/nio/zipfs/CopyMoveTests.java       1     1     0     0   
>>    jtreg:test/jdk/jdk/nio/zipfs/TestPosix.java           1     1     0     0   
>>    jtreg:test/jdk/java/nio/file/attribute/BasicFileAttributeView/Basic.java
>>                                                          1     1     0     0   
>>    jtreg:test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTime.java
>>                                                          1     1     0     0   
>>    jtreg:test/jdk/java/nio/file/Files/SetLastModifiedTime.java
>>                                                          1     1     0     0   
>>    jtreg:test/jdk/java/nio/file/Files/SymlinkTime.java
>>                                                          1     1     0     0   
>>    jtreg:test/langtools/tools/javac/modules/MOptionTest.java
>>                                                          1     1     0     0   
>>    jtreg:test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/DynamicSharedSymbols.java
>>                                                          1     1     0     0   
>>    jtreg:test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestDynamicDump.java
>>                                                          1     1     0     0   
>>    jtreg:test/hotspot/jtreg/runtime/cds/appcds/jcmd/JCmdTestFileSafety.java
>>                                                          1     1     0     0   
>>    jtreg:test/...
>
> Vladimir Petko has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - review comments: add ifdef, rename function
>  - Merge branch 'master' into JDK-8336529
>  - typo: remove extra whitespace
>  - fix typo: missing whitespace
>  - try to use 64 bit function if sizeof(time_t) > 4

I don't object to putting in a patch to deal with this porting challenge, we just need to make sure that it don't disrupt other builds or platforms.  To that end, I think it would be better to only compile in lookup_time_t_function on Linux, you can put it after statx_wrapper. I assume you can reduce it to:

    if (sizeof(time_t) > 4) {
        return dlsym(RTLD_DEFAULT, symbol64);
    } else {
        return dlsym(RTLD_DEFAULT, symbol32);
    }

In Java_sun_nio_fs_UnixNativeDispatcher_init think you can use something like this to avoid changing what is compiled in on other platforms. 

#if defined(__linux)
    my_futimesat_func = ..
#elif defined(_ALLBSD_SOURCE)
    my_futimesat_func = ..
#else
   my_futimesat_func = ..
#endif

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20208#issuecomment-2237125156