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

Wrong resolution of a generic struct reference causes a crash #21328

Closed
prantlf opened this issue Apr 21, 2024 · 0 comments · Fixed by #21335
Closed

Wrong resolution of a generic struct reference causes a crash #21328

prantlf opened this issue Apr 21, 2024 · 0 comments · Fixed by #21335
Assignees
Labels
Bug This tag is applied to issues which reports bugs. Comptime Features processed during compile time, like $if, $for, $env etc Unit: cgen Bugs/feature requests, that are related to the default C generating backend.

Comments

@prantlf
Copy link

prantlf commented Apr 21, 2024

Describe the bug

Generic function with a struct reference argument gets called with the argument cast to a wrong pointer type.

I reduced the problem to a small script which prints a part of a HAR file. There's a generic function, which accesses fields of various structs recursively:

fn show_struct[T](object &T) {
  $for field in T.fields {
    $if field.is_struct {
      item := object.$(field.name)
      show_struct(&item)            // recursion with various struct types
    } $else {
       ...
    }
  }
}

When the field log of the struct Har is processed, it's cast to a wrong pointer (struct reference type), as demonstrated with testing scripts further below:

show_struct((Har*) &har)              // correct
show_struct((HarContent*) &har->log)  // should be cast to HarLog*

The structs are nested like this: Har { HarLog { HarEntry { HarResponse { HarContent } } } }.

Details

A reduced HAR file:

{
  "log": {
    "entries": [
      {
        "response": {
          "content": {
            "size": 48752,
            "mimeType": "text/html"
          }
        }
      }
    ]
  }
}

Can be deserialised to the following (also reduced) structs:

struct HarContent {
  size      i64
  mime_type string @[json: 'mimeType']
}

struct HarResponse {
  content HarContent
}

struct HarEntry {
  response HarResponse
}

pub struct HarLog {
  entries []HarEntry
}

struct Har {
  log HarLog
}

And processed by the following generic methods:

fn show[T](val T) {
  $if T is $struct {
    show_struct(&val)
  } $else {
    ...
  }
}

fn show_struct[T](object &T) {
  $for field in T.fields {
    $if field.is_struct {
      item := object.$(field.name)
      show_struct(&item)
    } $else {
       ...
    }
  }
}

The compiler will perform:

har := Har{ ... }
show[Har](har)
.. show_struct[Har](&Har(&har))                      // correct
.... show_struct[HarContent](&HarContent(&har.log))  // should be &HarLog(&har.log)

There're more details in the steps to reproduce the problem below.

Reproduction Steps

Download all files from https://gist.github.com/prantlf/efaf649625af24223994de180088e104 to one directory. Run the vsh files to observe the behaviour described below.

Execution of crash-ref.vsh:

❯ ./crash-ref.vsh

struct Har
key: "log"
value Har.log (json: log) of type HarLog
struct HarContent
key: "size"
value HarContent.size (json: size) of type i64
primitive: 4455796480
key: "mimeType"
value HarContent.mime_type (json: mimeType) of type string
signal 11: segmentation fault
0   libsystem_platform.dylib            0x00007ff80393efdd _sigtramp + 29
1   ???                                 0x0000000000000006 0x0 + 6
2   libsystem_c.dylib                   0x00007ff8037bc88a __sfvwrite + 638
3   libsystem_c.dylib                   0x00007ff8037d9598 fwrite + 136
4   crash                               0x000000010972302a _write_buf_to_fd + 138
5   crash                               0x000000010971bd01 print + 33
6   crash                               0x000000010974d7ec main__show_string + 60
7   crash                               0x000000010974e3ca main__show_struct_T_main__HarContent + 2554
8   crash                               0x000000010974cfe8 main__show_struct_T_main__Har + 1288
9   crash                               0x000000010974cadd main__show_T_main__Har + 13
10  crash                               0x000000010974f0a0 main__main + 432
11  crash                               0x00000001097b4657 main + 71
12  dyld                                0x00007ff803584366 start + 1942
string: "

The first problem became visible when show_struct was called with the value of Har.log. Generics resolved Har.log to HarContent instead of to HarLog:

struct Har
key: "log"
value Har.log (json: log) of type HarLog
struct HarContent                         <<< wrong, should be HarLog

Printing the first field survived, interpreting some memory as i64:

struct HarContent
key: "size"
value HarContent.size (json: size) of type i64
primitive: 4455796480                           <<< wrong, should be 48752

Printing the second field crashed, interpreting some memory as string and getting a null pointer:

key: "mimeType"
value HarContent.mime_type (json: mimeType) of type string
signal 11: segmentation fault
...
string: "

The last text printed was string: ". Printing the string value and the trailing " was interrupted by the wrong memory access.

Expected Behavior

When a generic method is called with the type of a struct field, the compiler should resolve the type properly and call the right function:

har := Har{ ... }
show[Har](har)
.. show_struct[Har](&Har(&har))
.... show_struct[HarLog](&HarLog(&har.log))
...... show_struct[HarEntry](&HarEntry(&log.entry))
........ show_struct[HarResponse](&HarResponse(&entry.response))
.......... show_struct[HarContent](&HarContent(&response.content))

