Skip to content

Commit

Permalink
Fix RISC-V file syscalls to check flags properly
Browse files Browse the repository at this point in the history
  • Loading branch information
@raccog
raccog committed Dec 8, 2023
1 parent e837e23 commit 4eee2da
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 46 deletions.
9 changes: 5 additions & 4 deletions src/syscall/file.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ class OpenSyscall : public BaseSyscall {

public:
OpenSyscall()
: BaseSyscall("Open", "Opens a file from a path",
{{0, "Pointer to null terminated string for the path"},
{1, "flags"}},
{{0, "the file decriptor or -1 if an error occurred"}}) {}
: BaseSyscall(
"Open", "Opens a file from a path",
{{0, "Pointer to null terminated string for the path"},
{1, "flags" /* TODO(raccog): Add descriptions for each flag */}},
{{0, "the file decriptor or -1 if an error occurred"}}) {}
void execute() {
const AInt arg0 = BaseSyscall::getArg(BaseSyscall::REG_FILE, 0);
const AInt arg1 = BaseSyscall::getArg(BaseSyscall::REG_FILE, 1);
Expand Down
96 changes: 54 additions & 42 deletions src/syscall/systemio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <QTextStream>
#include <QWaitCondition>

#include <set>
#include <stdexcept>
#include <sys/stat.h>

Expand Down Expand Up @@ -76,13 +77,17 @@ class SystemIO : public QObject {
// Maximum number of files that can be open
static constexpr int SYSCALL_MAXFILES = 32;

static constexpr int O_RDONLY = 0x00000000;
static constexpr int O_WRONLY = 0x00000001;
static constexpr int O_RDWR = 0x00000002;
static constexpr int O_APPEND = 0x00000008;
static constexpr int O_CREAT = 0x00000200; // 512
static constexpr int O_TRUNC = 0x00000400; // 1024
static constexpr int O_EXCL = 0x00000800; // 2048
/// Flags used in the open syscall
enum Flags : unsigned {
O_RDONLY = 0x00000000,
O_WRONLY = 0x00000001,
O_RDWR = 0x00000002,
O_ACCMODE = 0x00000003,
O_CREAT = 0x00000100,
O_EXCL = 0x00000200,
O_TRUNC = 0x00001000,
O_APPEND = 0x00002000
};

// //////////////////////////////////////////////////////////////////////////////
// Maintain information on files in use. The index to the arrays is the "file
Expand All @@ -91,8 +96,7 @@ class SystemIO : public QObject {
struct FileIOData {
// The filenames in use. Null if file descriptor i is not in use.
static std::map<int, QString> fileNames;
// The flags of this file, 0=READ, 1=WRITE. Invalid if this file descriptor
// is not in use.
// The flags of this file. Invalid if this file descriptor is not in use.
static std::map<int, unsigned> fileFlags;
// The streams in use, associated with the filenames
static std::map<int, QTextStream> streams;
Expand Down Expand Up @@ -143,34 +147,37 @@ class SystemIO : public QObject {

// Open a file stream assigned to the given file descriptor
static void openFilestream(int fd, const QString &filename) {
files.emplace(fd, filename);

// Ensure flags are valid
const auto flags = fileFlags[fd];
auto qtOpenFlags = // Translate from stdlib file flags to Qt flags
(flags == O_RDONLY ? QIODevice::ReadOnly : QIODevice::NotOpen) |
if ((flags & O_ACCMODE) == O_ACCMODE) {
throw std::runtime_error(
"Tried to open file with incompatible read/write mode flags");
}

// Translate from stdlib file flags to Qt flags
auto qtOpenFlags =
((flags & O_RDONLY) == O_RDONLY ? QIODevice::ReadOnly
: QIODevice::NotOpen) |
(flags & O_WRONLY ? QIODevice::WriteOnly : QIODevice::NotOpen) |
(flags & O_RDWR ? QIODevice::ReadWrite : QIODevice::NotOpen) |
(flags & O_EXCL ? QIODevice::NewOnly : QIODevice::NotOpen);
if (flags & O_WRONLY || flags & O_RDWR) {
qtOpenFlags |=
(flags & O_TRUNC ? QIODevice::Truncate : QIODevice::Append);
}
(flags & O_EXCL ? QIODevice::NewOnly : QIODevice::NotOpen) |
(flags & O_TRUNC ? QIODevice::Truncate : QIODevice::NotOpen) |
(flags & O_APPEND ? QIODevice::Append : QIODevice::NotOpen);

// Try to open file with the given flags
files[fd].open(qtOpenFlags);
files.emplace(fd, filename);
auto &file = files[fd];
file.open(qtOpenFlags);

if (!files[fd].exists() && flags & O_CREAT) {
files.erase(fd);
if (!file.exists() && flags & O_CREAT) {
throw std::runtime_error("Could not create file");
}

if (!files[fd].exists()) {
files.erase(fd);
if (!file.exists()) {
throw std::runtime_error("File not found");
}

if (!files[fd].isOpen()) {
files.erase(fd);
if (!file.isOpen()) {
throw std::runtime_error("File could not be opened");
}

Expand All @@ -188,14 +195,13 @@ class SystemIO : public QObject {
}

// Determine whether a given fd is already in use with the given flag.
static bool fdInUse(int fd, int flag) {
static bool fdInUse(int fd, unsigned flag) {
if (fd < 0 || fd >= SYSCALL_MAXFILES) {
return false;
} else if (fileNames[fd].isEmpty()) {
return false;
} else if ((fileFlags[fd] & flag) ==
static_cast<unsigned>(
flag) /* also compares ie. O_RDONLY (0x0) */) {
} else if ((flag == O_RDONLY) ? (fileFlags[fd] & O_ACCMODE) == flag
: (fileFlags[fd] & flag) == flag) {
return true;
}
return false;
Expand All @@ -208,7 +214,7 @@ class SystemIO : public QObject {
if (fd < STDIO_END || fd >= SYSCALL_MAXFILES)
return;

fileFlags[fd] = -1;
fileFlags[fd] = O_ACCMODE; // set flag to invalid read/write mode
files[fd].close();
streams.erase(fd);
files.erase(fd);
Expand All @@ -219,7 +225,7 @@ class SystemIO : public QObject {
// available file descriptor. Check that filename is not in use, flag is
// reasonable, and there is an available file descriptor. Return: file
// descriptor in 0...(SYSCALL_MAXFILES-1), or -1 if error
static int nowOpening(const QString &filename, int flag) {
static int nowOpening(const QString &filename, unsigned flag) {
int i = 0;
if (filenameInUse(filename)) {
s_fileErrorString = "File name " + filename + " is already open.";
Expand Down Expand Up @@ -251,11 +257,11 @@ class SystemIO : public QObject {
* Open a file for either reading or writing.
*
* @param filename string containing filename
* @param flags 0 for read, 1 for write
* @return file descriptor in the range 0 to SYSCALL_MAXFILES-1, or -1 if
* @param flags see SystemIO::Flags enum for all possible flags
* @return file descriptor in the range 0 to SYSCALL_MAXFILES-1, or if
* error
*/
static int openFile(QString filename, int flags) {
static int openFile(QString filename, unsigned flags) {
SystemIO::get(); // Ensure that SystemIO is constructed
// Internally, a "file descriptor" is an index into a table
// of the filename, flag, and the File???putStream associated with
Expand All @@ -273,8 +279,10 @@ class SystemIO : public QObject {

try {
FileIOData::openFilestream(fdToUse, filename);
} catch (int) {
s_fileErrorString = "File " + filename + " could not be opened.";
} catch (const std::runtime_error &error) {
FileIOData::files.erase(fdToUse);
s_fileErrorString =
"File " + filename + " could not be opened: " + error.what();
retValue = -1;
}

Expand All @@ -291,8 +299,10 @@ class SystemIO : public QObject {
* @return -1 on error
*/
static int seek(int fd, int offset, int base) {
SystemIO::get(); // Ensure that SystemIO is constructed
if (!FileIOData::fdInUse(fd, 0)) // Check the existence of the "read" fd
SystemIO::get(); // Ensure that SystemIO is constructed
if (!(FileIOData::fdInUse(fd, O_RDONLY) ||
FileIOData::fdInUse(fd,
O_RDWR))) // Check the existence of the "read" fd
{
s_fileErrorString =
"File descriptor " + QString::number(fd) + " is not open for reading";
Expand Down Expand Up @@ -333,8 +343,9 @@ class SystemIO : public QObject {
/////////////////////////////////////////////////////
/// Read from STDIN file descriptor while using IDE - get input from
/// Messages pane.
if (!FileIOData::fdInUse(fd,
O_RDONLY)) // Check the existence of the "read" fd
if (!(FileIOData::fdInUse(fd, O_RDONLY) ||
FileIOData::fdInUse(fd,
O_RDWR))) // Check the existence of the "read" fd
{
s_fileErrorString =
"File descriptor " + QString::number(fd) + " is not open for reading";
Expand Down Expand Up @@ -408,8 +419,9 @@ class SystemIO : public QObject {
return myBuffer.size();
}

if (!FileIOData::fdInUse(
fd, O_WRONLY | O_RDWR)) // Check the existence of the "write" fd
if (!(FileIOData::fdInUse(fd, O_WRONLY) ||
FileIOData::fdInUse(fd,
O_RDWR))) // Check the existence of the "write" fd
{
s_fileErrorString =
"File descriptor " + QString::number(fd) + " is not open for writing";
Expand Down

0 comments on commit 4eee2da

Please sign in to comment.