On Thu, 5 Apr 2018, Daniel Richard G. wrote:

> I have been building and testing PCRE2 on Windows, using somewhat older
> versions of Visual Studio (required by my employer for customer-system
> compatibility). I have found a few issues and have attached a patch
> (against r929) for a couple of them.
> 
> In patch order:
> 
> * RunTest.bat: This batch script passes an argument to -error in test 2
>   that does not match that in the RunTest shell script, so test 2 was
>   always failing on Windows

Oops. That is clearly an oversight. Patch applied.

> * pcre2_dfa_match.c: Several invocations of pcre2test were crashing due
>   to _chkstk(). Meaning, the program ran out of stack space. I tracked
>   the crash down to this file. To make a long story short, all those big
>   local_offsets[] and local_workspace[] arrays are the source of the
>   problem. Reducing their size from 1000 to 200 elements allows the test
>   to run to completion.

I am not sure what do do about this. I don't want to make the reduction 
in your patch because, sure as eggs are eggs, somebody's pattern match 
will need the higher number. I don't really want to use the heap either, 
as at the moment pcre2_dfa_match() doesn't use the heap at all. Having 
said that, pcre2_dfa_match() is not the main matching function and (I 
assume) much less used that pcre2_match(). More study of this issue 
needed.

> * pcre2grep.c: Needs the same snprintf() workaround as seen elsewhere
>   in the tree
> 
> * Also added some logic so that non-C99 snprintf() works correctly
>   (returns -1 in the case of overflow)

Both patches applied.

> With all that, there are a few remaining issues:
> 
> * The code currently cannot be compiled without a stdint.h header, which
>   is available only in relatively recent versions of Visual Studio.
>   However, this portable and permissively-licensed implementation of the
>   header worked without issue:
> 
>     http://www.azillionmonkeys.com/qed/pstdint.h
> 
>   Just rename it and drop it into the top level of the build tree.

I have copied that paragraph into the NON-AUTOTOOLS-BUILD documentation 
file.

> * Test 6 still crashes due to running out of stack space, only in this
>   case, it's a very deep call stack that is the issue. I had to add
>   /STACK:3000000 to the linker invocation for this issue to go away.

This is DFA matching again. On Linux it runs with a 2MB stack on my box. 
The Linux default is 8MB and I continue to be amazed that in these days 
of gigabyte memories, default stack sizes are so small. Not sure what to 
do about this yet, either. As you might know, pcre2_match() was recently 
refactored so as not to make much use of the stack. Perhaps something 
drastic along the same lines is needed for pcre2_dfa_match(). Added to 
the "think about" list.

> * Test 6 also fails because line 4932 of testinput6 contains a Ctrl-Z
>   character, which on DOS/Windows indicates EOF. 

That does look to be a very definite special binary character, as it's 
the only one in that test. Unfortunately, I cannot remember why or when
that test was added, and I haven't been able to find anything in the
ChangeLog. Would new logic something like this work?

  #ifdef WIN32
  On reading a line with no newline ending with fgets(), which pcre2test
  does now, try reading one more character with ... what? ... fgetc? ... 
  fread? and see if it's Ctrl-Z and if so add it and what follows to the 
  current line.
  #endif
  
OH! I've just noticed this comment in the source of pcre2test:

  /* A number of things vary for Windows builds. Originally, pcretest
  opened its input and output without "b"; then I was told that "b" was
  needed in some environments, so it was added for release 5.0 to both
  the input and output. (It makes no difference on Unix-like systems.)
  Later I was told that it is wrong for the input on Windows. I've now
  abstracted the modes into macros that are set here, to make it easier
  to fiddle with them, and removed "b" from the input mode under
  Windows. The BINARY versions are used when saving/restoring compiled
  patterns. */

Perhaps that "b" was right after all? There is no way I can test any of 
this as I have no access to Windows systems.

> * The following JIT tests failed:

I'm hoping Zoltán can look at those sometime.

Philip

-- 
Philip Hazel
-- 
## List details at https://lists.exim.org/mailman/listinfo/pcre-dev 

Reply via email to