[PATCH] posix: Sync gnulib regex implementation

[Paul Eggert]

Adhemerval Zanella wrote:
> This patch syncs regex code with gnulib (commit 16aa5a2).

Thanks for doing all that work. I merged some of the changes embodied in that 
patch back to Gnulib; see 
<https://lists.gnu.org/r/bug-gnulib/2018-06/msg00095.html>. Here are some 
comments on the changes that I had questions about:

>    * regcomp: add a __libc_lock_init init for dfa lock.

Why is this needed? The lock has already been initialized by the call to 
lock_init about 15 lines earlier. Please see attached patch, which reverts this 
change.

>    * regexec: instead of use intprops.h and its macros, this patch
>      the check_add_overflow_idx function to check for addition
>      overflow.  The function need to be defined on the file because
>      the file will be build multiple times wit 'Idx' type being
>      redefined (I think intprops.h addition should be done in a
>      separate patch).

When used with Gnulib, that implementation won't work with ICC 17.0.4 20170411, 
which defines __GNUC__ to be 5 but does not support __builtin_add_overflow. 
Also, the __GNUC__ < 5 code doesn't look right for multiple reasons: (1) it 
doesn't set *r when returning false, contrary to the comment describing the code 
(presumably the comment needs to be fixed?), (2) it incorrectly uses SSIZE_MAX 
instead of INT_MAX when _REGEX_LARGE_OFFSETS is not defined, and (3) it has an 
unnecessary test for a > 0 (you need to test only whether a < 0).

Although these issues could be fixed individually, why waste time? Let's just 
use intprops.h and avoid the hassle of hacking on the bugs of a partial replacement.

Attached please see an intprops.h addition that is done as a separate patch. The 
second attached patch goes back to using INT_ADD_WRAPV instead of 
check_add_overflow.

>    * regex.h: Remove _Restrict_ and _Restrict_arr_ definition based
>      on __STDC_VERSION__ because its usage leads to failures on
>      posix/check-installed-headers-c{xx}.

That implementation assumes glibc, so won't work in Gnulib. I attempted to work 
around the check-installed-headers issue in the attached patch; if this doesn't 
work please let me know what the issue is.

>    * regex_internal.h: Define lock_fini to empty macro because setting
>      to 0 lead to build issues (error: statement with no effect
>      [-Werror=unused-value]).

Making it empty is problematic, since lock_fini is supposed to be usable 
wherever a function call could appear. Instead, let's define it to ((void) 0). 
That should fix the -Wunused-value issue in a better way. See attached patch.

>    2. posix/PCRE.tests: the test '(a)|\1' uses a backreference along
>       with group creation and I am not sure if it is the correct
>       behavior to accept it with regcomp (REG_EXTENDED).  The GNU grep
>       accepts it with ERE option though.

POSIX allows recomp and grep to accept this pattern as an extension. I dunno 
what it means, though, and no doubt regcomp and grep should report an error 
instead of accepting it silently. That would be a different bugfix, though.

One other thing: let's use https: intead of http: in URLs, as per Gnulib style. 
We should be using these everywhere in Glibc, of course, but one step at a time.

See attached patches for what the above comments boil down to. They are intended 
to be applied after the patch you posted. The patches could all be squashed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-posix-add-intprops.h-to-prepare-for-regex-sync.patch
Type: text/x-patch
Size: 22683 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20180628/fb3fd274/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-posix-finish-merge-with-Gnulib-regex.patch
Type: text/x-patch
Size: 7838 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20180628/fb3fd274/attachment-0001.bin>