This is an archive of the discontinued LLVM Phabricator instance.

Differential D101323

[AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX
ClosedPublic

Authored by hubert.reinterpretcast on Apr 26 2021, 2:05 PM.

Details

Reviewers
daltenty
Xiangling_L
jsji
Group Reviewers
Restricted Project
Commits
rGbdc4ec04d42a: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX
Summary

The value observed for the mtim.tv_nsec member is erroneous in some AIX environments. Avoid using this member by forcing HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC to 0.

This resolves "mtime changed" errors such as the one http://lab.llvm.org:8014/#/builders/126/builds/330/steps/5/logs/FAIL__Clang__test_c has.

Diff Detail

Event Timeline

hubert.reinterpretcast created this revision.Apr 26 2021, 2:05 PM
Herald added a subscriber: mgorny. · View Herald TranscriptApr 26 2021, 2:05 PM
hubert.reinterpretcast requested review of this revision.Apr 26 2021, 2:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2021, 2:05 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
jsji added a reviewer: Restricted Project.Apr 26 2021, 2:08 PM
daltenty accepted this revision as: daltenty.Apr 26 2021, 2:23 PM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 26 2021, 2:23 PM
jsji added a comment.Apr 26 2021, 2:29 PM

Have we considered workaround this in llvm/lib/Support/Unix/Path.inc instead?

hubert.reinterpretcast added a comment.Apr 26 2021, 2:32 PM
In D101323#2717925, @jsji wrote:

Have we considered workaround this in llvm/lib/Support/Unix/Path.inc instead?

Yes. I think this is a more general fix that would automatically apply in other situations where code should be checking for this macro.

jsji accepted this revision as: jsji.Apr 26 2021, 2:44 PM
jsji added inline comments.
llvm/cmake/config-ix.cmake
285

Maybe add a warning here about we are forcing it to 0?

Harbormaster completed remote builds in B101018: Diff 340640.Apr 26 2021, 2:56 PM
This revision was landed with ongoing or failed builds.Apr 26 2021, 3:30 PM
Closed by commit rGbdc4ec04d42a: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX (authored by hubert.reinterpretcast). · Explain Why
This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added a commit: rGbdc4ec04d42a: [AIX] Avoid use of mtim.tv_nsec member of stat structure on AIX.
hubert.reinterpretcast added inline comments.Apr 26 2021, 3:30 PM
llvm/cmake/config-ix.cmake
285

As discussed off-list: For a platform where Clang and LLVM is relatively new, I don't think this leads to a loss of functionality at the level where a warning is warranted. The autodetection is going to set this to 0 on some platforms anyway.

Revision Contents

PathSize
llvm/
cmake/
10 lines

Diff 340674

llvm/cmake/config-ix.cmake

Show First 20 Lines • Show All 274 Lines • ▼ Show 20 Linesif( HAVE_DLFCN_H )
check_symbol_exists(dladdr dlfcn.h HAVE_DLADDR) check_symbol_exists(dladdr dlfcn.h HAVE_DLADDR)
if( HAVE_LIBDL ) if( HAVE_LIBDL )
list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES dl) list(REMOVE_ITEM CMAKE_REQUIRED_LIBRARIES dl)
endif() endif()
endif() endif()
CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtimespec.tv_nsec
"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC) "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC)
if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
# The st_mtim.tv_nsec member of a `stat` structure is not reliable on some AIX
# environments.
jsjiUnsubmitted
Done
ReplyInline Actions

Maybe add a warning here about we are forcing it to 0?

jsji: Maybe add a warning here about we are forcing it to 0?
hubert.reinterpretcastAuthorUnsubmitted
Done
ReplyInline Actions

As discussed off-list: For a platform where Clang and LLVM is relatively new, I don't think this leads to a loss of functionality at the level where a warning is warranted. The autodetection is going to set this to 0 on some platforms anyway.

hubert.reinterpretcast: As discussed off-list: For a platform where Clang and LLVM is relatively new, I don't think…
set(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC 0)
else()
CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec CHECK_STRUCT_HAS_MEMBER("struct stat" st_mtim.tv_nsec
"sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC) "sys/types.h;sys/stat.h" HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC)
endif()
check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC) check_symbol_exists(__GLIBC__ stdio.h LLVM_USING_GLIBC)
if( LLVM_USING_GLIBC ) if( LLVM_USING_GLIBC )
add_definitions( -D_GNU_SOURCE ) add_definitions( -D_GNU_SOURCE )
list(APPEND CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE") list(APPEND CMAKE_REQUIRED_DEFINITIONS "-D_GNU_SOURCE")
endif() endif()
# This check requires _GNU_SOURCE # This check requires _GNU_SOURCE
if (NOT PURE_WINDOWS) if (NOT PURE_WINDOWS)
▲ Show 20 Lines • Show All 392 Lines • Show Last 20 Lines