nickg / nvc

VHDL compiler and simulator
https://www.nickg.me.uk/nvc/
GNU General Public License v3.0
636 stars 80 forks source link

VHPIDIRECT string behaviour #976

Open avelure opened 2 months ago

avelure commented 2 months ago

I'm testing out VHPIDIRECT with strings and I'm running into some crashes. As I understand from the GHDL documentation the strings are treated as unconstrained arrays and passed as fat pointers. There is a header in ghdl_cosim to unwrap them https://github.com/ghdl/ghdl-cosim/blob/master/vhpidirect/vffi_user.h Before I tried with the vffi header I assumed it was just passed as null terminated c-strings, which I got working in NVC (though I realized later it is passing them as unterminated strings), but this just gives garbage characters in GHDL. With the fat pointer it works in GHDL, but NVC gives tracebacks.

On a side note I see GHDL supports passing the name of the library to load in the foreign string so that it is not neceseary to pass it on the commandline. NVC supports parsing it in the foreign string, but does not seem to load it if the --loadparameter is not passed.

entity test is
end entity test;
architecture rtl of test is
begin
  p_proc : process
    procedure str_vffi(constant str : in string) is
    begin report "VHPIDIRECT str_vffi" severity failure; end;
    attribute foreign of str_vffi : procedure is "VHPIDIRECT ./test_vhpi_str.so str_vffi";

    procedure str_char_0(constant str : in string) is
    begin report "VHPIDIRECT str_char_0" severity failure; end;
    attribute foreign of str_char_0 : procedure is "VHPIDIRECT ./test_vhpi_str.so str_char_0";

    procedure str_char(constant str : in string; constant len : integer) is
    begin report "VHPIDIRECT str_char" severity failure; end;
    attribute foreign of str_char : procedure is "VHPIDIRECT ./test_vhpi_str.so str_char";
  begin
    str_char("Test string 1" & NUL, 14);
    str_char("Test string 2", 13);
    str_char_0("Test string 3" & NUL);
    str_char_0("Test string 4");
    str_vffi("Test string 5" & NUL);
    str_vffi("Test string 6");
    wait;
  end process;
end architecture rtl;
#include <stdint.h>
#include <assert.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include "test_vhpi_str.h"

// -- from vffi_user.h

char* vffiNullTerminatedString(
  vffiNaturalDimArr_t* ptr
) {
  assert(ptr != NULL);
  assert(ptr->bounds != NULL);
  int len = ptr->bounds[0].len;
  char* str = malloc(sizeof(char) * len + 1);
  strncpy(str, ptr->data, len);
  str[len] = '\0';
  return str;
}

// --

void str_vffi(const vffiNaturalDimArr_t *str) {
    char* str_f = vffiNullTerminatedString(str);
    printf("'%s'\n", str_f);
}

void str_char_0(const char *str) {
    printf("'%s'\n", str);
}

void str_char(const char *str, int len) {
    printf("'%.*s'\n", len, str);
}
#include <stdint.h>
#include <assert.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>

// -- from vffi_user.h
// Range/bounds of a dimension of an unconstrained array with dimensions of type 'natural'
typedef struct {
  int32_t left;
  int32_t right;
  int32_t dir;
  int32_t len;
} range_t;

// Unconstrained array with dimensions of type 'natural'
typedef struct {
  void*    data;
  range_t* bounds;
} vffiNaturalDimArr_t;

// --

void str_vffi(const vffiNaturalDimArr_t *str);

void str_char_0(const char *str);

void str_char(const char *str, int len);
$ gcc -shared -Wall -O5 -o test_vhpi_str.so test_vhpi_str.h test_vhpi_str.c -fPIC -lws2_32
$ nvc --std=08 -a test_vhpi_str.vhd
$ nvc -e test -r --load test_vhpi_str.so
** Note: loading VHPI plugin test_vhpi_str.so
'Test string 1'
'Test string 2'

*** Caught exception c0000005 (EXCEPTION_ACCESS_VIOLATION) [address=00007FF800000002, ip=00007FF8A6FC161C] ***

[00007FF623AB2EA0]
[00007FF623AB3259]
[00007FF8AF6A0B0C] UnhandledExceptionFilter+0x1ec
[00007FF8B1F5987D] RtlCopyMemory+0x2bbd
[00007FF8B1F3F6A7] _C_specific_handler+0x97
[00007FF8B1F551DF] _chkstk+0x12f
[00007FF8B1ECE866] RtlFindCharInUnicodeString+0xa96
[00007FF8B1F541CE] KiUserExceptionDispatcher+0x2e
[00007FF8A6FC161C] ffi_prep_cif+0x5c
[00007FF623C202F4] vhpi_is_printable+0x819c4
[00007FF623C4D77A] _nvc_do_exit+0x9a
[00007FF8920518F0] WORK.TEST.P_PROC.STR_CHAR_0(S)+0xa0
[00007FF892051579] WORK.TEST.P_PROC+0x169
[00007FF623B860E7] std_env_get_assert_format+0x2db7
[00007FF623B77F40] nvc_current_delta+0x5010
[00007FF623AAAFF9]
[00007FF623AAC094]
[00007FF623AA293B]
[00007FF623A91313]
[00007FF623A91366]
[00007FF8B0CA257D] BaseThreadInitThunk+0x1d
[00007FF8B1F0AF28] RtlUserThreadStart+0x28

