Bug: segfault on missing final newline

[leahneukirchen]

--xcat breaks when the input file does not contain a trailing newline.

% echo -n foo >foo
% mlr --xtab cat foo
zsh: segmentation fault  mlr --xtab cat foo

Sometimes, it doesn't segfault but print something like

malloc(18446744072608038873) failed.

Note that this doesn't appear when stdin is used.
--no-mmap works too, so this may be a regression of #29.

Reproducible with Miller 5.6.2 on Void Linux x86_64 glibc, Kernel 5.2.21, glibc 2.30.

[aborruso]

Hi, I think it's related to how POSIX define a line https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_206 It's mandatory to have a carriage return. My 2 cents

…

On Mon, 20 Jan 2020 at 18:06, Leah Neukirchen ***@***.***> wrote: --xcat breaks when the input file does not contain a trailing newline. % echo -n foo >foo % mlr --xtab cat foo zsh: segmentation fault mlr --xtab cat foo Sometimes, it doesn't segfault but print something like malloc(18446744072608038873) failed. Note that this doesn't appear when stdin is used. --no-mmap works too, so this may be a regression of #29 <#29>. Reproducible with Miller 5.6.2 on Void Linux x86_64 glibc, Kernel 5.2.21, glibc 2.30. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#301?email_source=notifications&email_token=AAAHPD7GTH57TCO4QC2PGZLQ6XK3FA5CNFSM4KJHMPV2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IHNIKUA>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAHPD2PKREIEAZGX7KU34DQ6XK3FANCNFSM4KJHMPVQ> .

--

___________________ Andrea Borruso website: https://medium.com/tantotanto 38° 7' 48" N, 13° 21' 9" E, EPSG:4326

___________________ "cercare e saper riconoscere chi e cosa, in mezzo all’inferno, non è inferno, e farlo durare, e dargli spazio" Italo Calvino

[leahneukirchen]

Still doesn't justify a segfault on user-controlled input.

[aborruso]

Hi Leah, I'm not a Miller developer. My goal was only to add something to the thread, not to give you the official reply. I'm sorry

…

On Mon, 20 Jan 2020 at 18:27, Leah Neukirchen ***@***.***> wrote: Still doesn't justify a segfault on user-controlled input. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#301?email_source=notifications&email_token=AAAHPD3LMIPXHSHCEQCPP2DQ6XNIHA5CNFSM4KJHMPV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJNLIEQ#issuecomment-576369682>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAHPD5CXTL67A2U4JXHQ43Q6XNIHANCNFSM4KJHMPVQ> .

--

___________________ Andrea Borruso website: https://medium.com/tantotanto 38° 7' 48" N, 13° 21' 9" E, EPSG:4326

___________________ "cercare e saper riconoscere chi e cosa, in mezzo all’inferno, non è inferno, e farlo durare, e dargli spazio" Italo Calvino

[johnkerl]

@leahneukirchen thanks for reporting. @aborruso segfaults are never acceptable (unless someone does a print statement with %s formatting and non-string data). And the huge-malloc fail is clearly a memory-corruption issue.

[johnkerl]

@leahneukirchen as a workaround please use either mlr --xtab cat < foo (the <) or mlr --no-mmap --xtab cat foo until I get to the bottom of this.

[johnkerl]

P.S. Trailing carriage returns are not mandatory in Miller.

[johnkerl]

@leahneukirchen Having two implementations of all file-format readers (mmap & stdin) has been a development overhead for me, and error-prone for me (as you've encountered), and causes issues with too-many or too-large files (memory-mapped pages aren't freed by the OS, as one would intuit, despite all my attempts on #160):

#29
#102
#160
#181
#256
#296

I am highly tempted to jettison the entire mmap approach and leave only the stdin readers in place. I hesitate only because I explicitly created the mmap stuff for performance improvement -- and it is a bit faster.

But mmap is already opted out when there are too many files, or or files too large (as noted above) so I think it is already unused at those times when its performance improvement would most help.

[johnkerl]

... 2632ddc 4-KLOC red diff. :)

[johnkerl]

$ echo -n foo > foo
$ mlr --xtab cat foo
foo

I'm still accepting --mmap and --no-mmap from the command line, as no-ops, for backward compatibility.

[johnkerl]

This will go out in 5.6.3. Thanks for reporting!! :)

[johnkerl]

P.S. @leahneukirchen having a single stdio code-path for each file-format reader will make #304 easier to implement. So this is a double win. :)