search

radareorg / ideas

4 stars 1 forks source link

Esil concept changes #320

Open condret opened 7 years ago

condret commented 7 years ago

to be honest, I'm a bit confused. seems like lots of stuff is wrong in esil atm. I quite often observe 64-bit specific code in the core-engine and wrong use of == operator. Anyone who thinks the same? What can we do about this?

alvarofe commented 7 years ago

I don't know if that was introduced due to radeco's requirements. I've not been following that much ESIL @XVilka should know better

radare commented 7 years ago

Just use git log esil.c and add examples so we can discuss

On 7 May 2017, at 11:35, Álvaro Felipe Melchor notifications@github.com wrote:

I don't know if that was introduced due to radeco's requirements. I've not been following that much ESIL @XVilka should know better

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

XVilka commented 7 years ago

Well. My bad I changed '==' to '-', but the problem is bigger. Currently ESIL is a not quite balanced. We need to produce cleaner design, add missing pieces if needed, remove spurious ones. ESIL need better definition and standartization, then stop to change the language itself.

XVilka commented 7 years ago

It's also quite related to bignums (SSE/AVX/NEON registers support).

alvarofe commented 7 years ago

What about meta issue about ESIL or hackmd to start discussing? just avoiding long conversation over here that goes nowhere.

radare commented 7 years ago

Esil didnt changed in a year. Whats the specific complain?

On 8 May 2017, at 04:58, Anton Kochkov notifications@github.com wrote:

Well. My bad I changed '==' to '-', but the problem is bigger. Currently ESIL is a not quite balanced. We need to produce cleaner design, add missing pieces if needed, remove spurious ones. ESIL need better definition and standartization, then stop to change the language itself.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

radare commented 7 years ago

This will be a extension, not touching the standard

On 8 May 2017, at 04:59, Anton Kochkov notifications@github.com wrote:

It's also quite related to bignums (SSE/AVX/NEON registers support).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

condret commented 7 years ago

https://github.com/radare/radare2/blob/master/libr/anal/esil.c#L1555 things like these are wrong in my opinion. Further more I think we need to remove "<", ">", "<=" and ">=" operations (not sure if all of them exist, but some of them exist for sure). We only need "==". In my opinion the bignum-support is something to think about, maybe even for the core-engine. The we should make every num bignum, to keep it simple. Maybe this is just something that we want to apply in the anal-plugin. It shouldn't be too hard to do, since you never access a bignum, only a vector.

radare commented 7 years ago

there’r libr/util/big.c as a wrapper around gmp and openssl bignum. so if we go for this we should cook that bignum support in esil based on this api. anyway, i think we can discuss all this stuff during the r2con hackaton ;)

On 10 May 2017, at 12:12, condret notifications@github.com wrote:

https://github.com/radare/radare2/blob/master/libr/anal/esil.c#L1555 https://github.com/radare/radare2/blob/master/libr/anal/esil.c#L1555 things like these are wrong in my opinion. Further more I think we need to remove "<", ">", "<=" and ">=" operations (not sure if all of them exist, but some of them exist for sure). We only need "==". In my opinion the bignum-support is something to think about, maybe even for the core-engine. The we should make every num bignum, to keep it simple. Maybe this is just something that we want to apply in the anal-plugin. It shouldn't be too hard to do, since you never access a bignum, only a vector.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/radare/radare2/issues/7437#issuecomment-300438181, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-lgde-uD8wX_l4HloTli1DeK4aEpsks5r4Y10gaJpZM4NS99N.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. Considering a lot has changed since its creation, we kindly ask you to check again if the issue you reported is still relevant in the current version of radare2. If it is, update this issue with a comment, otherwise it will be automatically closed if no further activity occurs. Thank you for your contributions.