Skip to content

Commit

Permalink
Fix ecall support for read-only file operations (#325)
Browse files Browse the repository at this point in the history 
* Fix ecall support for read-only file operations

* Fix RISC-V file syscalls to check flags properly

* Add RISC-V ecall test for file operations

Updated `postToGUIThread()` to only run the passed function if the
Qt app is initialized. This was causing problems when using ecall for
file operations in `tst_riscv.cpp` becuse it would try to use the
uninitialized Qt app (a null pointer) to access the GUI thread during
syscall execution.

Added syscall execution to `tst_riscv.cpp`.

* Fix ecall test filename to be relative (fixes problems on Windows)
  • Loading branch information
@raccog
raccog authored Dec 12, 2023
1 parent f6130db commit bb2988e
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 52 deletions.
10 changes: 9 additions & 1 deletion src/processorhandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,16 @@ class ProcessorHandler : public QObject {
return get()->_fullISA();
}

/// Returns a reference to the system call manager.
/// Returns a const reference to the system call manager.
static const SyscallManager &getSyscallManager() {
return get()->_getSyscallManager();
}

/// Returns a non-const reference to the system call manager.
static SyscallManager &getSyscallManagerNonConst() {
return get()->_getSyscallManagerNonConst();
}

/// Sets the program p as the currently instantiated program.
static void loadProgram(const std::shared_ptr<Program> &p) {
get()->_loadProgram(p);
Expand Down Expand Up @@ -294,6 +299,9 @@ private slots:
return m_currentProcessor->fullISA();
}
const SyscallManager &_getSyscallManager() const { return *m_syscallManager; }
SyscallManager &_getSyscallManagerNonConst() const {
return *m_syscallManager;
}
void _loadProcessorToWidget(vsrtl::VSRTLWidget *widget,
bool doPlaceAndRoute = false);
void _selectProcessor(
Expand Down
10 changes: 6 additions & 4 deletions src/statusmanager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ namespace Ripes {

/**
* @brief postToGUIThread
* Schedules the execution of @param fun in the GUI thread.
* Schedules the execution of @param fun in the GUI thread if it exists.
* @param connection type.
*/
template <typename F>
static void postToGUIThread(F &&fun,
Qt::ConnectionType type = Qt::QueuedConnection) {
auto *obj = QAbstractEventDispatcher::instance(qApp->thread());
Q_ASSERT(obj);
QMetaObject::invokeMethod(obj, std::forward<F>(fun), type);
if (qApp) {
auto *obj = QAbstractEventDispatcher::instance(qApp->thread());
Q_ASSERT(obj);
QMetaObject::invokeMethod(obj, std::forward<F>(fun), type);
}
}

class StatusEmitter : public QObject {
Expand Down
11 changes: 7 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 All @@ -29,6 +30,8 @@ class OpenSyscall : public BaseSyscall {
ProcessorHandler::getMemory().readMemConst(address++, 1) & 0xFF);
string.append(byte);
} while (byte != '\0');
if (string.endsWith('\0'))
string.removeLast(); // Remove null-byte

int ret = SystemIO::openFile(QString::fromUtf8(string), arg1);

Expand Down
93 changes: 54 additions & 39 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,31 +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];
const 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_TRUNC ? QIODevice::Truncate : QIODevice::Append) |
(flags & O_EXCL ? QIODevice::NewOnly : QIODevice::NotOpen);
(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 @@ -185,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 @@ -205,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 @@ -216,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 @@ -248,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 @@ -270,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 @@ -288,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 @@ -330,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 @@ -405,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
1 change: 1 addition & 0 deletions test/riscv-tests-64/ecall_file.S
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../riscv-tests/ecall_file.s
Loading

0 comments on commit bb2988e

Please sign in to comment.