Repository navigation
Description
Hi again, and thanks for looking at #280. While testing the same program (fg-sra) we came across a second, unrelated crash, in ReferenceObj_Read() in libs/align/reference.c. A PR with a proposed fix follows; please handle it however suits you best.
Problem
In ReferenceObj_Read(), when TableReader_ReadRow() fails (for example for an external reference that isn't available locally, with remote access disabled), the loop still evaluates cself->mgr->reader_cols[ereflst_cn_SEQ_LEN].base.coord_len[0] (line 1030). If no row has been read through that reference list's reader yet, the cell's pointer is NULL and the read segfaults. If a row was read earlier, it reads that row's stale value, which is harmless because the loop then ends on rc != 0.
So a program that reads references in an order where the first read is of a missing reference crashes instead of getting an error back. A program that happens to read an available reference first doesn't see it.
Reproduction
- Archive: SRR390728 (aligned, 25 external RefSeq references): a copy of the
.sraalone, without its references, with remote access disabled (/repository/remote/disabled = "true"). - A small harness (below) opens the database, makes a
ReferenceListwithereferencelist_4na, and callsReferenceObj_Read(obj, 0, 5000, ...)on one reference before any other. - Against
engineering(ac67f21), debug build, macOS 26.6.1 arm64: references whose first rows aren't in the archive (e.g. index 6, NC_000016.8) segfault:
stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
frame #0: ReferenceObj_Read(cself=..., offset=0, len=5000, ...) at reference.c:1030:26
- With the check moved inside
if (rc == 0), the same calls returnrc=0x9be50398with nothing written, and references whose data are present read as before.
Fix
Evaluate the SEQ_LEN check only after a successful TableReader_ReadRow(), i.e. inside the if (rc == 0) block. The loop exits on a failed read either way, so nothing else changes.
Thanks again for taking a look.
missing-reference-read.c: the harness
/*
* Reproduction: ReferenceObj_Read on a reference whose rows can't be read.
*
* Opens an aligned (cSRA) archive whose external references are not available (e.g. a copy
* of the .sra alone, with remote access disabled), and reads the first bases of reference
* <index> before any other. TableReader_ReadRow fails, but ReferenceObj_Read then reads the
* row's SEQ_LEN cell anyway; no row has been read yet, so the cell's pointer is NULL.
*
* Usage: missing-reference-read <archive.sra> <index>
*/
#include <align/reference.h>
#include <klib/rc.h>
#include <vdb/database.h>
#include <vdb/manager.h>
#include <stdio.h>
#include <stdlib.h>
int main(int argc, char *argv[])
{
const VDBManager *mgr = NULL;
const VDatabase *db = NULL;
const ReferenceList *list = NULL;
const ReferenceObj *obj = NULL;
uint8_t bases[5000];
INSDC_coord_len written = 0;
rc_t rc;
if (argc != 3) {
fprintf(stderr, "usage: %s <archive.sra> <index>\n", argv[0]);
return 2;
}
rc = VDBManagerMakeRead(&mgr, NULL);
if (rc == 0)
rc = VDBManagerOpenDBRead(mgr, &db, NULL, "%s", argv[1]);
if (rc == 0)
rc = ReferenceList_MakeDatabase(&list, db, ereferencelist_4na, 0, NULL, 0);
if (rc == 0)
rc = ReferenceList_Get(list, &obj, (uint32_t)atoi(argv[2]));
if (rc != 0) {
fprintf(stderr, "setup failed: rc=%u\n", rc);
return 2;
}
rc = ReferenceObj_Read(obj, 0, sizeof(bases), bases, &written);
{
const char *seq_id = NULL;
ReferenceObj_SeqId(obj, &seq_id);
printf("%s: ReferenceObj_Read rc=%u written=%u\n", seq_id, rc, written);
}
ReferenceObj_Release(obj);
ReferenceList_Release(list);
VDatabaseRelease(db);
VDBManagerRelease(mgr);
return rc == 0 ? 0 : 1;
}
Activity
fix(vdb-sys): patch three ncbi-vdb bugs before building it
durbrow commentedon Sep 25, 2026
Thank you for your report. This is great analysis on your part. Very helpful. We will be getting to your pull request soon.
fix(vdb-sys): patch three ncbi-vdb bugs before building it
New "fastq" subcommand to generate bgzipped fastqs from aligned and u…