Make UnixFileSystem Y2K38-ready. In addition to being unable to represent Unix times that don't fit an int, the current storage format is unnecessarily complex. Instead, use a single long field with milliseconds since the Unix epoch, which takes up the same amount of space but is safe for the next ~200 million years. PiperOrigin-RevId: 680948657 Change-Id: I84546f3463ed5a174c0ebd34e6bb4d4fe9bdbdaa
diff --git a/src/main/java/com/google/devtools/build/lib/unix/ErrnoFileStatus.java b/src/main/java/com/google/devtools/build/lib/unix/ErrnoFileStatus.java index f208ac7..814a70f 100644 --- a/src/main/java/com/google/devtools/build/lib/unix/ErrnoFileStatus.java +++ b/src/main/java/com/google/devtools/build/lib/unix/ErrnoFileStatus.java
@@ -43,14 +43,10 @@ ENODATA = constants.errnoENODATA; } - /** - * Constructs a ErrnoFileSatus instance. (Called only from JNI code.) - */ - private ErrnoFileStatus(int st_mode, int st_atime, int st_atimensec, int st_mtime, - int st_mtimensec, int st_ctime, int st_ctimensec, long st_size, - int st_dev, long st_ino) { - super(st_mode, st_atime, st_atimensec, st_mtime, st_mtimensec, st_ctime, st_ctimensec, st_size, - st_dev, st_ino); + /** Constructs a ErrnoFileSatus instance. (Called only from JNI code.) */ + private ErrnoFileStatus( + int mode, long atime, long mtime, long ctime, long size, int dev, long ino) { + super(mode, atime, mtime, ctime, size, dev, ino); this.errno = 0; } @@ -58,7 +54,7 @@ * Constructs a ErrnoFileSatus instance. (Called only from JNI code.) */ private ErrnoFileStatus(int errno) { - super(0, 0, 0, 0, 0, 0, 0, 0, 0, 0); + super(0, 0, 0, 0, 0, 0, 0); this.errno = errno; }
diff --git a/src/main/java/com/google/devtools/build/lib/unix/FileStatus.java b/src/main/java/com/google/devtools/build/lib/unix/FileStatus.java index 3182e2e..e21f280 100644 --- a/src/main/java/com/google/devtools/build/lib/unix/FileStatus.java +++ b/src/main/java/com/google/devtools/build/lib/unix/FileStatus.java
@@ -14,57 +14,43 @@ package com.google.devtools.build.lib.unix; /** - * <p>Equivalent to UNIX's "struct stat", a FileStatus instance contains - * various bits of metadata about a directory entry. + * Equivalent to UNIX's "struct stat", a FileStatus instance contains various bits of metadata about + * a directory entry. * - * <p>The Java SDK provides access to some but not all of the information - * available via the stat(2) and lstat(2) syscalls, but often requires that - * multiple calls be made to obtain it. By reifying stat buffers as Java - * objects and providing a wrapper around the stat/lstat calls, we give client - * applications access to the richer file metadata and enable a reduction in - * the number of system calls, which is critical for high-performance tools. + * <p>The Java SDK provides access to some but not all of the information available via the stat(2) + * and lstat(2) syscalls, but often requires that multiple calls be made to obtain it. By reifying + * stat buffers as Java objects and providing a wrapper around the stat/lstat calls, we give client + * applications access to the richer file metadata and enable a reduction in the number of system + * calls, which is critical for high-performance tools. * - * <p>This class is optimized for memory usage. Operations that are not yet - * required for any client are intentionally unimplemented to save space. - * Currently, we only support these fields: st_mode, st_size, st_atime, - * st_atimensec, st_mtime, st_mtimensec, st_ctime, st_ctimensec, st_dev, st_ino. - * Methods that require other fields throw UnsupportedOperationException. + * <p>This class is optimized for memory usage. Fields not required by Bazel are omitted. */ public class FileStatus { - private final int st_mode; - private final int st_atime; // (unsigned) - private final int st_atimensec; // (unsigned) - private final int st_mtime; // (unsigned) - private final int st_mtimensec; // (unsigned) - private final int st_ctime; // (unsigned) - private final int st_ctimensec; // (unsigned) - private final long st_size; - private final int st_dev; - private final long st_ino; + private final int mode; + private final long atime; // milliseconds since Unix epoch + private final long mtime; // milliseconds since Unix epoch + private final long ctime; // milliseconds since Unix epoch + private final long size; + private final int dev; + private final long ino; - /** - * Constructs a FileStatus instance. (Called only from JNI code.) - */ - protected FileStatus(int st_mode, int st_atime, int st_atimensec, int st_mtime, int st_mtimensec, - int st_ctime, int st_ctimensec, long st_size, int st_dev, long st_ino) { - this.st_mode = st_mode; - this.st_atime = st_atime; - this.st_atimensec = st_atimensec; - this.st_mtime = st_mtime; - this.st_mtimensec = st_mtimensec; - this.st_ctime = st_ctime; - this.st_ctimensec = st_ctimensec; - this.st_size = st_size; - this.st_dev = st_dev; - this.st_ino = st_ino; + /** Constructs a FileStatus instance. (Called only from ErrnoFileStatus and JNI code.) */ + protected FileStatus(int mode, long atime, long mtime, long ctime, long size, int dev, long ino) { + this.mode = mode; + this.atime = atime; + this.mtime = mtime; + this.ctime = ctime; + this.size = size; + this.dev = dev; + this.ino = ino; } /** * Returns the device number of this inode. */ public int getDeviceNumber() { - return st_dev; + return dev; } /** @@ -72,19 +58,19 @@ * a given device. */ public long getInodeNumber() { - return st_ino; + return ino; } /** * Returns true iff this file is a regular file. */ public boolean isRegularFile() { - return (st_mode & S_IFMT) == S_IFREG; + return (mode & S_IFMT) == S_IFREG; } /** Returns true iff this file is a directory. */ public boolean isDirectory() { - return (st_mode & S_IFMT) == S_IFDIR; + return (mode & S_IFMT) == S_IFDIR; } public static boolean isDirectory(int rawType) { @@ -94,7 +80,7 @@ /** Returns true iff this file is a symbolic link. */ public boolean isSymbolicLink() { - return (st_mode & S_IFMT) == S_IFLNK; + return (mode & S_IFMT) == S_IFLNK; } public static boolean isSymbolicLink(int rawType) { @@ -106,42 +92,40 @@ * Returns true iff this file is a character device. */ public boolean isCharacterDevice() { - return (st_mode & S_IFMT) == S_IFCHR; + return (mode & S_IFMT) == S_IFCHR; } /** * Returns true iff this file is a block device. */ public boolean isBlockDevice() { - return (st_mode & S_IFMT) == S_IFBLK; + return (mode & S_IFMT) == S_IFBLK; } - /** - * Returns true iff this file is a FIFO. - */ - public boolean isFIFO() { - return (st_mode & S_IFMT) == S_IFIFO; + /** Returns true iff this file is a FIFO. */ + public boolean isFifo() { + return (mode & S_IFMT) == S_IFIFO; } /** * Returns true iff this file is a UNIX-domain socket. */ public boolean isSocket() { - return (st_mode & S_IFMT) == S_IFSOCK; + return (mode & S_IFMT) == S_IFSOCK; } /** * Returns true iff this file has its "set UID" bit set. */ public boolean isSetUserId() { - return (st_mode & S_ISUID) != 0; + return (mode & S_ISUID) != 0; } /** * Returns true iff this file has its "set GID" bit set. */ public boolean isSetGroupId() { - return (st_mode & S_ISGID) != 0; + return (mode & S_ISGID) != 0; } /** @@ -149,78 +133,46 @@ * explanation. */ public boolean isSticky() { - return (st_mode & S_ISVTX) != 0; + return (mode & S_ISVTX) != 0; } /** - * Returns the user/group/other permissions part of the mode bits (i.e. - * st_mode masked with 0777), interpreted according to longstanding UNIX - * tradition. + * Returns the user/group/other permissions part of the mode bits (i.e. mode masked with 0777), + * interpreted according to longstanding UNIX tradition. */ public int getPermissions() { - return st_mode & S_IRWXA; + return mode & S_IRWXA; } /** * Returns the total size, in bytes, of this file. */ public long getSize() { - return st_size; + return size; } - /** - * Returns the last access time of this file (seconds since UNIX epoch). - */ + /** Returns the last access time of this file (milliseconds since UNIX epoch). */ public long getLastAccessTime() { - return unsignedIntToLong(st_atime); + return atime; } - /** - * Returns the fractional part of the last access time of this file (nanoseconds). - */ - public long getFractionalLastAccessTime() { - return unsignedIntToLong(st_atimensec); - } - - /** - * Returns the last modified time of this file (seconds since UNIX epoch). - */ + /** Returns the last modified time of this file (milliseconds since UNIX epoch). */ public long getLastModifiedTime() { - return unsignedIntToLong(st_mtime); + return mtime; } - /** - * Returns the fractional part of the last modified time of this file (nanoseconds). - */ - public long getFractionalLastModifiedTime() { - return unsignedIntToLong(st_mtimensec); - } - - /** - * Returns the last change time of this file (seconds since UNIX epoch). - */ + /** Returns the last change time of this file (milliseconds since UNIX epoch). */ public long getLastChangeTime() { - return unsignedIntToLong(st_ctime); - } - - /** - * Returns the fractional part of the last change time of this file (nanoseconds). - */ - public long getFractionalLastChangeTime() { - return unsignedIntToLong(st_ctimensec); + return ctime; } //////////////////////////////////////////////////////////////////////// @Override public String toString() { - return String.format("FileStatus(mode=0%06o,size=%d,mtime=%d)", - st_mode, st_size, st_mtime); - } - - @Override - public int hashCode() { - return st_mode; + return String.format( + "FileStatus(mode=0%06o,atime=%d,mtime=%d,ctime=%d,size=%d,device=%d,ino=%d)", + mode, atime, mtime, ctime, size, dev, ino); } //////////////////////////////////////////////////////////////////////// @@ -261,10 +213,6 @@ public static final int S_IEXEC = 00111; // owner, group, world execute - static long unsignedIntToLong(int i) { - return (i & 0x7FFFFFFF) - (long) (i & 0x80000000); - } - public static boolean isFile(int rawType) { int type = rawType & S_IFMT; return type == S_IFREG || isSpecialFile(rawType);
diff --git a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java index 01a423f..1738049 100644 --- a/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java
@@ -66,8 +66,8 @@ /** * Eager implementation of FileStatus for file systems that have an atomic stat(2) syscall. A - * proxy for {@link com.google.devtools.build.lib.unix.FileStatus}. Note that isFile and - * getLastModifiedTime have slightly different meanings between UNIX and VFS. + * proxy for {@link com.google.devtools.build.lib.unix.FileStatus}. Note that isFile has a + * slightly different meaning between UNIX and VFS. */ @VisibleForTesting protected static class UnixFileStatus implements FileStatus { @@ -105,13 +105,12 @@ @Override public long getLastModifiedTime() { - return (status.getLastModifiedTime() * 1000) - + (status.getFractionalLastModifiedTime() / 1000000); + return status.getLastModifiedTime(); } @Override public long getLastChangeTime() { - return (status.getLastChangeTime() * 1000) + (status.getFractionalLastChangeTime() / 1000000); + return status.getLastChangeTime(); } @Override
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java b/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java index 5827576..9cb7249 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/FileStatus.java
@@ -72,8 +72,11 @@ long getLastModifiedTime() throws IOException; /** - * Returns the last change time of this file, where change means any change - * to the file, including metadata changes (milliseconds since UNIX epoch). + * Returns the last change time of this file, where change means any change to the file, including + * metadata changes (milliseconds since UNIX epoch). + * + * <p>On systems where the last change time is not supported, the last modified time will be + * returned instead. */ long getLastChangeTime() throws IOException;
diff --git a/src/main/native/darwin/file_jni.cc b/src/main/native/darwin/file_jni.cc index 2f9c910..8d93d24 100644 --- a/src/main/native/darwin/file_jni.cc +++ b/src/main/native/darwin/file_jni.cc
@@ -77,25 +77,18 @@ return r; } -int StatSeconds(const portable_stat_struct &statbuf, StatTimes t) { +uint64_t StatEpochMilliseconds(const portable_stat_struct &statbuf, + StatTimes t) { switch (t) { case STAT_ATIME: - return statbuf.st_atime; + return statbuf.st_atimespec.tv_sec * 1000L + + statbuf.st_atimespec.tv_nsec / 1000000; case STAT_CTIME: - return statbuf.st_ctime; + return statbuf.st_ctimespec.tv_sec * 1000L + + statbuf.st_ctimespec.tv_nsec / 1000000; case STAT_MTIME: - return statbuf.st_mtime; - } -} - -int StatNanoSeconds(const portable_stat_struct &statbuf, StatTimes t) { - switch (t) { - case STAT_ATIME: - return statbuf.st_atimespec.tv_nsec; - case STAT_CTIME: - return statbuf.st_ctimespec.tv_nsec; - case STAT_MTIME: - return statbuf.st_mtimespec.tv_nsec; + return statbuf.st_mtimespec.tv_sec * 1000L + + statbuf.st_mtimespec.tv_nsec / 1000000; } }
diff --git a/src/main/native/unix_jni.cc b/src/main/native/unix_jni.cc index d97ea7b..9b33a5a 100644 --- a/src/main/native/unix_jni.cc +++ b/src/main/native/unix_jni.cc
@@ -321,11 +321,12 @@ static jobject NewFileStatus(JNIEnv *env, const portable_stat_struct &stat_ref) { return env->NewObject( - file_status_class, file_status_class_ctor, stat_ref.st_mode, - StatSeconds(stat_ref, STAT_ATIME), StatNanoSeconds(stat_ref, STAT_ATIME), - StatSeconds(stat_ref, STAT_MTIME), StatNanoSeconds(stat_ref, STAT_MTIME), - StatSeconds(stat_ref, STAT_CTIME), StatNanoSeconds(stat_ref, STAT_CTIME), - static_cast<jlong>(stat_ref.st_size), static_cast<int>(stat_ref.st_dev), + file_status_class, file_status_class_ctor, + static_cast<jint>(stat_ref.st_mode), + static_cast<jlong>(StatEpochMilliseconds(stat_ref, STAT_ATIME)), + static_cast<jlong>(StatEpochMilliseconds(stat_ref, STAT_MTIME)), + static_cast<jlong>(StatEpochMilliseconds(stat_ref, STAT_CTIME)), + static_cast<jlong>(stat_ref.st_size), static_cast<jint>(stat_ref.st_dev), static_cast<jlong>(stat_ref.st_ino)); } @@ -338,11 +339,11 @@ } return env->NewObject( errno_file_status_class, errno_file_status_class_no_error_ctor, - stat_ref.st_mode, StatSeconds(stat_ref, STAT_ATIME), - StatNanoSeconds(stat_ref, STAT_ATIME), StatSeconds(stat_ref, STAT_MTIME), - StatNanoSeconds(stat_ref, STAT_MTIME), StatSeconds(stat_ref, STAT_CTIME), - StatNanoSeconds(stat_ref, STAT_CTIME), - static_cast<jlong>(stat_ref.st_size), static_cast<int>(stat_ref.st_dev), + static_cast<jint>(stat_ref.st_mode), + static_cast<jlong>(StatEpochMilliseconds(stat_ref, STAT_ATIME)), + static_cast<jlong>(StatEpochMilliseconds(stat_ref, STAT_MTIME)), + static_cast<jlong>(StatEpochMilliseconds(stat_ref, STAT_CTIME)), + static_cast<jlong>(stat_ref.st_size), static_cast<jint>(stat_ref.st_dev), static_cast<jlong>(stat_ref.st_ino)); } @@ -381,9 +382,9 @@ errno_file_status_class = makeStaticClass( env, "com/google/devtools/build/lib/unix/ErrnoFileStatus"); file_status_class_ctor = - getConstructorID(env, file_status_class, "(IIIIIIIJIJ)V"); + getConstructorID(env, file_status_class, "(IJJJJIJ)V"); errno_file_status_class_no_error_ctor = - getConstructorID(env, errno_file_status_class, "(IIIIIIIJIJ)V"); + getConstructorID(env, errno_file_status_class, "(IJJJJIJ)V"); errno_file_status_class_errorno_ctor = getConstructorID(env, errno_file_status_class, "(I)V"); dirents_class = makeStaticClass(
diff --git a/src/main/native/unix_jni.h b/src/main/native/unix_jni.h index c7a1a73..742af2c 100644 --- a/src/main/native/unix_jni.h +++ b/src/main/native/unix_jni.h
@@ -19,6 +19,7 @@ #include <errno.h> #include <jni.h> +#include <stdint.h> #include <sys/stat.h> #include <string> @@ -58,18 +59,16 @@ int portable_fstatat(int dirfd, char *name, portable_stat_struct *statbuf, int flags); -// Encoding for different timestamps in a struct stat{}. +// Encoding for different timestamps in a struct stat. enum StatTimes { STAT_ATIME, // access STAT_MTIME, // modification STAT_CTIME, // status change }; -// Returns seconds from a stat buffer. -int StatSeconds(const portable_stat_struct &statbuf, StatTimes t); - -// Returns nanoseconds from a stat buffer. -int StatNanoSeconds(const portable_stat_struct &statbuf, StatTimes t); +// Returns milliseconds since Unix epoch from the given struct stat field. +uint64_t StatEpochMilliseconds(const portable_stat_struct &statbuf, + StatTimes t); // Runs getxattr(2). If the attribute is not found, returns -1 and sets // attr_not_found to true. For all other errors, returns -1, sets attr_not_found
diff --git a/src/main/native/unix_jni_bsd.cc b/src/main/native/unix_jni_bsd.cc index 2acda1c..7f783f8 100644 --- a/src/main/native/unix_jni_bsd.cc +++ b/src/main/native/unix_jni_bsd.cc
@@ -57,25 +57,18 @@ return fstatat(dirfd, name, statbuf, flags); } -int StatSeconds(const portable_stat_struct &statbuf, StatTimes t) { +uint64_t StatEpochMilliseconds(const portable_stat_struct &statbuf, + StatTimes t) { switch (t) { case STAT_ATIME: - return statbuf.st_atime; + return statbuf.st_atimespec.tv_sec * 1000L + + statbuf.st_atimespec.tv_nsec / 1000000; case STAT_CTIME: - return statbuf.st_ctime; + return statbuf.st_ctimespec.tv_sec * 1000L + + statbuf.st_ctimespec.tv_nsec / 1000000; case STAT_MTIME: - return statbuf.st_mtime; - } -} - -int StatNanoSeconds(const portable_stat_struct &statbuf, StatTimes t) { - switch (t) { - case STAT_ATIME: - return statbuf.st_atimespec.tv_nsec; - case STAT_CTIME: - return statbuf.st_ctimespec.tv_nsec; - case STAT_MTIME: - return statbuf.st_mtimespec.tv_nsec; + return statbuf.st_mtime.tv_sec * 1000L + + statbuf.st_mtimespec.tv_nsec / 1000000; } }
diff --git a/src/main/native/unix_jni_linux.cc b/src/main/native/unix_jni_linux.cc index 1aa0f25..a886558 100644 --- a/src/main/native/unix_jni_linux.cc +++ b/src/main/native/unix_jni_linux.cc
@@ -13,6 +13,7 @@ // limitations under the License. #include <errno.h> +#include <stdint.h> #include <stdlib.h> #include <string.h> #include <sys/stat.h> @@ -47,28 +48,16 @@ return fstatat64(dirfd, name, statbuf, flags); } -int StatSeconds(const portable_stat_struct &statbuf, StatTimes t) { +uint64_t StatEpochMilliseconds(const portable_stat_struct &statbuf, + StatTimes t) { switch (t) { case STAT_ATIME: - return statbuf.st_atim.tv_sec; + return statbuf.st_atim.tv_sec * 1000L + statbuf.st_atim.tv_nsec / 1000000; case STAT_CTIME: - return statbuf.st_ctim.tv_sec; + return statbuf.st_ctim.tv_sec * 1000L + statbuf.st_ctim.tv_nsec / 1000000; case STAT_MTIME: - return statbuf.st_mtim.tv_sec; + return statbuf.st_mtim.tv_sec * 1000L + statbuf.st_mtim.tv_nsec / 1000000; } - return 0; -} - -int StatNanoSeconds(const portable_stat_struct &statbuf, StatTimes t) { - switch (t) { - case STAT_ATIME: - return statbuf.st_atim.tv_nsec; - case STAT_CTIME: - return statbuf.st_ctim.tv_nsec; - case STAT_MTIME: - return statbuf.st_mtim.tv_nsec; - } - return 0; } ssize_t portable_getxattr(const char *path, const char *name, void *value,
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java index a9d911b..55575cb 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java
@@ -1171,12 +1171,21 @@ // Test the date functions @Test - public void testSetLastModifiedTime() throws Exception { + public void testSetLastModifiedTime_32bit() throws Exception { Path file = absolutize("file"); FileSystemUtils.createEmptyFile(file); - file.setLastModifiedTime(1234567890L); - assertThat(file.getLastModifiedTime()).isEqualTo(1234567890L); + file.setLastModifiedTime(1 << 30); + assertThat(file.getLastModifiedTime()).isEqualTo(1 << 30); + } + + @Test + public void testSetLastModifiedTime_64bit() throws Exception { + Path file = absolutize("file"); + FileSystemUtils.createEmptyFile(file); + + file.setLastModifiedTime(1L << 34); + assertThat(file.getLastModifiedTime()).isEqualTo(1L << 34); } @Test