GNU bug report logs - #24323
"regcomp.c", line 467: error: syntax error before or at: preg

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24323 in the body.
You can then email your comments to 24323 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dennis Clarke <dclarke <at> blastwave.org>
To: bug-grep <at> gnu.org
Subject: "regcomp.c", line 467: error: syntax error before or at: preg
Date: Sat, 27 Aug 2016 23:40:45 -0400

lib/regcomp.c will not compile with C99 strict compiler because of
the usage of the non-standard "__restrict".

I question the use here of the "restrict" for the purpose of
optimization so I compiled and tested 2.25 both with and without
the "restrict" and there was no measurable difference. The testsuite
passes fully regardless.

We may simply change "__restrict" to the C99 spec clean "restrict" or
remove it entirely with no worries.



--- ./lib/regcomp.c_backup      Sun Apr 10 16:50:01 2016
+++ ./lib/regcomp.c     Sun Aug 28 02:35:43 2016
@@ -464,7 +464,7 @@
    the return codes and their meanings.)  */

 int
-regcomp (regex_t *__restrict preg, const char *__restrict pattern, int 
cflags)
+regcomp (regex_t *restrict preg, const char *restrict pattern, int cflags)
 {
   reg_errcode_t ret;
   reg_syntax_t syntax = ((cflags & REG_EXTENDED) ? 
RE_SYNTAX_POSIX_EXTENDED
@@ -523,7 +523,7 @@
    from either regcomp or regexec.   We don't use PREG here.  */

 size_t
-regerror (int errcode, const regex_t *__restrict preg, char *__restrict 
errbuf,
+regerror (int errcode, const regex_t *restrict preg, char *restrict errbuf,
          size_t errbuf_size)
 {
   const char *msg;




The compiler here was c99 in Oracle Studio 12.5 on Solaris 10 sparc
with strict C99 compliance mode enforced.

Dennis Clarke

Message #8 received at 24323 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Dennis Clarke <dclarke <at> blastwave.org>, 24323 <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 01:45:49 -0700

[Message part 1 (text/plain, inline)]

Dennis Clarke wrote:

> lib/regcomp.c will not compile with C99 strict compiler because of
> the usage of the non-standard "__restrict".

Thanks for the bug report. What are the diagnostics?

> I compiled and tested 2.25 both with and without
> the "restrict" and there was no measurable difference.

POSIX requires a 'restrict' here; see 
<http://pubs.opengroup.org/onlinepubs/9699919799/functions/regcomp.html>. 
(Whether a compiler takes advantage of 'restrict' is not something under our 
control....)

> We may simply change "__restrict" to the C99 spec clean "restrict" or
> remove it entirely with no worries.

The Gnulib code is supposed to be portable to compilers that do not support 
'restrict'. It does this by #defining 'restrict' to something equivalent, on 
compilers that are picky about 'restrict' for one reason or another. Sun C has 
this problem (actually C++), so the code #defines 'restrict' to '__restrict' to 
pacify it.

Sun C supports __restrict, so I'm puzzled as to why you're observing a problem. 
I did not have a problem building grep 2.25 on my Solaris 10 sparc host with 
Oracle Developer Studio 12.5. Please see the attached compressed build log; you 
might want to compare it with your build's output and debug any significant 
discrepancies (you can look in config.log).

One more thing: why use 'c99'? On Solaris it's typically better to use 'cc', as 
'c99' is mainly for portability pedants. If 'grep' builds with 'cc' but not with 
'c99', I'm inclined to declare victory with Oracle Developer Studio.

[build.txt.gz (application/gzip, attachment)]

Message #11 received at 24323 <at> debbugs.gnu.org (full text, mbox):

From: Dennis Clarke <dclarke <at> blastwave.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, 24323 <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 12:34:27 -0400

On 08/28/2016 04:45 AM, Paul Eggert wrote:
> Dennis Clarke wrote:
>
>> lib/regcomp.c will not compile with C99 strict compiler because of
>> the usage of the non-standard "__restrict".
>
> Thanks for the bug report. What are the diagnostics?

I didn't get to see much other than this :


  CC       regex.o
"regcomp.c", line 467: error: syntax error before or at: preg
"regcomp.c", line 467: warning: undefined or missing type for: preg
"regcomp.c", line 467: warning: undefined or missing type for: const
"regcomp.c", line 467: warning: undefined or missing type for: int
"regcomp.c", line 467: warning: parameter mismatch: 3 declared, 0 defined
"regcomp.c", line 470: error: undefined symbol: cflags
"regcomp.c", line 473: error: undefined symbol: preg
"regcomp.c", line 473: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 474: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 475: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 478: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 479: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 490: error: non-unique member requires struct/union 
pointer: newline_anchor
"regcomp.c", line 490: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 493: error: non-unique member requires struct/union 
pointer: newline_anchor
"regcomp.c", line 493: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 494: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 495: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 497: error: undefined symbol: pattern
"regcomp.c", line 497: warning: improper pointer/integer combination: arg #1
"regcomp.c", line 497: warning: improper pointer/integer combination: arg #1
"regcomp.c", line 497: warning: improper pointer/integer combination: arg #2
"regcomp.c", line 508: warning: improper pointer/integer combination: arg #1
"regcomp.c", line 512: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 513: error: left operand of "->" must be pointer to 
struct/union
"regcomp.c", line 526: error: syntax error before or at: preg
"regcomp.c", line 526: warning: undefined or missing type for: preg
"regcomp.c", line 526: warning: undefined or missing type for: char
"regcomp.c", line 527: warning: undefined or missing type for: size_t
"regcomp.c", line 526: warning: parameter mismatch: 4 declared, 0 defined
"regcomp.c", line 545: error: undefined symbol: errbuf_size
"regcomp.c", line 551: error: undefined symbol: errbuf
"regcomp.c", line 551: error: cannot dereference non-pointer type
"regcomp.c", line 553: error: undefined symbol: errbuf
"regcomp.c", line 553: warning: improper pointer/integer combination: arg #1
c99: acomp failed for regex.c
gmake[3]: *** [regex.o] Error 2
gmake[3]: Leaving directory 
`/usr/local/build/grep-2.25_SunOS5.10_sparcv9.001/lib'
gmake[2]: *** [all] Error 2
gmake[2]: Leaving directory 
`/usr/local/build/grep-2.25_SunOS5.10_sparcv9.001/lib'
gmake[1]: *** [all-recursive] Error 1
gmake[1]: Leaving directory 
`/usr/local/build/grep-2.25_SunOS5.10_sparcv9.001'
gmake: *** [all] Error 2


>> I compiled and tested 2.25 both with and without
>> the "restrict" and there was no measurable difference.
>
> POSIX requires a 'restrict' here; see
> <http://pubs.opengroup.org/onlinepubs/9699919799/functions/regcomp.html>. (Whether
> a compiler takes advantage of 'restrict' is not something under our
> control....)

Right .. yes. I went and found that *after* the first rebuild where I
removed "__restrict" entirely and then put it back in as simply the
standard "restrict". Which of course, works fine and everything tests
just fine.

>
>> We may simply change "__restrict" to the C99 spec clean "restrict" or
>> remove it entirely with no worries.
>
> The Gnulib code is supposed to be portable to compilers that do not
> support 'restrict'. It does this by #defining 'restrict' to something
> equivalent, on compilers that are picky about 'restrict' for one reason
> or another. Sun C has this problem (actually C++), so the code #defines
> 'restrict' to '__restrict' to pacify it.

Guess that didn't work. However I didn't bother to go fishing to find if
the '__restrict' was defined somewhere to be 'restrict'. I was looking
for a quick fix and I felt a simple 'restrict' gets the job done nicely.

> Sun C supports __restrict, so I'm puzzled as to why you're observing a
> problem.

dunno :-\

However I went and looked at the source and when I saw the not so spec
 standard double underscore there I simply muttered 'ah ha' and removed
it entirely because it is just a programmers notice of intent. Then
later there was some dust in the attic and I went and looked at
regcomp(3C) which is right there smack dab in the manpages :

Standard C Library Functions                          regcomp(3C)

NAME
     regcomp, regexec, regerror,  regfree  -  regular  expression
     matching

SYNOPSIS
     #include <sys/types.h>
     #include <regex.h>

     int regcomp(regex_t *restrict  preg,  const  char  *restrict
     pattern, int cflags);

So I thought .. oops .. better put that 'restrict' back in. No usage of
the double underscore however. Everything works great.

> I did not have a problem building grep 2.25 on my Solaris 10
> sparc host with Oracle Developer Studio 12.5. Please see the attached
> compressed build log; you might want to compare it with your build's
> output and debug any significant discrepancies (you can look in
> config.log).

Well here comes the next interesting bit. This was one of my older
machines and it has Oracle Studio 12.4 on it. That may be the oddball
here but I doubt it.  I will take a look at your logs however. Thank you
for the comparison data. Normally I never get data to look at.

> One more thing: why use 'c99'? On Solaris it's typically better to use
> 'cc', as 'c99' is mainly for portability pedants. If 'grep' builds with
> 'cc' but not with 'c99', I'm inclined to declare victory with Oracle
> Developer Studio.

I agree and yes I am a bit of a 'portability pedant' to be honest. For
general purpose building of gnuified sources I give c99 a whirl first
and see if the code compiles clean. If so .. be happy right?  If not
then take a look at see if there is something trivial. That was what
happened here. If not trivial then change the CFLAGS and give up on ye
old -Xc strict compliance mode and switch to cc with -Xa and a pile of
other flags for my pedantic debugging mode needs :

mimas$ echo $CFLAGS
-m64 -xtarget=ultra2e -xarch=sparcvis -xchip=ultra2e -xcache=generic 
-errfmt=error -erroff=%none -errshort=full -xstrconst -xildoff -m64 
-xmemalign=8s -xnolibmil -Xc -xcode=pic32 -xregs=no%appl -xlibmieee -mc 
-g -xs -ftrap=%none -Qy -xbuiltin=%none -xdebugformat=dwarf -xunroll=1 
-D_TS_ERRNO -D_POSIX_PTHREAD_SEMANTICS -D_LARGEFILE64_SOURCE

However portability is a real issue for me. Certainly with super
critical tools like 'grep' or 'ls' etc. If the code were tightly
constrained to C99 spec ISO/IEC 9899:TC3 with the use of strict posix
functions then everything would work everywhere. Utopia. Not very likely
to happen in this millenium.

Dennis

Message #14 received at 24323 <at> debbugs.gnu.org (full text, mbox):

From: Dennis Clarke <dclarke <at> blastwave.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, 24323 <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 12:56:50 -0400

> POSIX requires a 'restrict' here; see
> <http://pubs.opengroup.org/onlinepubs/9699919799/functions/regcomp.html>. (Whether
> a compiler takes advantage of 'restrict' is not something under our
> control....)

Maybe the first coffee has not hit yet but why is there a redefined
"int regcomp" function inside lib/regcomp.c at all??  This is provided
via the standard C lib and regex.h and so this really is not needed.

I see in lib/regcomp.c : ( complete with the new page control char )


^L
/* Entry point for POSIX code.  */
/* regcomp takes a regular expression as a string and compiles it.

   PREG is a regex_t *.  We do not expect any fields to be initialized,
   since POSIX says we shouldn't.  Thus, we set

     'buffer' to the compiled pattern;
     'used' to the length of the compiled pattern;
     'syntax' to RE_SYNTAX_POSIX_EXTENDED if the
       REG_EXTENDED bit in CFLAGS is set; otherwise, to
       RE_SYNTAX_POSIX_BASIC;
     'newline_anchor' to REG_NEWLINE being set in CFLAGS;
     'fastmap' to an allocated space for the fastmap;
     'fastmap_accurate' to zero;
     're_nsub' to the number of subexpressions in PATTERN.

   PATTERN is the address of the pattern string.

   CFLAGS is a series of bits which affect compilation.

     If REG_EXTENDED is set, we use POSIX extended syntax; otherwise, we
     use POSIX basic syntax.

     If REG_NEWLINE is set, then . and [^...] don't match newline.
     Also, regexec will try a match beginning after every newline.

     If REG_ICASE is set, then we considers upper- and lowercase
     versions of letters to be equivalent when matching.

     If REG_NOSUB is set, then when PREG is passed to regexec, that
     routine will report only success or failure, and nothing about the
     registers.

   It returns 0 if it succeeds, nonzero if it doesn't.  (See regex.h for
   the return codes and their meanings.)  */

int
regcomp (regex_t *restrict preg, const char *restrict pattern, int cflags)
{
.
.
.


Not needed at all.

I see in lib/regex.c this :

#ifdef _LIBC
/* We have to keep the namespace clean.  */
.
.
.
# define regcomp(preg, pattern, cflags) __regcomp (preg, pattern, cflags)
.
.
.
#include "regcomp.c"


The only places that use "regcomp" are :

    ./lib/exclude.c ./lib/regex.c

So really these can all use the standarc C library function provided and 
not use the thing in regexec.c at all.

Hold on a sec here ... are we re-writing the POSIX standard C library
 functions for some reason ?


Dennis

Message #17 received at 24323 <at> debbugs.gnu.org (full text, mbox):

From: Dennis Clarke <dclarke <at> blastwave.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, 24323 <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 13:13:05 -0400

On 08/28/2016 04:45 AM, Paul Eggert wrote:
> Dennis Clarke wrote:

Coffee number two.

Digging through the Changelog and looking at things like
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16911#24  I see the 
problems run deeper. This rabbit hole just goes on and on.

I am happy with the change of '__restrict' to the standard 'restrict'
which seems to be a no problem but the use of a special new regcomp
function just doesn't seem to make sense when the libC already provides
that.

Dennis

Message #22 received at 24323-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Dennis Clarke <dclarke <at> blastwave.org>, 24323-done <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 12:58:39 -0700

[Message part 1 (text/plain, inline)]

I see the problem: the grep 2.25 tarball contains a regression from Gnulib that 
breaks compilation on some non-GCC compilers. Details below.

Dennis Clarke wrote:

> it has Oracle Studio 12.4 on it.

Ah, that explains it. 12.4 c99 supports __restrict__ but not __restrict, and 
this runs afoul of a porting bug in grep 2.25, based on Gnulib commit 
b7bc3c1a4e78add4cbad39ae1a0c4fb0747b483f (2016-04-09), which contains a 
regression that causes regcomp.c to not build on (non-GCC) compilers that lack 
__restrict. After grep 2.25 came out, this regression was fixed in Gnulib commit 
334d97f35cef30af0cf61c5eeaa0a67890a45578 (2016-06-08), which you can see here:

http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=334d97f35cef30af0cf61c5eeaa0a67890a45578

This Gnulib commit replaces __restrict with _Restrict_ in lib/regcomp.c, which 
fixes the regression. Applying this change to grep 2.25 should fix things for you.

I fixed the bug by installing the attached patch into the grep master branch on 
Savannah. This patch updates the Gnulib version to the current latest version, 
which pulls in the abovementioned bug fix.

> If not trivial then change the CFLAGS and give up on ye
> old -Xc strict compliance mode and switch to cc with -Xa and a pile of
> other flags for my pedantic debugging mode needs :
>
> mimas$ echo $CFLAGS
> -m64 -xtarget=ultra2e -xarch=sparcvis -xchip=ultra2e -xcache=generic -errfmt=error -erroff=%none -errshort=full -xstrconst -xildoff -m64 -xmemalign=8s -xnolibmil -Xc -xcode=pic32 -xregs=no%appl -xlibmieee -mc -g -xs -ftrap=%none -Qy -xbuiltin=%none -xdebugformat=dwarf -xunroll=1 -D_TS_ERRNO -D_POSIX_PTHREAD_SEMANTICS -D_LARGEFILE64_SOURCE

You might want to look here for new compiler options in 12.5:

https://docs.oracle.com/cd/E60778_01/html/E60742/gncki.html

> Hold on a sec here ... are we re-writing the POSIX standard C library
>  functions for some reason ?

Yes, as grep needs a regular expression matcher that works on data that can 
contain NUL bytes ('\0') and encoding errors. The POSIX API does not provide 
this, so grep supplies a substitute regex matcher on systems like Solaris that 
lack the extensions to POSIX that grep needs.

[0001-build-update-gnulib-submodule-to-latest.patch (text/x-diff, attachment)]

Message #25 received at 24323-done <at> debbugs.gnu.org (full text, mbox):

From: Dennis Clarke <dclarke <at> blastwave.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, 24323-done <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 17:10:41 -0400

On 08/28/2016 03:58 PM, Paul Eggert wrote:
> I see the problem ...

> Ah, that explains it. 12.4 c99 supports __restrict__ but not __restrict,

Sort of what I said .. I think.  For that matter are you sure about the
 use of "__restrict__" ?

> This Gnulib commit replaces __restrict with _Restrict_

I saw that in a few places and wondered if there was a mistake. I don't
 think that __Restrict__ exists in any standard anywhere either.

> I fixed the bug by installing the attached patch into the grep master
> branch on Savannah. This patch updates the Gnulib version to the current
> latest version, which pulls in the abovementioned bug fix.

I will have to wait for a release to use that.  For now I just toss out
the __restrict and use proper "restrict" and everything is fine.

> You might want to look here for new compiler options in 12.5:
>
> https://docs.oracle.com/cd/E60778_01/html/E60742/gncki.html

Yep.  A lot of build scripts will have to be looked at. Eventually.

>> Hold on a sec here ... are we re-writing the POSIX standard C library
>>  functions for some reason ?
>
> Yes, as grep needs a regular expression matcher that works on data that
> can contain NUL bytes ('\0') and encoding errors.

Well this explains why I generally install GNU grep and use it as
opposed to /usr/xpg4/bin/grep on Solaris machines. However the pedantic
scripts still use the XPG4 version. Mostly because we know it will
always exist.


> The POSIX API does not
> provide this, so grep supplies a substitute regex matcher on systems
> like Solaris that lack the extensions to POSIX that grep needs.

yep .. Solaris systems can be very spec tight and this is why I often
compile things with c99 just to shake loose these little bugs which
would otherwise fly under radar.

Thank you for the quick bug fix.

Dennis

Message #28 received at 24323-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Dennis Clarke <dclarke <at> blastwave.org>, 24323-done <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 16:58:53 -0700

Dennis Clarke wrote:
> On 08/28/2016 03:58 PM, Paul Eggert wrote:
>> I see the problem ...
>
>> Ah, that explains it. 12.4 c99 supports __restrict__ but not __restrict,
>
> Sort of what I said .. I think.  For that matter are you sure about the
>  use of "__restrict__" ?

Your original bug report said 12.5, which caused me to waste some time 
installing 12.5 and finding no bug.

Yes, I observed the behavior with __restrict__ and __restrict.

>> This Gnulib commit replaces __restrict with _Restrict_
>
> I saw that in a few places and wondered if there was a mistake. I don't
>  think that __Restrict__ exists in any standard anywhere either.

It doesn't. _Restrict_ is a macro defined by Gnulib, which should expand to the 
appropriate keyword (or non-keyword).

> I will have to wait for a release to use that.  For now I just toss out
> the __restrict and use proper "restrict" and everything is fine.

Yes, that should work.

Message #31 received at 24323-done <at> debbugs.gnu.org (full text, mbox):

From: Dennis Clarke <dclarke <at> blastwave.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, 24323-done <at> debbugs.gnu.org
Subject: Re: bug#24323: "regcomp.c", line 467: error: syntax error before or
 at: preg
Date: Sun, 28 Aug 2016 21:38:10 -0400

On 08/28/2016 07:58 PM, Paul Eggert wrote:
> Dennis Clarke wrote:
>> On 08/28/2016 03:58 PM, Paul Eggert wrote:
>>> I see the problem ...
>>
>>> Ah, that explains it. 12.4 c99 supports __restrict__ but not __restrict,
>>
>> Sort of what I said .. I think.  For that matter are you sure about the
>>  use of "__restrict__" ?
>
> Your original bug report said 12.5, which caused me to waste some time
> installing 12.5 and finding no bug.

Sorry. Wrong machine. I have a pile of different revs here and the
system which tossed the error was in actual fact Oracle Studio 12.4
whereas nearly any other Solaris 10 or 11 machine has Studio 12.5 on
them.

>
> Yes, I observed the behavior with __restrict__ and __restrict.
>
>>> This Gnulib commit replaces __restrict with _Restrict_
>>
>> I saw that in a few places and wondered if there was a mistake. I don't
>>  think that __Restrict__ exists in any standard anywhere either.
>
> It doesn't. _Restrict_ is a macro defined by Gnulib, which should expand
> to the appropriate keyword (or non-keyword).
>
>> I will have to wait for a release to use that.  For now I just toss out
>> the __restrict and use proper "restrict" and everything is fine.
>
> Yes, that should work.

I think the testsuite is a valid test and everything passes fine so it
looks fine as just "restrict".

Dennis

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.