bcftools annotate -x FILTER bug

[freeseek]

The following code reproduces the issue:

$ echo -e "##fileformat=VCFv4.1\n##contig=<ID=1>\n#CHROM\tPOS\tID\tREF\tALT\tQUAL\tFILTER\tINFO\n1\t58814\t.\tG\tA\t.\t.\t." | \
  valgrind bcftools annotate -x FILTER
==9804== Memcheck, a memory error detector
==9804== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9804== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==9804== Command: ./bcftools annotate -x FILTER
==9804== 
==9804== Conditional jump or move depends on uninitialised value(s)
==9804==    at 0x15A992: remove_filter (vcfannotate.c:170)
==9804==    by 0x1668B3: annotate (vcfannotate.c:2382)
==9804==    by 0x1668B3: main_vcfannotate (vcfannotate.c:2658)
==9804==    by 0x4A59B6A: (below main) (libc-start.c:308)
==9804== 
##fileformat=VCFv4.1
##contig=<ID=1>
##bcftools_annotateVersion=1.9-80-gff3137d-dirty+htslib-1.9-66-gbcf9bff
##bcftools_annotateCommand=annotate -x FILTER; Date=Wed Apr 24 12:24:54 2019
#CHROM	POS	ID	REF	ALT	QUAL	FILTER	INFO
1	58814	.	G	A	.	.	.
==9804== 
==9804== HEAP SUMMARY:
==9804==     in use at exit: 0 bytes in 0 blocks
==9804==   total heap usage: 184 allocs, 184 frees, 20,504 bytes allocated
==9804== 
==9804== All heap blocks were freed -- no leaks are possible
==9804== 
==9804== For counts of detected and suppressed errors, rerun with: -v
==9804== Use --track-origins=yes to see where uninitialised values come from
==9804== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Basically what is going on is that, if you want to remove the whole FILTER column using bcftools annotate, the following code is used (from vcfannotate.c):

        else if ( !strcasecmp("FILTER",str.s) )
        {
            tag->handler = remove_filter;
            if ( !args->keep_sites ) remove_hdr_lines(args->hdr_out,BCF_HL_FLT);
        }

And when the function remove_filter is called:

void remove_filter(args_t *args, bcf1_t *line, rm_tag_t *tag)
{
    if ( tag->hdr_id<0 )
    {
        assert(args->force);
        tag->hdr_id = bcf_hdr_id2int(args->hdr, BCF_DT_ID, tag->key);
        if ( !bcf_hdr_idinfo_exists(args->hdr,BCF_HL_FLT,tag->hdr_id) ) error("Error: parsing error\n");    // this should not happen
    }
    if ( !tag->key ) bcf_update_filter(args->hdr, line, NULL, args->flt_keep_pass);
    else bcf_remove_filter(args->hdr, line, tag->hdr_id, args->flt_keep_pass);
}

The tag->hdr_id value is tested without ever being set. I have looked at the code and I cannot figure out the logic behind, but it seems like it should be an easy fix.

[freeseek]

I think changing remove_filter to something like this works:

void remove_filter(args_t *args, bcf1_t *line, rm_tag_t *tag)
{
    if ( !tag->key ) bcf_update_filter(args->hdr, line, NULL, args->flt_keep_pass);
    else
    {
        if ( tag->hdr_id<0 )
        {
            assert(args->force);
            tag->hdr_id = bcf_hdr_id2int(args->hdr, BCF_DT_ID, tag->key);
            if ( !bcf_hdr_idinfo_exists(args->hdr,BCF_HL_FLT,tag->hdr_id) ) error("Error: parsing error\n");    // this should not happen
        }
        bcf_remove_filter(args->hdr, line, tag->hdr_id, args->flt_keep_pass);
    }
}

[pd3]

This is now fixed, thank you for looking into it!