Current Behavior

The compiler picks a wrong type. It might be a coincidence; HarContent is the first struct declared in the test script:

har := Har{ ... }
show[Har](har)
.. show_struct[Har](&Har(&har))                      // correct
.... show_struct[HarContent](&HarContent(&har.log))  // should be &HarLog(&har.log)

Possible Solution

I wish I knew :-)

Additional Information/Context

I wasn't able to reduce the code any further while still keeping the bug triggered. Performing any of the three changes below made the script crash-ref.vsh succeed:

Reduce struct nesting level. For example, if I modify crash-ref.vsh to pass HarLog instead of Har, the code will succeed:

- show(har)
+ show(har.log)
❯ ./crash-ref.vsh (modified as above)

struct HarLog
key: "entries"
value HarLog.entries (json: entries) of type HarEntry
array []HarEntry
item 0
struct HarEntry
key: "response"
value HarEntry.response (json: response) of type HarResponse
struct HarContent
key: "size"
value HarContent.size (json: size) of type i64
primitive: 48752
key: "mimeType"
value HarContent.mime_type (json: mimeType) of type string
string: "text/html"

Let us remove the reference to pass the structs by value:

- fn show_struct[T](object &T) {
+ fn show_struct[T](object T) {

...and remove the `&` from the places where the function is called

This version of the script - succeed-value.vsh - succeeds and prints the correct contents of all structs:

❯ ./succeed-value.vsh

struct Har
key: "log"
value Har.log (json: log) of type HarLog
struct HarLog
key: "entries"
value HarLog.entries (json: entries) of type HarEntry
array []HarEntry
item 0
struct HarEntry
key: "response"
value HarEntry.response (json: response) of type HarResponse
struct HarResponse
key: "content"
value HarResponse.content (json: content) of type HarContent
struct HarContent
key: "size"
value HarContent.size (json: size) of type i64
primitive: 48752
key: "mimeType"
value HarContent.mime_type (json: mimeType) of type string
string: "text/html"

Declaring the struct arguments mutable - fail-mut.vsh, fails in the compiler. It shows that the compiler mixes a struct with a pointer to the struct:

❯ ./fail-mut.vsh
==================
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/v_501/fail-mut.01HW07ZQRJX1ZWRFZ5J4DZ5403.tmp.c:20263:101: error: passing 'main__HarEntry *' (aka 'struct main__HarEntry *') to parameter of incompatible type 'main__HarEntry' (aka 'struct main__HarEntry'); dereference with *
                print( str_intp(2, _MOV((StrIntpData[]){{_SLIT("primitive: "), 0xfe10, {.d_s = main__HarEntry_str(*val)}}, {_SLIT0, 0, { .d_c = 0 }}})));
                                                                                                                  ^~~~
/tmp/v_501/fail-mut.01HW07ZQRJX1ZWRFZ5J4DZ5403.tmp.c:3954:49: note: passing argument to parameter 'it' here
static string main__HarEntry_str(main__HarEntry it) { return indent_main__HarEntry_str(it, 0);}
                                                ^
/tmp/v_501/fail-mut.01HW07ZQRJX1ZWRFZ5J4DZ5403.tmp.c:20271:26: error: used type 'main__HarEntry' (aka 'struct main__HarEntry') where arithmetic or pointer type is required
                main__HarEntry item = ((main__HarEntry)__v_array->data) + i;
                                       ^               ~~~~~~~~~~~~~~~
2 warnings and 2 errors generated.

V version

V 0.4.5 dbf48ea

Environment details (OS name and version, etc.)

V full version: V 0.4.5 27cc277.dbf48ea
OS: macos, macOS, 14.4.1, 23E224
Processor: 12 cpus, 64bit, little endian, Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz

getwd: /Users/prantlf/Sources/github/crash
vexe: /usr/local/Cellar/vlang/0.4.5/libexec/v
vexe mtime: 2024-04-21 10:51:55

vroot: OK, value: /usr/local/Cellar/vlang/0.4.5/libexec
VMODULES: OK, value: /Users/prantlf/.vmodules
VTMP: OK, value: /tmp/v_501

Git version: git version 2.44.0
Git vroot status: weekly.2024.13-194-gdbf48eaa
.git/config present: true

CC version: Apple clang version 15.0.0 (clang-1500.3.9.4)
thirdparty/tcc status: thirdparty-macos-amd64 46662e2

Note

You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote.
Other reactions and those to comments will not be taken into account.

@prantlf prantlf added the Bug This tag is applied to issues which reports bugs. label Apr 21, 2024
@felipensp felipensp self-assigned this Apr 23, 2024
@felipensp felipensp added Unit: cgen Bugs/feature requests, that are related to the default C generating backend. Comptime Features processed during compile time, like $if, $for, $env etc labels Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This tag is applied to issues which reports bugs. Comptime Features processed during compile time, like $if, $for, $env etc Unit: cgen Bugs/feature requests, that are related to the default C generating backend.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants