From 80022dd5c49241bb4b7b967021218d476f82e09a Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 17 Oct 2019 15:52:07 -0400 Subject: [PATCH] src.ctf.fs: error out when failing to create index MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This patch makes src.ctf.fs fail if it is not able to build an index for a data stream file. This is generally a sign of a corrupted trace. The plan is to be strict by default (abort if we find the trace looks corrupted), but to later introduce a parameter to tell the component to make those checks not fatal. So for now, we can assume that the index will always be present, hence the comment change in fs.h. Without this patch, the trace provided as a test leads to this abort: 10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS/DS build_index_from_idx_file@data-stream-file.c:462 [auto-disc-source-ctf-fs] Invalid LTTng trace index file; indexed size != stream file size: file-size=16699392, total-packets-size=16896000 10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS/DS build_index_from_stream_file@data-stream-file.c:596 [auto-disc-source-ctf-fs] Invalid packet size reported in file: stream="/tmp/single/ch-1_17", packet-offset=12701696, packet-size-bytes=4194304, file-size=16699392 10-17 15:59:00.749 14522 14522 W PLUGIN/SRC.CTF.FS add_ds_file_to_ds_file_group@fs.c:789 [auto-disc-source-ctf-fs] Failed to index CTF stream file '/tmp/single/ch-1_17' (╯°□°)╯︵ ┻━┻ /home/smarchi/src/babeltrace/src/plugins/ctf/fs-src/fs.c:1572: fix_index_lttng_event_after_packet_bug(): Assertion `index` failed. [1] 14522 abort ./src/cli/babeltrace2 /tmp/single The reason is that the index fails to build (the cumulative packet sizes is larger than the data stream file size), but we keep going anyway. The fix_index_lttng_event_after_packet_bug function, expecting an index to always be present, hits that assert. I added the problematic trace to our test trace collection, and a new test script "test_fail" to test how the src.ctf.fs component class handles invalid traces. Signed-off-by: Simon Marchi Change-Id: I5320102352eb8ef5004f0555dec2cc2736297a4f --- src/plugins/ctf/fs-src/fs.c | 5 +- src/plugins/ctf/fs-src/fs.h | 7 +-- tests/Makefile.am | 1 + .../fail/invalid-packet-size/README | 2 + .../fail/invalid-packet-size/trace/channel0_3 | Bin 0 -> 2533 bytes .../trace/index/channel0_3.idx | Bin 0 -> 88 bytes .../fail/invalid-packet-size/trace/metadata | Bin 0 -> 16384 bytes tests/plugins/src.ctf.fs/Makefile.am | 1 + tests/plugins/src.ctf.fs/fail/test_fail | 55 ++++++++++++++++++ 9 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 tests/data/ctf-traces/fail/invalid-packet-size/README create mode 100644 tests/data/ctf-traces/fail/invalid-packet-size/trace/channel0_3 create mode 100644 tests/data/ctf-traces/fail/invalid-packet-size/trace/index/channel0_3.idx create mode 100644 tests/data/ctf-traces/fail/invalid-packet-size/trace/metadata create mode 100755 tests/plugins/src.ctf.fs/fail/test_fail diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c index ee322c0e..09730401 100644 --- a/src/plugins/ctf/fs-src/fs.c +++ b/src/plugins/ctf/fs-src/fs.c @@ -785,8 +785,11 @@ int add_ds_file_to_ds_file_group(struct ctf_fs_trace *ctf_fs_trace, index = ctf_fs_ds_file_build_index(ds_file, ds_file_info); if (!index) { - BT_COMP_LOGW("Failed to index CTF stream file \'%s\'", + BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE( + self_comp, self_comp_class, + "Failed to index CTF stream file \'%s\'", ds_file->file->path->str); + goto error; } if (begin_ns == -1) { diff --git a/src/plugins/ctf/fs-src/fs.h b/src/plugins/ctf/fs-src/fs.h index ba9718d6..d1a1023d 100644 --- a/src/plugins/ctf/fs-src/fs.h +++ b/src/plugins/ctf/fs-src/fs.h @@ -161,12 +161,7 @@ struct ctf_fs_ds_file_group { struct ctf_fs_trace *ctf_fs_trace; /* - * Owned by this. May be NULL. - * - * A stream cannot be assumed to be indexed as the indexing might have - * been skipped. Moreover, the index's fields may not all be available - * depending on the producer (e.g. timestamp_begin/end are not - * mandatory). + * Owned by this. */ struct ctf_fs_ds_index *index; }; diff --git a/tests/Makefile.am b/tests/Makefile.am index 542b06c8..c4b0a0a5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -92,6 +92,7 @@ TESTS_LIB += lib/test_plugin endif TESTS_PLUGINS = \ + plugins/src.ctf.fs/fail/test_fail \ plugins/src.ctf.fs/succeed/test_succeed \ plugins/sink.ctf.fs/succeed/test_succeed \ plugins/sink.text.details/succeed/test_succeed diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/README b/tests/data/ctf-traces/fail/invalid-packet-size/README new file mode 100644 index 00000000..e275e81d --- /dev/null +++ b/tests/data/ctf-traces/fail/invalid-packet-size/README @@ -0,0 +1,2 @@ +The sum of the packet sizes in this trace (both in the index file and in the +actual packet headers) is larger than the data file. diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/trace/channel0_3 b/tests/data/ctf-traces/fail/invalid-packet-size/trace/channel0_3 new file mode 100644 index 0000000000000000000000000000000000000000..7682714ba5ed3e6d3c0dd3c92a5ffef1cb1fb98b GIT binary patch literal 2533 zcmX>o|L0(HM&Q%}YyVXf9|k@VOcxMk00L$Z0R&!IS5K78Vq~z`LiobhWZ)IrM2I1 znNMp!R~~WE1mzSMhWZ)I#pNbii__ZAKY3j>S%DsfVW^+MTw41Lm-)2zvt_)iCL7SB zFbwrGn2XCzv=*nepKoq;)no^H6o#RG26JicH(ch^+Rr;#+%!3W9))43pTS&QZlbj~ a&HWtWrpXEPC=5gW4Cd0>Z@A2-wVwgWZHXBG literal 0 HcmV?d00001 diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/trace/index/channel0_3.idx b/tests/data/ctf-traces/fail/invalid-packet-size/trace/index/channel0_3.idx new file mode 100644 index 0000000000000000000000000000000000000000..c8f306cff3c1aff4dcac48d8f316c3dd91dd5c0f GIT binary patch literal 88 wcmX^3@yn+a literal 0 HcmV?d00001 diff --git a/tests/data/ctf-traces/fail/invalid-packet-size/trace/metadata b/tests/data/ctf-traces/fail/invalid-packet-size/trace/metadata new file mode 100644 index 0000000000000000000000000000000000000000..46d334f1fdab7c127b30c9455acbbe71bbbd20af GIT binary patch literal 16384 zcmeHOOK;mo5KeEEPVJ$`0t=s#AaX=fl4Yqb;5u?*AeI~1Mo|vMfImFh~$;cR923`eydQ*Wdo_{XTj1=d^qD*B}49 z`uFScSKmCqKfeY49z6Ks%cqaY-pM`@HrwR!(`wayKV?+cs6#Z?F0(|>f}iK*~BqPoMnr zr=i(5+ET0C6tAO$}SoNJx;k znvP3Nh56l=-75U8GV>fcO)SSXsez8Y;TY6bCR&Y+Mw+5o4o+LO46328Z+MQ24O}s{ z9KNHwt~suy`$Cn8@$gV{Lfz-C=$3MZ{6jj1Wm>LfY6|iQbt^VSq26q?YuoMNcCA5& zQjKn_thS}LTTRxWswDF9DsvQDo4T6CXL=aYB74XNDUNLR4ulZu;h!#nh~I}+kD1E* zClKc3#Rm-i3bgyPJEywGh&38H%q1h+GDvS~DHEbOov`&Wc_=(Z+K>3F!C2ta+!92o z-H-&K*_6bVC~f(2XW&Y;5&+9$4@FO$f(U$Wn~}OU`7D8Wh&$lFe25$k@%?!o&?3f* z5oB4$;D=XbycltoWel#K#W;v2Kgwjq0vx~K!G*Dv^$u}0gF&PiQ+de7nu(5BJd~Lj zbHPBDa6YChqfL|T!8G!k**n?v416Jpi-ess({*H3a})~D5pv=ouS`82&=|~ttbDla z0}4oN08=h81KOm+(-OcWq_VJYht$dD<|dKqPY7rSWZ}6?5+ILpEIFsPMopJ|r>Wm# zxK;vA_{fFMa<^EKAW-EpiT&8-)v;$AJi>FhGhp4mDHO%#jr_#azA$(+a>rx){tuuzHt3Ua?fo9D@|5WkP8!#vhQ6Zip5gBi{TqzS?yXL(B9NhXh+B6iWVeHNM{PyK>qP8TcFoKQy| zdYY~V^~-uGt8D0vWnDARKoeJTn_xgs)*u3lyBYC8OZJx6jkM!Dcr)1o%@nZieZ;;#f&<<>W%_$yFdiY0$Bz1yce&YyJW+G zitEa?1Z?jmissXkeTbv5Xbzy&8Dkj{lV7re;F^MEMdxI&yVsL@hrPk`NMpDswaDt& zeLan(1xv36e8HeR#ng_|;DtM;`_ymGdTCV|luu;JR3A@Wc3J)-D%|^Y!AP%E~gE+7ANe9}9NY$_D2X0f}p)g9UD$HCF_!EVA>1#4>TE{%0}#@g#0AIFhZ zuzP^rrLpeFScgZ?PG82=hgh&}!M8O^W8IOlPWy?4u~@Wuf&Hba?#NX8r~SQ?gQI?g zf?~lwhEQ%~z|L%R6EnS(yur;TzZiUZ&`*G1!G1>W{gHA*V~5Q5FBjq1PF<6J)!9zsA@niZ~gzGmpK0q*dwyf z^Uu2rkVd}Dy-v8@zFxWDyeM^*e8@jKkWJ>^gbE8-P1wbn6*koA?;JR2R`5Z&X{J`YDSoYaEd4fn;7N6s}NYF7tmil0$0@6A^d27GSK0{LDY%Ya^;5-MhSbApN2 z7AniZLZQT@Kq0G}69%yv167t5Nws2*$qLs{&X{ph;J7vApW)2||Nkgwjo^L^WR|f! zwIIL1jd^x4h3^sYRx<>;0pA7$zr^&{V{jXpzq$+|0k9!KsVT)@Y5@