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

segfault with x86 (32 bit) createindex #418

Closed
mr-c opened this issue Feb 25, 2021 · 5 comments
Closed

segfault with x86 (32 bit) createindex #418

mr-c opened this issue Feb 25, 2021 · 5 comments

Comments

@mr-c
Copy link
Contributor

mr-c commented Feb 25, 2021

Expected Behavior

no segfault

Current Behavior

run (gdb) createindex DB tmp tmp
Starting program: /usr/bin/mmseqs-avx2 createindex DB tmp
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
createindex DB tmp 


Program received signal SIGILL, Illegal instruction.
Parameters::printParameters (this=0x56b9c090, module="createindex", argc=2, pargv=0xffffcddc, par=std::vector of length 38, capacity 64 = {...})
    at ./src/commons/Parameters.cpp:2017
2017	            ss << ByteParser::format(*((size_t *)par[i]->value), 'a', 'h');
(gdb) bt
#0  Parameters::printParameters (this=0x56b9c090, module="createindex", argc=2, pargv=0xffffcddc, par=std::vector of length 38, capacity 64 = {...})
    at ./src/commons/Parameters.cpp:2017
#1  0x566a3d88 in Parameters::checkIfDatabaseIsValid (this=0x56b9c090, command=..., argc=2, argv=0xffffcddc, isStartVar=false, isMiddleVar=false, isEndVar=false)
    at ./src/commons/Parameters.cpp:1905
#2  0x566a7f29 in Parameters::parseParameters (this=0x56b9c090, argc=2, pargv=0xffffcddc, command=..., printPar=<optimized out>, parseFlags=0, outputFlags=0)
    at ./src/commons/Parameters.cpp:1850
#3  0x568ab4cc in createindex (argc=2, argv=0xffffcddc, command=...) at ./src/workflow/CreateIndex.cpp:137
#4  0x565bc79d in runCommand (p=0x56ba50c0, argc=2, argv=0xffffcddc) at ./src/commons/Application.cpp:38
#5  0x565ab4cb in main (argc=4, argv=0xffffcdd4) at ./src/commons/Application.cpp:196

same with the sse4.1 variant, etc..

(gdb) run createindex DB tmp
Starting program: /usr/bin/mmseqs-sse4.1 createindex DB tmp
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
createindex DB tmp 


Program received signal SIGILL, Illegal instruction.
Parameters::printParameters (this=0x56bba090, module="createindex", argc=2, pargv=0xffffcddc, par=std::vector of length 38, capacity 64 = {...})
    at ./src/commons/Parameters.cpp:2017
2017	            ss << ByteParser::format(*((size_t *)par[i]->value), 'a', 'h');
(gdb) bt
#0  Parameters::printParameters (this=0x56bba090, module="createindex", argc=2, pargv=0xffffcddc, par=std::vector of length 38, capacity 64 = {...})
    at ./src/commons/Parameters.cpp:2017
#1  0x5669efec in Parameters::checkIfDatabaseIsValid (this=0x56bba090, command=..., argc=2, argv=0xffffcddc, isStartVar=false, isMiddleVar=false, isEndVar=false)
    at ./src/commons/Parameters.cpp:1905
#2  0x566a32a9 in Parameters::parseParameters (this=0x56bba090, argc=2, pargv=0xffffcddc, command=..., printPar=<optimized out>, parseFlags=0, outputFlags=0)
    at ./src/commons/Parameters.cpp:1850
#3  0x5689d5ac in createindex (argc=2, argv=0xffffcddc, command=...) at ./src/workflow/CreateIndex.cpp:137
#4  0x565bbf0d in runCommand (p=0x56bc30c0, argc=2, argv=0xffffcddc) at ./src/commons/Application.cpp:38
#5  0x565aabcb in main (argc=4, argv=0xffffcdd4) at ./src/commons/Application.cpp:196

Steps to Reproduce (for bugs)

Please make sure to execute the reproduction steps with newly recreated and empty tmp folders.

mmseqs-avx2 createdb examples/DB.fasta DB
mmseqs-avx2 createindex DB tmp

MMseqs Output (for bugs)

Create directory tmp
createindex DB tmp 

Illegal instruction (core dumped)

Context

Providing context helps us come up with a solution and improve our documentation for the future.

Your Environment

Include as many relevant details about the environment you experienced the bug in.

  • Git commit used (The string after "MMseqs Version:" when you execute MMseqs without any parameters): 45111
  • Which MMseqs version was used (Statically-compiled, self-compiled, Homebrew, etc.): Compiled for 32-bit x86 on Debian using https://salsa.debian.org/med-team/mmseqs2/-/tree/debian/experimental/debian/patches
  • Server specifications (especially CPU support for AVX2/SSE and amount of system memory): avx2 capable processor, > 60 GiB memory
  • Operating system and version: Debian "testing"
milot-mirdita added a commit that referenced this issue Feb 25, 2021
@milot-mirdita
Copy link
Member

So i fixed this error (d9744e3 + 852f04d).

I ran the regression test and there are a lot of failures. I guess some are related to #253 as nucleotide 13-mers (or even worse by default 15-mers) require more than 4GB RAM. But other failures are way worse:
The sensitivity changes slightly in many tests, meaning 32-bit MMseqs2 produces different than 64-bit MMseqs2. This is pretty bad and would require to go through every module invocation separately to track where differences first happen and then find the root cause.

This would be very time consuming and for an architecture where you can barely search against any realistically sized database.

@mr-c
Copy link
Contributor Author

mr-c commented Feb 26, 2021

So i fixed this error

Thanks!

But other failures are way worse

Okay. Do you want to officially not support 32-bit architectures?

@mr-c
Copy link
Contributor Author

mr-c commented Mar 8, 2021

@milot-mirdita We have not uploaded MMSeqs2 version 13 to Debian yet. The deadline for updates for the forthcoming stable release of Debian is but a few days away.

Should we

  1. Upload MMSeqs2 version 13, but drop 32-bit architectures
    or
  2. Upload MMSeqs2 version 13 + some patches cherry picked
    or
  3. Keep MMSeqs2 version 12 for Debian "Bullseye"

Thanks!

@milot-mirdita
Copy link
Member

Sorry for the delay. I was dealing with some private stuff :/

I would drop 32-bit support. Even with the patch it doesn't pass the integration tests and tracking down why would be very low priority as we require a >4GB address space for everything except small databases anyway.

@mr-c
Copy link
Contributor Author

mr-c commented Mar 9, 2021

@milot-mirdita No worries, I'll send a request to drop the 32-bit packages now

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