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

Add CLI Support for Executing ELF and C Files #348

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

federicovilla55
Copy link
Contributor

@federicovilla55 federicovilla55 commented Mar 16, 2024

In the Command Line Interface mode, although the possible execution file types are c, asm, bin (as shown by the command-line option --t), it is not possible to simulate C programs.

int Ripes::CLIRunner::processInput(): Assertion `false && "Command-line support for this source type is not yet implemented"' failed.
Aborted (core dumped)

This has been fixed with the possibility to execute RISC-V ELF files too.

In the clirunner.cpp file I've added the logic to load three new source types: C, InternalELF and ExternalELF.
For an ELF file first a check is done to be sure that the specified file is valid for the selected processor and then the file is loaded with the method loadElfFile. I moved this method to the programutilities.cpp file, following what was done for loading binary files with the method loadFlatBinaryFile. The function was moved so that it can be used both by CLI and GUI.

To execute C programs first they are compiled using the already existing methods (such as compileRaw() from ccmanager.cpp), then they are converted as ExternalELF and loaded as mentioned above.

Since I've noticed that running CLI programs can sometimes have problems with large addresses (that cannot fit in an int variable), I've changed the type of the byteAddress variable to long. (Fixing #266)

In the Command Line Interface mode, although the possible execution file types are `c, asm, bin` (as shown by the command-line option `--t`), it is not possible to simulate `C` programs.

```
int Ripes::CLIRunner::processInput(): Assertion `false && "Command-line support for this source type is not yet implemented"' failed.
Aborted (core dumped)
```

This has been fixed with the possibility to execute *RISC-V* `ELF` files too.
In the `clirunner.cpp` file I've added the logic to load three new source types: `C`, `InternalELF` and `ExternalELF`.
For an ELF file first a check is done to be sure that the specified file is valid for the selected processor and then the file is loaded with the method `loadElfFile`. I moved this method to the `programutilities.cpp` file, following what was done for loading binary files with the method `loadFlatBinaryFile`. The function was moved so that it can be used both by CLI and GUI.
To execute `C` programs first they are compiled using the already existing methods (such as `compileRaw()` from `ccmanager.cpp`), then they are converted as `ExternalELF` and loaded as mentioned above.

Since I've noticed that running CLI programs can sometimes have problems with large addresses (that cannot fit in an `int` variable), I've changed the type of the `byteAddress` variable to `long`.
Copy link
Owner

@mortbopet mortbopet left a comment

Choose a reason for hiding this comment

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

Sorry for the review delay; these are sensible changes. Some minor nits, and it should be good to go.

Re. the int fix - thank you for that; i'm sure there must be other places where narrowing issues like this are present (would probably be prudent to build Ripes with some -Werror to find these).

src/cli/clirunner.cpp Outdated Show resolved Hide resolved
src/cli/programutilities.h Show resolved Hide resolved
src/cli/programutilities.cpp Outdated Show resolved Hide resolved
Added detailed comments to the methods in `programutilities.h` and `programutilities.cpp`.
Applied suggested improvements, such as adding `static` to a local function definition.
@mortbopet mortbopet merged commit 8780873 into mortbopet:master Apr 4, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants