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

vg construct crashes on chromosome X #1488

Closed
glennhickey opened this issue Mar 5, 2018 · 3 comments · Fixed by #1492
Closed

vg construct crashes on chromosome X #1488

glennhickey opened this issue Mar 5, 2018 · 3 comments · Fixed by #1492
Assignees

Comments

@glennhickey
Copy link
Contributor

Stack and command line below. The VCF in question is here:

ftp://ftp.1000genomes.ebi.ac.uk/vol1/ftp/release/20130502/ALL.chrX.phase3_shapeit2_mvncall_integrated_v1b.20130502.genotypes.vcf.gz

Pretty sure this was working up until quite recently...

Stack trace (most recent call last):
#10   Object "/vg/bin/vg", at 0x4ba848, in _start
#9    Object "/vg/bin/vg", at 0x115b3a9, in __libc_start_main
#8    Object "/vg/bin/vg", at 0x115b1b5, in generic_start_main
#7    Source "src/main.cpp", line 61, in main [0x405356]
#6  | Source "src/subcommand/subcommand.cpp", line 72, in operator()
      Source "/usr/include/c++/5/functional", line 2267, in operator() [0x7f3837]
#5    Source "src/subcommand/construct_main.cpp", line 287, in main_construct [0x826e03]
#4    Source "src/constructor.cpp", line 1631, in construct_graph [0x8f4053]
#3    Source "src/constructor.cpp", line 1501, in construct_graph [0x8f348c]
#2    Source "src/constructor.cpp", line 389, in construct_chunk [0x8f17e7]
#1    Object "/vg/bin/vg", at 0xb695a9, in vcflib::Variant::get_sv_len(int)
#0    Object "/vg/bin/vg", at 0x10d4085, in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::swap(std::__
cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)
Segmentation fault (Address not mapped to object [(nil)])
/bin/bash: line 1:     7 Segmentation fault      (core dumped) vg construct --reference hs37d5.fa --vcf ALL.chrX.phase3_shapeit2_mvncall_
integrated_v1b.20130502.genotypes.vcf.gz --region X --region-is-chrom --node-max 32 --alt-paths --threads 1

@edawson
Copy link
Contributor

edawson commented Mar 5, 2018

This is my fault, though I'm not sure why this code is getting hit.

I added some logic to construct to deal with SVs, but it shouldn't do anything unless you pass -S.

I'll work on seeing why this is happening and push a fix.

@glennhickey
Copy link
Contributor Author

OK thanks. The good news is that it is very fast to reproduce. You can even just do something like bcftools view ftp://ftp.1000genomes.ebi.ac.uk/vol1/ftp/release/20130502/ALL.chrX.phase3_shapeit2_mvncall_integrated_v1b.20130502.genotypes.vcf.gz | head -1000 > small.vcf
and work from there to save on downloading time. The crash is pretty much instant in either case.

@adamnovak
Copy link
Member

It looks like vcflib may not be being built with debug info (-g), which means the line number isn't being reported in the stacktrace.

It also looks like construct is unconditionally calling get_sv_len(0) on every variant that is_sv(), with no check for any constructor configuration flag.

I think the real problem is here:

https://github.com/vgteam/vcflib/blob/master/src/Variant.cpp#L210

We're calling .assign() on the 0th entry in a vector that we previously established didn't exist in the info map, and so is default constructed as empty. Assign internally uses swap() I guess, which turns around and segfaults.

We need to fix that line, and preferably harden the whole function (with .at() or explicit vector length checking) against SV descriptions that either lack the right fields or the right number of entries in a field.

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

Successfully merging a pull request may close this issue.

3 participants