Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8214976: Warn about uses of functions replaced for portability #7248

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -2300,12 +2300,12 @@ int os::create_binary_file(const char* path, bool rewrite_existing) {

// return current position of file pointer
jlong os::current_file_offset(int fd) {
return (jlong)::lseek(fd, (off_t)0, SEEK_CUR);
return (jlong)os::lseek(fd, (off_t)0, SEEK_CUR);
}

// move file pointer to the specified offset
jlong os::seek_to_file_offset(int fd, jlong offset) {
return (jlong)::lseek(fd, (off_t)offset, SEEK_SET);
return (jlong)os::lseek(fd, (off_t)offset, SEEK_SET);
}

// This code originates from JDK's sysAvailable
@@ -2326,11 +2326,11 @@ int os::available(int fd, jlong *bytes) {
}
}
}
if ((cur = ::lseek(fd, 0L, SEEK_CUR)) == -1) {
if ((cur = os::lseek(fd, 0L, SEEK_CUR)) == -1) {
return 0;
} else if ((end = ::lseek(fd, 0L, SEEK_END)) == -1) {
} else if ((end = os::lseek(fd, 0L, SEEK_END)) == -1) {
return 0;
} else if (::lseek(fd, cur, SEEK_SET) == -1) {
} else if (os::lseek(fd, cur, SEEK_SET) == -1) {
return 0;
}
*bytes = end - cur;
@@ -2487,7 +2487,7 @@ void os::pause() {
if (fd != -1) {
struct stat buf;
::close(fd);
while (::stat(filename, &buf) == 0) {
while (os::stat(filename, &buf) == 0) {
(void)::poll(NULL, 0, 100);
}
} else {
@@ -385,7 +385,7 @@ LinuxAttachOperation* LinuxAttachListener::dequeue() {
// write the given buffer to the socket
int LinuxAttachListener::write_fully(int s, char* buf, int len) {
do {
ssize_t n = ::write(s, buf, len);
ssize_t n = os::write(s, buf, len);
if (n == -1) {
if (errno != EINTR) return -1;
} else {
@@ -130,7 +130,7 @@ ZPhysicalMemoryBacking::ZPhysicalMemoryBacking(size_t max_capacity) :
}

// Truncate backing file
while (ftruncate(_fd, max_capacity) == -1) {
while (os::ftruncate(_fd, max_capacity) == -1) {
if (errno != EINTR) {
ZErrno err;
log_error_p(gc)("Failed to truncate backing file (%s)", err.to_string());
@@ -2608,9 +2608,9 @@ void linux_wrap_code(char* base, size_t size) {
int fd = ::open(buf, O_CREAT | O_RDWR, S_IRWXU);

if (fd != -1) {
off_t rv = ::lseek(fd, size-2, SEEK_SET);
off_t rv = os::lseek(fd, size-2, SEEK_SET);
if (rv != (off_t)-1) {
if (::write(fd, "", 1) == 1) {
if (os::write(fd, "", 1) == 1) {
mmap(base, size,
PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_PRIVATE|MAP_FIXED|MAP_NORESERVE, fd, 0);
@@ -4923,12 +4923,12 @@ int os::create_binary_file(const char* path, bool rewrite_existing) {

// return current position of file pointer
jlong os::current_file_offset(int fd) {
return (jlong)::lseek64(fd, (off64_t)0, SEEK_CUR);
return (jlong)os::lseek(fd, (off64_t)0, SEEK_CUR);
}

// move file pointer to the specified offset
jlong os::seek_to_file_offset(int fd, jlong offset) {
return (jlong)::lseek64(fd, (off64_t)offset, SEEK_SET);
return (jlong)os::lseek(fd, (off64_t)offset, SEEK_SET);
}

// This code originates from JDK's sysAvailable
@@ -4949,11 +4949,11 @@ int os::available(int fd, jlong *bytes) {
}
}
}
if ((cur = ::lseek64(fd, 0L, SEEK_CUR)) == -1) {
if ((cur = os::lseek(fd, 0L, SEEK_CUR)) == -1) {
return 0;
} else if ((end = ::lseek64(fd, 0L, SEEK_END)) == -1) {
} else if ((end = os::lseek(fd, 0L, SEEK_END)) == -1) {
return 0;
} else if (::lseek64(fd, cur, SEEK_SET) == -1) {
} else if (os::lseek(fd, cur, SEEK_SET) == -1) {
return 0;
}
*bytes = end - cur;
@@ -5144,7 +5144,7 @@ void os::pause() {
if (fd != -1) {
struct stat buf;
::close(fd);
while (::stat(filename, &buf) == 0) {
while (os::stat(filename, &buf) == 0) {
(void)::poll(NULL, 0, 100);
}
} else {
@@ -289,11 +289,11 @@ static int get_systemtype(void) {
}

// Check whether we have a task subdirectory
if ((taskDir = opendir("/proc/self/task")) == NULL) {
if ((taskDir = os::opendir("/proc/self/task")) == NULL) {
procEntriesType = UNDETECTABLE;
} else {
// The task subdirectory exists; we're on a Linux >= 2.6 system
closedir(taskDir);
os::closedir(taskDir);
procEntriesType = LINUX26_NPTL;
}

@@ -675,7 +675,7 @@ bool SystemProcessInterface::SystemProcesses::ProcessIterator::is_dir(const char
struct stat mystat;
int ret_val = 0;

ret_val = stat(name, &mystat);
ret_val = os::stat(name, &mystat);
if (ret_val < 0) {
return false;
}
@@ -688,7 +688,7 @@ int SystemProcessInterface::SystemProcesses::ProcessIterator::fsize(const char*
size = 0;
struct stat fbuf;

if (stat(name, &fbuf) < 0) {
if (os::stat(name, &fbuf) < 0) {
return OS_ERR;
}
size = fbuf.st_size;
@@ -660,28 +660,36 @@ void os::dll_unload(void *lib) {
}

jlong os::lseek(int fd, jlong offset, int whence) {
return (jlong) BSD_ONLY(::lseek) NOT_BSD(::lseek64)(fd, offset, whence);
#ifdef BSD
ALLOW_C_FUNCTION(lseek, return (jlong)::lseek(fd, offset, whence);)
#else
ALLOW_C_FUNCTION(lseek64, return (jlong)::lseek64(fd, offset, whence);)
#endif
}

int os::fsync(int fd) {
return ::fsync(fd);
ALLOW_C_FUNCTION(fsync, return ::fsync(fd);)
}

int os::ftruncate(int fd, jlong length) {
return BSD_ONLY(::ftruncate) NOT_BSD(::ftruncate64)(fd, length);
#ifdef BSD
ALLOW_C_FUNCTION(ftruncate, return ::ftruncate(fd, length);)
#else
ALLOW_C_FUNCTION(ftruncate64, return ::ftruncate64(fd, length);)
#endif
}

const char* os::get_current_directory(char *buf, size_t buflen) {
return getcwd(buf, buflen);
}

FILE* os::fdopen(int fd, const char* mode) {
return ::fdopen(fd, mode);
ALLOW_C_FUNCTION(fdopen, return ::fdopen(fd, mode);)
}

ssize_t os::write(int fd, const void *buf, unsigned int nBytes) {
ssize_t res;
RESTARTABLE(::write(fd, buf, (size_t) nBytes), res);
ALLOW_C_FUNCTION(write, RESTARTABLE(::write(fd, buf, (size_t) nBytes), res);)
return res;
}

@@ -690,46 +698,46 @@ ssize_t os::read_at(int fd, void *buf, unsigned int nBytes, jlong offset) {
}

void os::flockfile(FILE* fp) {
::flockfile(fp);
ALLOW_C_FUNCTION(flockfile, ::flockfile(fp);)
}

void os::funlockfile(FILE* fp) {
::funlockfile(fp);
ALLOW_C_FUNCTION(funlockfile, ::funlockfile(fp);)
}

DIR* os::opendir(const char* dirname) {
assert(dirname != NULL, "just checking");
return ::opendir(dirname);
ALLOW_C_FUNCTION(opendir, return ::opendir(dirname);)
}

struct dirent* os::readdir(DIR* dirp) {
assert(dirp != NULL, "just checking");
return ::readdir(dirp);
ALLOW_C_FUNCTION(readdir, return ::readdir(dirp);)
}

int os::closedir(DIR *dirp) {
assert(dirp != NULL, "just checking");
return ::closedir(dirp);
ALLOW_C_FUNCTION(closedir, return ::closedir(dirp);)
}

int os::socket_close(int fd) {
return ::close(fd);
}

int os::recv(int fd, char* buf, size_t nBytes, uint flags) {
RESTARTABLE_RETURN_INT(::recv(fd, buf, nBytes, flags));
ALLOW_C_FUNCTION(recv, RESTARTABLE_RETURN_INT(::recv(fd, buf, nBytes, flags));)
}

int os::send(int fd, char* buf, size_t nBytes, uint flags) {
RESTARTABLE_RETURN_INT(::send(fd, buf, nBytes, flags));
ALLOW_C_FUNCTION(send, RESTARTABLE_RETURN_INT(::send(fd, buf, nBytes, flags));)
}

int os::raw_send(int fd, char* buf, size_t nBytes, uint flags) {
return os::send(fd, buf, nBytes, flags);
}

int os::connect(int fd, struct sockaddr* him, socklen_t len) {
RESTARTABLE_RETURN_INT(::connect(fd, him, len));
int os::connect(int fd, const struct sockaddr* him, socklen_t len) {
ALLOW_C_FUNCTION(connect, RESTARTABLE_RETURN_INT(::connect(fd, him, len));)
}

struct hostent* os::get_host_by_name(char* name) {
@@ -862,7 +870,7 @@ char* os::Posix::realpath(const char* filename, char* outbuf, size_t outbuflen)
}

int os::stat(const char *path, struct stat *sbuf) {
return ::stat(path, sbuf);
ALLOW_C_FUNCTION(stat, return ::stat(path, sbuf);)
}

char * os::native_path(char *path) {
@@ -335,7 +335,7 @@ static DIR *open_directory_secure(const char* dirname) {
}

// Open the directory.
dirp = ::opendir(dirname);
dirp = os::opendir(dirname);
if (dirp == NULL) {
// The directory doesn't exist, close fd and return.
::close(fd);
@@ -868,7 +868,7 @@ static int create_sharedmem_resources(const char* dirname, const char* filename,
ssize_t result;

// truncate the file to get rid of any existing data
RESTARTABLE(::ftruncate(fd, (off_t)0), result);
RESTARTABLE(os::ftruncate(fd, (off_t)0), result);
if (result == OS_ERR) {
if (PrintMiscellaneous && Verbose) {
warning("could not truncate shared memory file: %s\n", os::strerror(errno));
@@ -877,7 +877,7 @@ static int create_sharedmem_resources(const char* dirname, const char* filename,
return -1;
}
// set the file size
RESTARTABLE(::ftruncate(fd, (off_t)size), result);
RESTARTABLE(os::ftruncate(fd, (off_t)size), result);
if (result == OS_ERR) {
if (PrintMiscellaneous && Verbose) {
warning("could not set shared memory file size: %s\n", os::strerror(errno));
@@ -5721,7 +5721,7 @@ int os::socket_close(int fd) {
return ::closesocket(fd);
}

int os::connect(int fd, struct sockaddr* him, socklen_t len) {
int os::connect(int fd, const struct sockaddr* him, socklen_t len) {
return ::connect(fd, him, len);
}

@@ -1268,7 +1268,8 @@ FILE* os::fopen(const char* path, const char* mode) {
char modified_mode[20];
assert(strlen(mode) + 1 < sizeof(modified_mode), "mode chars plus one extra must fit in buffer");
sprintf(modified_mode, "%s" LINUX_ONLY("e") BSD_ONLY("e") WINDOWS_ONLY("N"), mode);
FILE* file = ::fopen(path, modified_mode);

ALLOW_C_FUNCTION(fopen, FILE* file = ::fopen(path, modified_mode);)

#if !(defined LINUX || defined BSD || defined _WINDOWS)
// assume fcntl FD_CLOEXEC support as a backup solution when 'e' or 'N'
@@ -787,7 +787,7 @@ class os: AllStatic {
static int recv(int fd, char* buf, size_t nBytes, uint flags);
static int send(int fd, char* buf, size_t nBytes, uint flags);
static int raw_send(int fd, char* buf, size_t nBytes, uint flags);
static int connect(int fd, struct sockaddr* him, socklen_t len);
static int connect(int fd, const struct sockaddr* him, socklen_t len);
static struct hostent* get_host_by_name(char* name);

// Support for signals (see JVM_RaiseSignal, JVM_RegisterSignal)
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -70,4 +70,12 @@
#define PRAGMA_NONNULL_IGNORED
#endif

#ifndef FORBID_C_FUNCTION
#define FORBID_C_FUNCTION(signature, alternative)
#endif

#ifndef ALLOW_C_FUNCTION
#define ALLOW_C_FUNCTION(name, invocation) invocation
#endif

#endif // SHARE_UTILITIES_COMPILERWARNINGS_HPP
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -63,4 +63,44 @@

#endif // clang/gcc version check

#if __GNUC__ >= 9

#include <sys/socket.h>
#include <stdio.h>

#define FORBID_C_FUNCTION(signature, alternative) \
extern "C" signature __attribute__((__warning__(alternative)))

#define ALLOW_C_FUNCTION(name, invocation) \
PRAGMA_DIAG_PUSH \
PRAGMA_DISABLE_GCC_WARNING("-Wattribute-warning") \
invocation \
PRAGMA_DIAG_POP


FORBID_C_FUNCTION(int connect(int, const struct sockaddr*, socklen_t), "use os::connect");
FORBID_C_FUNCTION(FILE* fdopen(int, const char*), "use os::fdopen");
FORBID_C_FUNCTION(void flockfile(FILE*), "use os::flockfile");
FORBID_C_FUNCTION(FILE* fopen(const char*, const char*), "use os::fopen");
FORBID_C_FUNCTION(int fsync(int), "use os::fsync");
FORBID_C_FUNCTION(int ftruncate(int, off_t), "use os::ftruncate");
#ifndef BSD
FORBID_C_FUNCTION(int ftruncate64(int, off64_t), "use os::ftruncate");
#endif
FORBID_C_FUNCTION(void funlockfile(FILE *), "use os::funlockfile");
FORBID_C_FUNCTION(off_t lseek(int, off_t, int), "use os::lseek");
Copy link
Member

@dholmes-ora dholmes-ora Jan 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly there should be a lseek64 definition too.

Copy link
Member Author

@hseigel hseigel Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like ftruncate(), platform agnostic code would call lseek(), not lseek64(). So I think this is correct as is.

Copy link
Member

@dholmes-ora dholmes-ora Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree - you are not enabling the warnings for all the functions that would be used.

#ifndef BSD
FORBID_C_FUNCTION(off64_t lseek64(int, off64_t, int), "use os::lseek");
#endif
FORBID_C_FUNCTION(long int random(void), "use os::random");
FORBID_C_FUNCTION(ssize_t recv(int, void*, size_t, int), "use os::recv");
FORBID_C_FUNCTION(int stat(const char*, struct stat*), "use os::stat");
FORBID_C_FUNCTION(ssize_t send(int, const void*, size_t, int), "use os::send");
FORBID_C_FUNCTION(char* strerror(int), "use os::strerror");
FORBID_C_FUNCTION(ssize_t write(int, const void*, size_t ), "use os::write");

FORBID_C_FUNCTION(char* strtok(char*, const char*), "use strtok_r");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these functions are portable and ought to be forbidden in a platform agnostic location, so the restriction also applies if/when we have real support on other platforms. I think almost none are gcc (or clang) specific, but are instead probably posix and not windows, so maybe should go in a different place as well. Basically I think the structure / placement considerations need some more work.

Copy link
Member

@dholmes-ora dholmes-ora Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put the list of forbidden functions in os.hpp?


#endif // __GNUC__ >= 9

#endif // SHARE_UTILITIES_COMPILERWARNINGS_GCC_HPP
@@ -614,7 +614,7 @@ void fileStream::flush() {
void fdStream::write(const char* s, size_t len) {
if (_fd != -1) {
// Make an unused local variable to avoid warning from gcc compiler.
ssize_t count = ::write(_fd, s, (int)len);
ssize_t count = os::write(_fd, s, (int)len);
update_position(s, len);
}
}