search

uxmal / reko

Reko is a binary decompiler.
https://uxmal.github.io/reko
GNU General Public License v2.0
2.1k stars 251 forks source link

Do not define signature of ProcedureConstant, signature of procedure can be changed later #1253

Closed ptomin closed 1 year ago

uxmal commented 1 year ago

Do we understand why not having the signature set makes such a difference? Is the Unifier becoming confused? I'd like to see a unit test that shows why the change is needed given that we are introducing nulls and that there are some regressions (and improvements) in the subjects.

ptomin commented 1 year ago

Do we understand why not having the signature set makes such a difference?

Yes, we do. See Signtature property of ProcedureConstant: https://github.com/uxmal/reko/blob/bfcc1e9225e1ae9fdcd76d25aa11a782a512b351/src/Core/Expressions/ProcedureConstant.cs#L45 this.sig ?? Procedure.Signature code means that if signature is undefined we get it from procedure. But if we define it at constructor than it ignores following changes during DFA which is incorrect.

uxmal commented 1 year ago

Makes sense. Thanks for fixing this.