llvm dev - Dec 2019 - [cfe-dev] clang and -D_FORTIFY_SOURCE=1

Hi folks (CCing llvm-dev, but that's probably more of a cfe-dev topic),

As a follow-up to that old thread about -D_FORTIFY_SOURCE=n

    http://lists.llvm.org/pipermail/cfe-dev/2015-November/045845.html

And, more recently, to this fedora thread where clang/llvm -D_FORTIFY_SOURCE
support is claimed to be only partial:

    https://pagure.io/fesco/issue/2020

I dig into the glibc headers in order to have a better understanding of
what's
going on, and wrote my notes here:

    https://sergesanspaille.fedorapeople.org/fortify_source_requirements.rst

TL;DR: clang does provide a similar compile-time checking as gcc, but no runtime
checking. To assert that I wrote a small test suite:

    https://github.com/serge-sans-paille/fortify-test-suite

And indeed, clang doesn't pass it, mostly because it turns call to
__builtin__(.*)_chk into calls to __builtin__\1.

We need to support the runtime behavior of the following builtins:

- __builtin___memcpy_chk
- __builtin___memmove_chk
- __builtin___mempcpy_chk
- __builtin___memset_chk
- __builtin___snprintf_chk
- __builtin___sprintf_chk
- __builtin___stpcpy_chk
- __builtin___strcat_chk
- __builtin___strcpy_chk
- __builtin___strncat_chk
- __builtin___strncpy_chk
- __builtin___vsnprintf_chk
- __builtin___vsprintf_chk

And I'd like to implement them at clang level, leveraging their existing
implementation. Is that the right way to go / any comments / issue with that
approach ?

Are you sure you've diagnosed the issue correctly? __builtin___memcpy_chk
works correctly, as far as I know.

-Eli
> -----Original Message-----
> From: cfe-dev <cfe-dev-bounces at lists.llvm.org> On Behalf Of Serge
Guelton via
> cfe-dev
> Sent: Tuesday, December 3, 2019 2:07 AM
> To: cfe-dev at lists.llvm.org
> Cc: llvm-dev at lists.llvm.org
> Subject: [EXT] [cfe-dev] clang and -D_FORTIFY_SOURCE=1
>
> Hi folks (CCing llvm-dev, but that's probably more of a cfe-dev topic),
>
> As a follow-up to that old thread about -D_FORTIFY_SOURCE=n
>
>     http://lists.llvm.org/pipermail/cfe-dev/2015-November/045845.html
>
> And, more recently, to this fedora thread where clang/llvm -
> D_FORTIFY_SOURCE
> support is claimed to be only partial:
>
>     https://pagure.io/fesco/issue/2020
>
> I dig into the glibc headers in order to have a better understanding of
what's
> going on, and wrote my notes here:
>
>    
https://sergesanspaille.fedorapeople.org/fortify_source_requirements.rst
>
> TL;DR: clang does provide a similar compile-time checking as gcc, but no
> runtime
> checking. To assert that I wrote a small test suite:
>
>     https://github.com/serge-sans-paille/fortify-test-suite
>
> And indeed, clang doesn't pass it, mostly because it turns call to
> __builtin__(.*)_chk into calls to __builtin__\1.
>
> We need to support the runtime behavior of the following builtins:
>
> - __builtin___memcpy_chk
> - __builtin___memmove_chk
> - __builtin___mempcpy_chk
> - __builtin___memset_chk
> - __builtin___snprintf_chk
> - __builtin___sprintf_chk
> - __builtin___stpcpy_chk
> - __builtin___strcat_chk
> - __builtin___strcpy_chk
> - __builtin___strncat_chk
> - __builtin___strncpy_chk
> - __builtin___vsnprintf_chk
> - __builtin___vsprintf_chk
>
> And I'd like to implement them at clang level, leveraging their
existing
> implementation. Is that the right way to go / any comments / issue with
that
> approach ?
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

On Tue, 3 Dec 2019, Serge Guelton via cfe-dev wrote:
> Hi folks (CCing llvm-dev, but that's probably more of a cfe-dev topic),
>
> As a follow-up to that old thread about -D_FORTIFY_SOURCE=n
>
>    http://lists.llvm.org/pipermail/cfe-dev/2015-November/045845.html
>
> And, more recently, to this fedora thread where clang/llvm
-D_FORTIFY_SOURCE
> support is claimed to be only partial:
>
>    https://pagure.io/fesco/issue/2020
>
> I dig into the glibc headers in order to have a better understanding of
what's
> going on, and wrote my notes here:
>
>    https://sergesanspaille.fedorapeople.org/fortify_source_requirements.rst
>
> TL;DR: clang does provide a similar compile-time checking as gcc, but no
runtime
> checking. To assert that I wrote a small test suite:
>
>    https://github.com/serge-sans-paille/fortify-test-suite
>
> And indeed, clang doesn't pass it, mostly because it turns call to
> __builtin__(.*)_chk into calls to __builtin__\1.
I remember looking at the fortify macros recently, and iirc the issue was 
that the __builtin_object_size builtin, when used in an inline function, 
can't evaluate the size of the object in the context where it is inlined, 
which the glibc fortify macros/inline functions depend on.

This has been discussed before, e.g. here: 
http://lists.llvm.org/pipermail/cfe-dev/2015-November/045846.html

// Martin

The glibc implementation of FORTIFY_SOURCE is indeed not compatible with
clang, because it uses compiler extensions which were trivial to implement
in GCC's architecture, but difficult/impossible in clang's.

However, clang does provide other mechanisms by which the same results can
be achieved.

If anyone is interested in glibc fully supporting fortify mode with clang,
way to achieve this would be to look at the implementation in the bionic
libc's headers, and then implement the same technique in glibc.

On Tue, Dec 3, 2019 at 2:48 PM Martin Storsjö via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Tue, 3 Dec 2019, Serge Guelton via cfe-dev wrote:
>
> > Hi folks (CCing llvm-dev, but that's probably more of a cfe-dev
topic),
> >
> > As a follow-up to that old thread about -D_FORTIFY_SOURCE=n
> >
> >    http://lists.llvm.org/pipermail/cfe-dev/2015-November/045845.html
> >
> > And, more recently, to this fedora thread where clang/llvm
> -D_FORTIFY_SOURCE
> > support is claimed to be only partial:
> >
> >    https://pagure.io/fesco/issue/2020
> >
> > I dig into the glibc headers in order to have a better understanding
of
> what's
> > going on, and wrote my notes here:
> >
> >
> https://sergesanspaille.fedorapeople.org/fortify_source_requirements.rst
> >
> > TL;DR: clang does provide a similar compile-time checking as gcc, but
no
> runtime
> > checking. To assert that I wrote a small test suite:
> >
> >    https://github.com/serge-sans-paille/fortify-test-suite
> >
> > And indeed, clang doesn't pass it, mostly because it turns call to
> > __builtin__(.*)_chk into calls to __builtin__\1.
>
> I remember looking at the fortify macros recently, and iirc the issue was
> that the __builtin_object_size builtin, when used in an inline function,
> can't evaluate the size of the object in the context where it is
inlined,
> which the glibc fortify macros/inline functions depend on.
>
> This has been discussed before, e.g. here:
> http://lists.llvm.org/pipermail/cfe-dev/2015-November/045846.html
>
> // Martin
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191203/41ca7311/attachment.html>

On Tue, Dec 03, 2019 at 09:48:00PM +0200, Martin Storsjö via cfe-dev
wrote:
> On Tue, 3 Dec 2019, Serge Guelton via cfe-dev wrote:
> 
> > Hi folks (CCing llvm-dev, but that's probably more of a cfe-dev
topic),
> > 
> > As a follow-up to that old thread about -D_FORTIFY_SOURCE=n
> > 
> >    http://lists.llvm.org/pipermail/cfe-dev/2015-November/045845.html
> > 
> > And, more recently, to this fedora thread where clang/llvm
-D_FORTIFY_SOURCE
> > support is claimed to be only partial:
> > 
> >    https://pagure.io/fesco/issue/2020
> > 
> > I dig into the glibc headers in order to have a better understanding
of what's
> > going on, and wrote my notes here:
> > 
> >   
https://sergesanspaille.fedorapeople.org/fortify_source_requirements.rst
> > 
> > TL;DR: clang does provide a similar compile-time checking as gcc, but
no runtime
> > checking. To assert that I wrote a small test suite:
> > 
> >    https://github.com/serge-sans-paille/fortify-test-suite
> > 
> > And indeed, clang doesn't pass it, mostly because it turns call to
> > __builtin__(.*)_chk into calls to __builtin__\1.
> 
> I remember looking at the fortify macros recently, and iirc the issue was
> that the __builtin_object_size builtin, when used in an inline function,
> can't evaluate the size of the object in the context where it is
inlined,
> which the glibc fortify macros/inline functions depend on.
> 
> This has been discussed before, e.g. here:
> http://lists.llvm.org/pipermail/cfe-dev/2015-November/045846.html
That discussion is no longer relevant. __bos is only lowered to a
result, if the answer is definitely known and otherwise kept through a
good chunk of the pass pipeline, most importantly until after inlining
is done.

Joerg

> Are you sure you've diagnosed the issue correctly?
__builtin___memcpy_chk
works correctly, as far as I know.

100% sure. Let's have a look at the output of

  #include <string.h>
  static char dest[10];
  char* square(int n) {
    memcpy(dest, "hello", n);
    return dest;
  }

compiled with -D_FORTIFY_SOURCE=1 -O1 : https://godbolt.org/z/UvABWp

Clang issues a call to memcpy, while gcc issues a call to __memcpy_chk.
The call to __memcpy_chk performs extra runtime checks memcpy doesn't,
and clang doesn't generate the extra checks inline either. This is a
separate
concern from the accuracy of __builtin_object_size, just a different
runtime behavior.

Clang could generate the call to __memcpy_chk if its declaration is
available, which is the case for the glibc.

On Tue, Dec 3, 2019 at 8:41 PM Eli Friedman <efriedma at quicinc.com>
wrote:
> Are you sure you've diagnosed the issue correctly?
__builtin___memcpy_chk
> works correctly, as far as I know.
>
> -Eli
>
> > -----Original Message-----
> > From: cfe-dev <cfe-dev-bounces at lists.llvm.org> On Behalf Of
Serge
> Guelton via
> > cfe-dev
> > Sent: Tuesday, December 3, 2019 2:07 AM
> > To: cfe-dev at lists.llvm.org
> > Cc: llvm-dev at lists.llvm.org
> > Subject: [EXT] [cfe-dev] clang and -D_FORTIFY_SOURCE=1
> >
> > Hi folks (CCing llvm-dev, but that's probably more of a cfe-dev
topic),
> >
> > As a follow-up to that old thread about -D_FORTIFY_SOURCE=n
> >
> >     http://lists.llvm.org/pipermail/cfe-dev/2015-November/045845.html
> >
> > And, more recently, to this fedora thread where clang/llvm -
> > D_FORTIFY_SOURCE
> > support is claimed to be only partial:
> >
> >     https://pagure.io/fesco/issue/2020
> >
> > I dig into the glibc headers in order to have a better understanding
of
> what's
> > going on, and wrote my notes here:
> >
> >
> https://sergesanspaille.fedorapeople.org/fortify_source_requirements.rst
> >
> > TL;DR: clang does provide a similar compile-time checking as gcc, but
no
> > runtime
> > checking. To assert that I wrote a small test suite:
> >
> >     https://github.com/serge-sans-paille/fortify-test-suite
> >
> > And indeed, clang doesn't pass it, mostly because it turns call to
> > __builtin__(.*)_chk into calls to __builtin__\1.
> >
> > We need to support the runtime behavior of the following builtins:
> >
> > - __builtin___memcpy_chk
> > - __builtin___memmove_chk
> > - __builtin___mempcpy_chk
> > - __builtin___memset_chk
> > - __builtin___snprintf_chk
> > - __builtin___sprintf_chk
> > - __builtin___stpcpy_chk
> > - __builtin___strcat_chk
> > - __builtin___strcpy_chk
> > - __builtin___strncat_chk
> > - __builtin___strncpy_chk
> > - __builtin___vsnprintf_chk
> > - __builtin___vsprintf_chk
> >
> > And I'd like to implement them at clang level, leveraging their
existing
> > implementation. Is that the right way to go / any comments / issue
with
> that
> > approach ?
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191204/dbdc9896/attachment.html>