fread segfaults when reading a file with size a multiple of 4k

[st-pasha]

Happens in line 768: *_const_cast(eof) = eol;
The error is:

 *** caught bus error ***
address 0x1012b9000, cause 'non-existent physical address'

4k.txt

[st-pasha]

This requires a much larger change than I originally anticipated, so here's the outline:

• The idea of writing \n to the last byte of the memory-mapped region, although neat, sometimes fails to work, and so will be removed. Instead, we'll use the following strategy: search the file from the end for the last newline, and then move the last line (appending a newline) into a separate memory buffer. This splits the entire file into 2 separate parts: "main" part sof .. eof, and "hidden" part soh .. eoh. For example, if the input was A,B\n1,2\n3,4, then the first part will become A,b\n1,2\n, and the second 3,4\n. Both parts will have valid newline endings.

• The iteration over the file will have to be organized more carefully: while previously it was as simple as

  const char *ch = sof; 
  while ((ch < eof) && <condition>) ...;
  

  Now it becomes more complicated:

  const char *ch = sof, *end = eof;
  while (((ch < end) || (eoh && (end != eoh) && (ch=soh) && (end=eoh))
         && <condition>) ...;
  

  fortunately, this doesn't add any runtime overhead (assuming compiler does proper short-circuit evaluation).

• All parser functions will no longer require any ch < eof tests: the test *ch == eol is sufficient to test the end of the field. The eof variable is even no longer global (it can't be: different threads might be scanning different portions of the file simultaneously). This change is the original reason why we are going to such trouble to ensure there's always an eol at the end file. There is only 1 problem here: quoted string fields with embedded newlines.

  In order to circumvent this difficulty, we change the semantics of the _Bool Field(const char **ptr, void *target) function (currently this function parses the next string field and returns true/false indicator of success/failure; on success it also advances the pointer ptr to the end of the field just read). In the new semantics this function has int return value, 0 meaning success, 1 failure, and 2 "continuation failure". The latter status is returned for fields that are correctly quoted but the parser hit a newline before the field ended (thus, the Field() parser will stop parsing at a newline like all other parsers do). When status 2 is returned, the parser will also fill out the lenOff target structure with detected offset and length until the newline, and it will advance the pointer ptr.

  Additionally, there will be a second field parsing function int continue_parse_string(const char **ptr, lenOff *target), which will (a) assume that it begins inside a quoted field, (b) in case of successful scan increments target->off instead of overwriting it. This function also returns status 0 for success (field scanned till the end), 1 for failure (field is invalid under current quoting rule, for example quote is not followed by a field separator), and 2 for "continuation failure" (i.e. the field didn't end on this line: keep scanning). Both statuses 0 and 2 advance pointer ptr and increment target->off.

• Now the field parser logic will have to become more complicated as well: if previously it was

  while (!fun[type[j]](&ch, target[j])) {
    type[j]++;
    ch = fieldStart;
  }
  

  Now it is more like this:

  while (ret = fun[type[j]](&ch, target[j])) {
    while (ret == 2) {
      ch += eolLen;
      if (ch == end) {
        if (end == eof) { ch = soh; end = eoh; }
        else { ret = 1; break; }
      }
      ret = continue_parse_string(&ch, target[j]);
    }
    if (ret == 1) { 
      type[j]++;
      ch = fieldStart;
    }
    // if `ret` is 0 then don't do anything: field was parsed correctly
  }
  

  This adds small amount of overhead, but only for multi-line string fields, which are sufficiently rare. Overall runtime impact should be negligible for the most common cases.

• There are also lots of minor code changes: added more messages in verbose mode (and some of the existing ones may have been tweaked); added ASSERT(cond) macro that expands into if (!cond) STOP("Internal error on line %d, please report", __LINE__) -- this is intended for checks that should in theory never fail (and thus shouldn't show up red on the coverage report); the pos variable was removed, instead the sof variable is used to track the beginning of data section (it would be too hard to juggle pos / sof / soh triple); comments are added in various places to clarify the meaning of the new / existing code; in some cases compound statements are split into multiple lines to get a more accurate code coverage metric; moved around some functions so that there are 3 distinct sections: all helper functions first, then all parsers, then the main freadMain.

(the PR itself is not ready yet, but this is the plan of attack)