Get error messages from c_emulator?

[scottj97]

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]

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]

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]

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]

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]

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]

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]

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

[Alasdair]

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

https://github.com/rems-project/sail/commit/199e10df8e776e4b27f9cfd2357db6babf674ed1 https://github.com/rems-project/sail/commit/81516c8ad7c30adb3d52741ad6a7c68d4436ec35

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]

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

[jrtc27]

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]

Does that mean this should be closed? (As you can tell from comments on many issues, I'm trying to clean up any outstanding old issues that have actually been fixed or don't need to be).

On Thu, Sep 30, 2021 at 6:33 PM Jessica Clarke @.***> wrote:

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.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/25#issuecomment-931824233, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJTPWPBDMK4IBBYBADTUEUFWDANCNFSM4JMKOH3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[jrtc27]

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]

What are you even doing awake? But thanks, and thanks for the RV32D fixes; I was just about to get someone started on that. He's someone from Pakistan, and he added the RV32E support. Can you think of any Sail projects or fixes I should point him at?

On Thu, Sep 30, 2021 at 7:19 PM Jessica Clarke @.***> wrote:

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

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/25#issuecomment-931842604, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJWBNVVDPHKOOALQNPDUEULDDANCNFSM4JMKOH3A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[KotorinMinami]

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]

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 https://github.com/rems-project/sail/issues/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");
        }