[PATCH] errno can't alias locals (PR 92412)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] errno can't alias locals (PR 92412)

Martin Sebor-2
The conditional in default_ref_may_alias_errno has the function
return true even for local variables, implying that locals must
be assumed not to have been changed across calls to errno-setting
functions like malloc.  This leads to both worse code and also
false negatives in the strlen pass' detection of buffer overflow
across such calls.

The attached patch constrains the conditional to only consider
external declarations.

Tested on x86_64-linux.

Martin

gcc-92412.diff (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] errno can't alias locals (PR 92412)

Richard Biener-2
On Mon, Nov 11, 2019 at 11:38 PM Martin Sebor <[hidden email]> wrote:

>
> The conditional in default_ref_may_alias_errno has the function
> return true even for local variables, implying that locals must
> be assumed not to have been changed across calls to errno-setting
> functions like malloc.  This leads to both worse code and also
> false negatives in the strlen pass' detection of buffer overflow
> across such calls.
>
> The attached patch constrains the conditional to only consider
> external declarations.
>
> Tested on x86_64-linux.

OK.

This means a tentative definition for 'errno' is non-conforming?
(besides not working well in practice, of course)

Thanks,
Richard.

>
> Martin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] errno can't alias locals (PR 92412)

Andreas Schwab
On Nov 12 2019, Richard Biener wrote:

> This means a tentative definition for 'errno' is non-conforming?

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html

 If a macro definition is suppressed in order to access an actual
 object, or a program defines an identifier with the name errno, the
 behavior is undefined.

Andreas.

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] errno can't alias locals (PR 92412)

Richard Biener-2
On Tue, Nov 12, 2019 at 11:13 AM Andreas Schwab <[hidden email]> wrote:

>
> On Nov 12 2019, Richard Biener wrote:
>
> > This means a tentative definition for 'errno' is non-conforming?
>
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
>
>  If a macro definition is suppressed in order to access an actual
>  object, or a program defines an identifier with the name errno, the
>  behavior is undefined.

To quote the relevant parts before this:

The <errno.h> header shall provide a declaration or definition for
errno. The symbol errno shall expand to a modifiable lvalue of type
int. It is unspecified whether errno is a macro or an identifier
declared with external linkage.

The first sentence suggests errno.h can provide a tentative definition
(since that's also a definition?).  The third sentence gives choice only
between a macro and a declaration with external linkage (that's clearly
_not_ a definition).

So ...

Richard.

> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, [hidden email]
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] errno can't alias locals (PR 92412)

Martin Sebor-2
In reply to this post by Richard Biener-2
On 11/12/19 1:04 AM, Richard Biener wrote:

> On Mon, Nov 11, 2019 at 11:38 PM Martin Sebor <[hidden email]> wrote:
>>
>> The conditional in default_ref_may_alias_errno has the function
>> return true even for local variables, implying that locals must
>> be assumed not to have been changed across calls to errno-setting
>> functions like malloc.  This leads to both worse code and also
>> false negatives in the strlen pass' detection of buffer overflow
>> across such calls.
>>
>> The attached patch constrains the conditional to only consider
>> external declarations.
>>
>> Tested on x86_64-linux.
>
> OK.
>
> This means a tentative definition for 'errno' is non-conforming?
> (besides not working well in practice, of course)

Errno is a reserved name (just like all external identifiers)
so only an implementation can declare it at file scope.

But if you are asking if the patch changes the assumption GCC
makes about errno aliasing tentative definitions, I don't think
it does.  AFAICT, GCC has been making the assumption since 4.5.
Here's a slightly different test case than the one in pr92412
that makes it clearer:

   int errno;

   void* f (void)
   {
     int e = errno;
     void *p = __builtin_malloc (1);
     if (e != errno)   // eliminated
       __builtin_abort ();
     return p;
   }

But to be clear, my goal with the change is to let GCC (strlen
specifically) assume that errno doesn't alias local variables.

Martin