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

Get error messages from c_emulator? #25

Open
scottj97 opened this issue Nov 12, 2019 · 15 comments
Open

Get error messages from c_emulator? #25

scottj97 opened this issue Nov 12, 2019 · 15 comments
Labels
enhancement New feature or request

Comments

@scottj97
Copy link
Contributor

I'm running test cases randomly generated by Google's riscv-dv on the sail-riscv c_emulator model.

It runs a few thousand instructions and then dies:

[2399] [M]: 0x00000000800016BC (0x1D73222F) sc.w.aq tp, t1, s7
Sail exception!Exiting due to uncaught exception

How can I tell what this "uncaught exception" is referring to? I can't find anything in the c_emulator/* that would tell me anything more than have_exception.

When I run the same code on the ocaml emulator, I see:

[2399] [M]: 0x00000000800016BC (0x1D73222F) sc.w.aq tp, t1, s7
reservation: 0x000000008001E000, key=0x000000008001E000

Error: Not implemented: sc.aq

Which is a nice clear error message.

How can I find that same message when running the C model?

I see this struct zexception in the generated C code, but I'm not sure how I can access that struct from within riscv_sim.c. That struct is not defined in any header file, and since riscv_sim.c is compiled separately, it can't see it.

@Alasdair
Copy link
Collaborator

Currently the exception 'handling' as it were is set up as below in the C code generator. When have_exception is true the current_exception variable points to a zexception struct. Right now the C generation was pretty much just set up to generate a single monolithic emulator, which is why it doesn't output any separate headers. Recently I've been needing to interact with the C myself for some other reasons though, so I've been thinking about changing the code generator to output something more usable as a library. I'd be interested to hear what you'd like from that, if anything?

    let exn_boilerplate =
      if not (Bindings.mem (mk_id "exception") ctx.variants) then ([], []) else
        ([ "  current_exception = sail_malloc(sizeof(struct zexception));";
           "  CREATE(zexception)(current_exception);" ],
         [ "  KILL(zexception)(current_exception);";
           "  sail_free(current_exception);";
           "  if (have_exception) {fprintf(stderr, \"Exiting due to uncaught exception\\n\"); exit(EXIT_FAILURE);}" ])
    in

@scottj97
Copy link
Contributor Author

Today I'm building a shared library riscv_sim_RV64.so which I link in to Python.

It would probably be useful to have the state encapsulated in a struct (or C++ class) instead of globals like zx1, zx2, etc., so I can instantiate multiple CPUs to model multiple harts. (Is that reasonable? I'm not sure.)

@Alasdair
Copy link
Collaborator

Yes, that's one of the things I've been thinking of doing (at least the struct, I'm not much of a C++ person!), as well as more fine-grained control over the name mangling (all the z prefixes) to allow turning it off when it's not strictly required (for monomorphised generics)
Also it's more of a Robert or Prashanth question, but as far as I can tell the "sc.aq" error is thrown for a write with acquire semantics, which seems like it should be an illegal instruction?

@scottj97
Copy link
Contributor Author

Yes, the sc.aq is not a problem; the only problem is that I don't have any way to report the error properly except a generic "something went wrong in Sail" error message. Which means any time someone on my team hits such a problem, they punt it to me and I have to go build the ocaml model and try to recreate there. Instead of just saying "hey stupid, don't do sc.aq instructions."

@Alasdair
Copy link
Collaborator

Alasdair commented Nov 13, 2019

Yep, I can see the problem. I'll make sure to make the have_exception/current_exception variables be exposed. I can also make it generate a backtrace very easily.

@Alasdair
Copy link
Collaborator

Alasdair commented Dec 5, 2019

I just wanted to leave a update to say I haven't forgotten about this. I've just been busy with some other things the past few weeks, but I'll try to get back to it soon.

@scottj97
Copy link
Contributor Author

scottj97 commented Dec 5, 2019

No big deal. I'm much more concerned about #21 and #27.

@Alasdair
Copy link
Collaborator

(Very delayed update) I have most of this working now via these commits:

rems-project/sail@199e10d
rems-project/sail@81516c8

I re-factored the code-generator so all the model state is contained within a sail_state struct, which includes a state.throw_location variable which contains the source location for any exception thrown. There's also fine-grained control over the name-mangling for generated symbols. It also generate a separate X.h and X.c files, as well as an X_emu.c file which wraps that into a usable executable with some sensible defaults.

I still need to do some work to test and document this and integrate it into sail-riscv, as it's not backwards compatible.

@allenjbaum
Copy link
Collaborator

So, its been 16 months - what is the state of this one?

@jrtc27
Copy link
Collaborator

jrtc27 commented Oct 1, 2021

It's better than it used to be, in that the "Exiting due to uncaught exception" message from Sail itself now prints out the file name and source range (start line+col and end line+col). However the exception is not caught so the "Not implemented: sc.aq" or equivalent message doesn't get printed, you have to go hunt down the source line to see what the assertion is (though in a way that's sometimes more helpful, if the same message appears multiple times). So it's not as good as it could be but it's good enough, there's no longer the problem of having no clue which throw was responsible for the exception.

@allenjbaum
Copy link
Collaborator

allenjbaum commented Oct 1, 2021 via email

@jrtc27
Copy link
Collaborator

jrtc27 commented Oct 1, 2021

I would say the issue should remain open as it's not fully fixed, but the fact that line numbers are now printed means it's much lower priority than it used to be

@allenjbaum
Copy link
Collaborator

allenjbaum commented Oct 1, 2021 via email

@Timmmm Timmmm added the enhancement New feature or request label May 7, 2024
@KotorinMinami
Copy link
Contributor

Is there any further progress on this matter recently? I would like to try to advance the improvement in this area, so I hope to get some latest relevant information.

@Timmmm
Copy link
Collaborator

Timmmm commented Jun 14, 2024

Today I'm building a shared library riscv_sim_RV64.so which I link in to Python.

It would probably be useful to have the state encapsulated in a struct (or C++ class) instead of globals like zx1, zx2, etc., so I can instantiate multiple CPUs to model multiple harts. (Is that reasonable? I'm not sure.)

Slightly off topic but we are doing exactly this - wrapping the entire C code in a C++ struct. It works perfectly with some minor hacks that I'll hopefully turn into non-hacks at some point. See rems-project/sail#547

The only minor caveat is it isn't thread safe because sail.c contains some global temporary variables, but that's fixable.

It would probably be good if this project would build the emulator as a library. I suggested using our code a while ago but people weren't keen because it is C++.

On topic though, we just do this ... so no progress on this specific issue:

        if (m_ffi.have_exception) {
            throw std::runtime_error("Sail exception");
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants