prysmaticlabs / hashtree

SHA256 library highly optimized for Merkle tree computations
MIT License
27 stars 7 forks source link

Infinite loop in assembler on some binutils versions #2

Closed c-blake closed 1 year ago

c-blake commented 1 year ago

This is more of an "informational issue" as I suspect debugging binutils and/or re-writing Intel's asm macros are both out of scope for this repo.

That said, on Linux x64, on both binutils-2.39 and binutils-2.40, there is an infinite loop of i386_intel_simplify_symbol mutually recursing with i386_intel_simplify in (gas) the GNU assembler. binutils-2.38 seems to work fine, though.

The loop seems triggered by the assembler macro expansion - different macros in different .S source files. There is some code patching the file containing those two functions between 2.38 and 2.39, but I have not investigated further.

potuz commented 1 year ago

Thanks a lot for the report, changing the macro is certainly in-scope of this project. We have reimplemented most of them in goassembly anyway. I'll take a look at this issue eventually (unless you do before and open a PR for it that would be very wellcome) but before this I need to implement the shani version for ARM64 that is implemented in gohashtree but not here.

potuz commented 1 year ago

FYI this was introduced in this commit in this commit in GAS. Will open an issue with them when they allow my new bugzilla account, fixing this on my end would probably require expanding all macros.

c-blake commented 1 year ago

I already got a bug report in earlier today -- https://sourceware.org/bugzilla/show_bug.cgi?id=30292, but I did not identify the commit the way you did which helps some (although I did diff 2.38 and 2.39 and see that delta, I think). You probably still need that bugzilla account to add to that/comment on it/tell them which commit started the trouble. I just told them the version.

potuz commented 1 year ago

https://sourceware.org/bugzilla/show_bug.cgi?id=30294

potuz commented 1 year ago

Ohh crap probably should point to your post lol

c-blake commented 1 year ago

I did try to reply as fast as possible above. Ah well - at least our two bug reports complement each other somewhat even though they were uncoordinated. :-)

I have no idea how long the GNU folk will take to address the bug or if in light of the 2 versions spanning almost a year you still want to re-do the macros as a workaround. (To me it seems a critical gas bug - if you ulimit/limit stacksize to be unlimited then you can even out-of-memory your whole OS, not just crash gas.)

potuz commented 1 year ago

I do not think it is easy to redo those macros in a way that won't crash gas. a solution would be to explicitly pass arguments to the rounds macros as is done in the ARM versions, but I would be worried of introducing bugs in critical software by rewriting this. I'll hope gas fixes this

c-blake commented 1 year ago

We can maybe ping the binutils mailing list to see if anyone has looked into it or plans to. To avoid colliding again, I can let you do it. { Or tell me if you don't want to.. just minimizing round-trips here. :-) }

potuz commented 1 year ago

I'll be grateful if you contact them indeed.