The wrong value of argv was passed to main() in a lot of cases, causing argv[] to be corrupted when other variables used the same memory it did.
The code previously used hard-coded values like 1024 for the argv space. Now the space for argv is computed and allocated dynamically.
The code previously used MemTop and StackTop and played around with their values in unsafe ways. Now the code computes the space it needs for argv, and moves the StackTop down that many bytes, like pushing argv[] and all its pointed-to strings on the stack.
It used a SetNextThreadMemAddr() function to have a delayed effect to setting the thread's top memory address, when argv is shared across all threads and the StackTop only needs to be computed once and then shared among threads (each new thread creates a new segment).
Previously the Args and exe parameters were stored in a lot of different classes. Now, they are simply passed to the RevLoader constructor, which passes them to LoadElf(), which passes them to LoadProgramArgs(). There is no need to store them as class variables, nor to have functions to query them.
On RV32, argv[] needs to be an array of contiguous 4-byte pointers. Previously, it was always 8-byte pointers. The LoadProgramArgs() has been templatized with an XLEN template parameter which is determined based on ELF32 or ELF64.
WriteCacheLine()'s data pointer was not marked const even though it is read-only.
RevCore, RevOpts, and RevLoader had multiple copies of splitStr(), which is used to tokenize strings. Since RevOpts.h is commonly known to RevCore and RevLoader, and since tokenizing strings belongs more properly in "Options" handling, I replaced the 3 splitStr()'s with one in RevOpts which is much simpler and less buggy, since it uses strtok_r(). It was also generalized to accept a string of delimiter characters such as whitespace characters, rather than a single character.
The Processor Objects were initialized identically in both sides of an if depending on whether a coprocessor is present. The processor objects can be unconditionally initialized before the coprocessor objects are optionally initialized.
The args parameter can now be passed to Rev as a true array of values, arrays being written in Python's syntax. If the --args parameter does not, after skipping whitespace, start with a [, then it is assumed to be a string of whitespace-separated arguments, and the old Rev behavior applies. But if it starts with a [, then it is interpreted as a true array, and each argument may contain spaces in it.
In RevLoader::LoadElf, eh64 was a pointer which was cast from membuf and then dereferenced. Besides violating "strict aliasing", it made eh64 a dangling pointer when membuf was unmapped, and to tell whether to use RV32 or RV64 argv, I wanted to test it near the end of LoadElf. So I made eh64 a scalar variable, used memcpy() to set it safely, and then it was good until the end of LoadElf regardless if membuf had been unmapped.
I added two test cases which I verified fail on devel currently, but pass with this PR:
This fixes https://github.com/tactcomplabs/rev/issues/285.
argvwas passed tomain()in a lot of cases, causingargv[]to be corrupted when other variables used the same memory it did.1024for theargvspace. Now the space forargvis computed and allocated dynamically.MemTopandStackTopand played around with their values in unsafe ways. Now the code computes the space it needs forargv, and moves theStackTopdown that many bytes, like pushingargv[]and all its pointed-to strings on the stack.SetNextThreadMemAddr()function to have a delayed effect to setting the thread's top memory address, whenargvis shared across all threads and theStackToponly needs to be computed once and then shared among threads (each new thread creates a new segment).Argsandexeparameters were stored in a lot of different classes. Now, they are simply passed to theRevLoaderconstructor, which passes them toLoadElf(), which passes them toLoadProgramArgs(). There is no need to store them as class variables, nor to have functions to query them.argv[]needs to be an array of contiguous 4-byte pointers. Previously, it was always 8-byte pointers. TheLoadProgramArgs()has been templatized with anXLENtemplate parameter which is determined based on ELF32 or ELF64.WriteCacheLine()'s data pointer was not markedconsteven though it is read-only.RevCore,RevOpts, andRevLoaderhad multiple copies ofsplitStr(), which is used to tokenize strings. SinceRevOpts.his commonly known toRevCoreandRevLoader, and since tokenizing strings belongs more properly in "Options" handling, I replaced the 3splitStr()'s with one inRevOptswhich is much simpler and less buggy, since it usesstrtok_r(). It was also generalized to accept a string of delimiter characters such as whitespace characters, rather than a single character.ifdepending on whether a coprocessor is present. The processor objects can be unconditionally initialized before the coprocessor objects are optionally initialized.argsparameter can now be passed to Rev as a true array of values, arrays being written in Python's syntax. If the--argsparameter does not, after skipping whitespace, start with a[, then it is assumed to be a string of whitespace-separated arguments, and the old Rev behavior applies. But if it starts with a[, then it is interpreted as a true array, and each argument may contain spaces in it.RevLoader::LoadElf,eh64was a pointer which was cast frommembufand then dereferenced. Besides violating "strict aliasing", it madeeh64a dangling pointer whenmembufwas unmapped, and to tell whether to use RV32 or RV64argv, I wanted to test it near the end ofLoadElf. So I madeeh64a scalar variable, usedmemcpy()to set it safely, and then it was good until the end ofLoadElfregardless ifmembufhad been unmapped.develcurrently, but pass with this PR:argv's layout in memory on RV32.