Please report this bug at https://github.com/nickg/nvc/issues
$ ghdl -a --std=08 test_vhpi_str.vhd
$ ghdl --elab-run -Wl,test_vhpi_str.so --std=08 test
'X³Ô÷'
'˜³Ô÷'
'سÔ÷'
'³Ô÷'
'Test string 5'
'Test string 6'

When working from native commandline I normally have the path pointing to msys64/ucrt64/bin before msys64/clang64/bin. I see this gives another kind of traceback in nvc (as I compile it for clang64), but I don't know if this error is something that is fixable.

** Note: loading VHPI plugin test_vhpi_str.so

*** Caught exception c0000005 (EXCEPTION_ACCESS_VIOLATION) [address=0000000000009330, ip=0000000000009330] ***

[00007FF623AB2EA0]
[00007FF623AB3259]
[00007FF8AF6A0B0C] UnhandledExceptionFilter+0x1ec
[00007FF8B1F5987D] RtlCopyMemory+0x2bbd
[00007FF8B1F3F6A7] _C_specific_handler+0x97
[00007FF8B1F551DF] _chkstk+0x12f
[00007FF8B1ECE866] RtlFindCharInUnicodeString+0xa96
[00007FF8B1F541CE] KiUserExceptionDispatcher+0x2e
[0000000000009330]

Please report this bug at https://github.com/nickg/nvc/issues
nickg commented 1 month ago

This is a big mess at the moment and I'm not sure of the right way to fix it.

  1. GHDL supports "VHPIDIRECT ..." foreign subprograms but using a non-standard calling convention where VHDL types map to standard C types except arrays with non-static bounds which map to a special fat pointer type (the one you showed above).
  2. NVC also supports "VHPIDIRECT ..." foreign subprograms using a non-standard calling convention that is almost the same as GHDL except that arrays with non-static bounds are instead passed as a raw pointer followed by one int64_t length argument for each array dimension (if you want left/right/direction you need to pass that separately). So your example above should be void str_vffi(char *str, int64_t length); and the string is not null-terminated.
  3. The 2008 LRM standard specifies a different calling convention for "VHPIDIRECT ..." and "VHPI ..." subprograms where the C function receives a single const vhpiCbDataT * argument that you then need to extract the parameter values from using the VHPI API. The only difference between "VHPI" and "VHPIDIRECT" is that for the former subprograms need to be registered explicitly with the vhpi_register_foreignf VHPI function and for the latter they are looked up automatically. NVC implements only "VHPI" (see test/regress/vhpi14.vhd and test/vhpi/vhpi14.c for an example). As far as I know only Aldec implements fully compliant "VHPI" and "VHPIDIRECT". Questa might too but I haven't tried.
  4. Questa also has it's own proprietary FLI.

I think the situation I'd like to get to is:

  1. "VHPI ..." and "VHPIDIRECT ..." both implement the LRM VHPI calling convention (i.e. the C function takes a const vhpiCbDataT * argument).
  2. "GHDL lib.so my_func" emulates the GHDL calling convention including fat pointer passing. This would only really make sense if we could get GHDL to agree to also accept "GHDL" as an alias for "VHPIDIRECT" so code could be portable.
  3. "C my_func" would be the NVC-specific calling convention.
avelure commented 1 month ago

I see, this is a bit of a hot mess. I don't know if having 4 standards is the way to go unless it is to keep backwards compatibility.

At present time, what do you think would be the most cross compatible way to pass bulk data to/from VHPIDIRECT procedures?

nickg commented 1 month ago

At present time, what do you think would be the most cross compatible way to pass bulk data to/from VHPIDIRECT procedures?

I don't think there's anything that works across all simulators at the moment. But if you restrict the procedure to take a constrained array with static bounds then I think GHDL and NVC VHPIDIRECT are compatible:

procedure str_vffi_impl(constant str : in string(1 to 100)) is
begin report "VHPIDIRECT str_vffi" severity failure; end;

-- NVC does not support library name in attribute but I can add that...
attribute foreign of str_vffi_impl : procedure is "VHPIDIRECT str_vffi";

procedure str_vffi(constant str : in string) is
    variable padded : string(1 to 100) := (others => NUL);
begin
    padded(1 to str'length) := str;
    str_vffi_impl(padded);
end procedure;
void str_vffi(const char *str) {
    printf("'%s'\n", str);
}
nickg commented 1 month ago

"GHDL lib.so my_func" emulates the GHDL calling convention including fat pointer passing. This would only really make sense if we could get GHDL to agree to also accept "GHDL" as an alias for "VHPIDIRECT" so code could be portable.

I raised this in ghdl/ghdl#2760.