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

goalc: Confusion on return type when a sub-type is returned #772

Closed
Tracked by #774
xTVaser opened this issue Aug 19, 2021 · 1 comment
Closed
Tracked by #774

goalc: Confusion on return type when a sub-type is returned #772

xTVaser opened this issue Aug 19, 2021 · 1 comment

Comments

@xTVaser
Copy link
Member

xTVaser commented Aug 19, 2021

Take this example in target-util:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
; .function (method 16 target)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
  ;stack: total 0xb0, fp? 1 ra? 1 ep? 1
  ;stack_vars: 48 bytes at 16
  ;gprs: gp s5 s4 s3 s2 s1 s0
;; Warnings:
;; INFO: Return type mismatch control-info vs collide-shape.
...
L126:
    lwu v0, 108(gp)           ;; [129] (set! v0-6 (l.wu (+ obj 108))) [gp: target ] -> [v0: control-info ]

When compiled this results in the following:

The method dummy-16 of type target was originally defined as (function type int (inline-array vector) vector collide-shape), but has been redefined as (function target int (inline-array vector) vector control-info)

control-info is a sub-type of collide-shape so this should not be an issue however.

Sometimes this can be casted around, however in this case where it is accessing a field as the final arg, there doesn't seem to be a way to make the cast stick.

@water111
Copy link
Collaborator

yeah this is definitely a bug, I've noticed it too. We either need to make the compiler accept this, or have the decompiler be more strict at inserting casts on the return value.

I don't want to make the compiler accept these. It would work in this case, but this same problem could happen with a defun, and we can't specify the desired return type for a defun, aWe never want to have the function have a different type than what the decompiler assumes. So the decompiler should just add the appropriate cast automatically in these cases.

The return type code is ancient (git says I last touched this 7 months ago!) so it's probably worth another pass to clean it up.

At a first glance, it looks like we use a typecheck to see if we need to cast the return value, when we should really be checking if the types are exactly the same:
https://github.com/water111/jak-project/blob/master/decompiler/analysis/expression_build.cpp#L87

A similar issue is true in the return form: https://github.com/water111/jak-project/blob/master/decompiler/IR2/FormExpressionAnalysis.cpp#L3970

I'll take a closer look tomorrow, but this seems like it will be a quick fix.

@xTVaser xTVaser mentioned this issue Aug 19, 2021
1 task
@xTVaser xTVaser closed this as completed Aug 22, 2021
water111 pushed a commit that referenced this issue Aug 23, 2021
* add IR syntax highlighting

* set the filterFileRegex properly!

* stash

* decompiler: Add print if conditional fails

* decomp: Mostly finish `target-util`

* decomp: figured out a bit more with `target-util`, a bit stuck now

* decomp: *deep breath* `logic-target` mostly complete

* decompiler: More robust arg_count checking for `format` calls

* decomp: some more work in `target-util`

* fix sllv usage

* decomp: `logic-target` is compiling

* decomp: `target-util` very close - blocked by decomp issue!

* decomp: finish `target-util` except for one issue

#772

* demp: update goal_src

* linting

* add back the one remaining method

* address feedback, update source files
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

No branches or pull requests

2 participants