Re: [PATCH] remove attribute access from regexec

[Paul Eggert <eggert@cs.ucla.edu>]

On 8/13/21 2:30 PM, Martin Sebor wrote:
> Attached is a revised patch with this approach.

The revised patch is to include/regex.h but the original patch was to 
posix/regex.h. Is that intentional?

We need to check whether __STDC_VERSION__ is defined. Also, no need for 
parens around arg of 'defined'. Something like this perhaps:

   #if (defined __STDC_VERSION__ && 199901L <= __STDC_VERSION__ \
        && !defined __STDC_NO_VLA__)

Also, the duplication of the declarations make the headers harder to 
read and encourage typos (I noticed one typo: "_Restrict_arr" without 
the trailing "_"). Instead, I suggest something like this:

   #if (defined __STDC_VERSION__ && 199901L <= __STDC_VERSION__ \
        && !defined __STDC_NO_VLA__)
   # define _REGEX_VLA(arg) arg
   #else
   # define _REGEX_VLA(arg)
   #endif

That way, we can simply change "regmatch_t __pmatch[_Restrict_arr_]" to 
"regmatch_t __pmatch[_Restrict_arr_ _REGEX_VLA (__nmatch)]" without 
having to duplicate the entire function declaration.

> PS POSIX says regexec() ignores pnmatch when REG_NOSUB is set, so
> strictly speaking, warning for such calls to it in that case is
> also a false positive.

Ouch, this casts doubt on the entire exercise. It's not simply about 
warnings: it's about the code being generated for the matcher. For 
example, for:

int
f (_Bool flag, unsigned long n, int a[n])
{
   return n == 0 ? 0 : flag ? a[n - 1] : a[0];
}

a compiler is allowed to generate code that loads a[n - 1] even when 
FLAG is false. Similarly, if we add this VLA business to regexec, the 
generated machine code could dereference pmatch unconditionally even if 
our source code makes the dereferencing conditional on REG_NOSUB, and 
the resulting behavior would fail to conform to POSIX.