Bug 94910 - detect_stack_use_after_return=1 is much slower than clang's
Summary: detect_stack_use_after_return=1 is much slower than clang's
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 9.3.1
: P3 normal
Target Milestone: ---
Assignee: Martin Liška
URL: https://gcc.gnu.org/pipermail/gcc-pat...
Keywords: patch
: 95275 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-05-01 20:51 UTC by Rafael Avila de Espindola
Modified: 2020-06-12 08:30 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 11.0
Known to fail: 10.1.0, 8.4.0, 9.3.0
Last reconfirmed: 2020-05-04 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rafael Avila de Espindola 2020-05-01 20:51:52 UTC
The test I am using is from https://github.com/scylladb/seastar/. It can be build with

$ cmake -DCMAKE_BUILD_TYPE=Debug -GNinja <src>
$ ninja tests/unit/chunked_fifo_test

And can be run with:

Clang:

$ time ASAN_OPTIONS=detect_stack_use_after_return=1 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
...
1.80s user 0.02s system 99% cpu 1.826 total
$ time ASAN_OPTIONS=detect_stack_use_after_return=0 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
...
1.67s user 0.01s system 99% cpu 1.691 total

GCC:

$ time ASAN_OPTIONS=detect_stack_use_after_return=1 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
89.12s user 0.03s system 99% cpu 1:29.34 total
$ time ASAN_OPTIONS=detect_stack_use_after_return=0 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
1.32s user 0.02s system 99% cpu 1.350 total


So while plain asan is faster with gcc, enabling detect_stack_use_after_return makes it much slower.
Comment 1 Martin Liška 2020-05-04 05:27:39 UTC
Thanks for the report, I'll take a look.
Comment 2 Martin Liška 2020-05-15 11:31:31 UTC
I'm sorry but I can't install all pre-requisites.
Do you have a Docker/podman I can use to build the package?
Or do you have a reduced test-case?
Comment 3 Rafael Avila de Espindola 2020-05-15 16:28:39 UTC
Yes, our build bots use podman, so you can reproduce with:

$ git clone https://github.com/scylladb/seastar
$ cd seastar
$ podman run -v $PWD:$PWD:z -w $PWD -it docker.io/scylladb/scylla-toolchain:fedora-31-20200512
$ mkdir build
$ cd build
$ cmake -DCMAKE_BUILD_TYPE=Debug -G Ninja ../
$ ninja tests/unit/chunked_fifo_test
$ time ASAN_OPTIONS=detect_stack_use_after_return=0 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
$ time ASAN_OPTIONS=detect_stack_use_after_return=1 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
$ cd ..
$ dnf install clang
$ mkdir build-clang
$ cd build-clang
$ CC=clang CXX=clang++ cmake -DCMAKE_BUILD_TYPE=Debug -G Ninja ../ -DSeastar_CXX_FLAGS=-Wno-error
$ ninja tests/unit/chunked_fifo_test
$ time ASAN_OPTIONS=detect_stack_use_after_return=0 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
$ time ASAN_OPTIONS=detect_stack_use_after_return=1 ./tests/unit/chunked_fifo_test -t chunked_fifo_big
Comment 4 Martin Liška 2020-05-18 09:11:26 UTC
I was able to reproduce that!
The problem is the number of fake stacks that are exhausted.
I see 27000x:

FakeStack (stack_size_log=20):succ after 0
FakeStack (stack_size_log=20):succ after 0
FakeStack (stack_size_log=20):succ after 0
FakeStack (stack_size_log=20):succ after 0
...
FakeStack (stack_size_log=20):bail out after 8192
FakeStack (stack_size_log=20):bail out after 16384
FakeStack (stack_size_log=20):bail out after 4096
FakeStack (stack_size_log=20):bail out after 8192
...

So it seems that the program uses enormous number of stacks. Or the run-time does not release them in an efficient way.
Comment 5 Martin Liška 2020-05-18 09:12:33 UTC
> FakeStack (stack_size_log=20):bail out after 8192
> FakeStack (stack_size_log=20):bail out after 16384
> FakeStack (stack_size_log=20):bail out after 4096
> FakeStack (stack_size_log=20):bail out after 8192
> ...

This comes after the 27000 success Fake stack allocations.
Comment 6 Jakub Jelinek 2020-05-18 10:39:08 UTC
(In reply to Martin Liška from comment #5)
> > FakeStack (stack_size_log=20):bail out after 8192
> > FakeStack (stack_size_log=20):bail out after 16384
> > FakeStack (stack_size_log=20):bail out after 4096
> > FakeStack (stack_size_log=20):bail out after 8192
> > ...
> 
> This comes after the 27000 success Fake stack allocations.

I guess the important question is what is different with clang that it behaves differently, whether it allocates fake stack for fewer functions, or what else.  E.g. try ltrace on it to see how many each one allocates and if it is very different, it would be nice to understand why.
Comment 7 Martin Liška 2020-05-19 13:26:36 UTC
I was able to reproduce that on tramp3d as well. I bet the problem is that the inline expansion does not clear:
**SavedFlagPtr(FakeStack) = 0

as LLVM does:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L3279-L3292

Let me take a look.
Comment 8 Rafael Avila de Espindola 2020-05-19 21:05:44 UTC
I can confirm that the proposed patch fixes the issue for me.

Thank you so much!
Comment 9 Martin Liška 2020-05-20 09:11:10 UTC
(In reply to Rafael Avila de Espindola from comment #8)
> I can confirm that the proposed patch fixes the issue for me.
> 
> Thank you so much!

I thank you for the bug report. We had the regression since the very beginning!
Comment 10 Martin Liška 2020-05-25 08:32:56 UTC
*** Bug 95275 has been marked as a duplicate of this bug. ***
Comment 11 CVS Commits 2020-06-10 11:19:34 UTC
The master branch has been updated by Martin Liska <marxin@gcc.gnu.org>:

https://gcc.gnu.org/g:8b6731e674c76cb48a417f2eef74ced92a17f469

commit r11-1145-g8b6731e674c76cb48a417f2eef74ced92a17f469
Author: Martin Liska <mliska@suse.cz>
Date:   Tue May 19 16:57:56 2020 +0200

    Add missing store in emission of asan_stack_free.
    
    gcc/ChangeLog:
    
    2020-05-19  Martin Liska  <mliska@suse.cz>
    
            PR sanitizer/94910
            * asan.c (asan_emit_stack_protection): Emit
            also **SavedFlagPtr(FakeStack, class_id) = 0 in order to release
            a stack frame.
Comment 12 Martin Liška 2020-06-10 11:20:23 UTC
Fixed on master so far.
Comment 13 CVS Commits 2020-06-12 08:03:45 UTC
The releases/gcc-10 branch has been updated by Martin Liska <marxin@gcc.gnu.org>:

https://gcc.gnu.org/g:036b288ca4cf5d3b1d908ef97c25b7f92153ff8a

commit r10-8283-g036b288ca4cf5d3b1d908ef97c25b7f92153ff8a
Author: Martin Liska <mliska@suse.cz>
Date:   Tue May 19 16:57:56 2020 +0200

    Add missing store in emission of asan_stack_free.
    
    gcc/ChangeLog:
    
    2020-05-19  Martin Liska  <mliska@suse.cz>
    
            PR sanitizer/94910
            * asan.c (asan_emit_stack_protection): Emit
            also **SavedFlagPtr(FakeStack, class_id) = 0 in order to release
            a stack frame.
    
    (cherry picked from commit 8b6731e674c76cb48a417f2eef74ced92a17f469)
Comment 14 CVS Commits 2020-06-12 08:17:44 UTC
The releases/gcc-9 branch has been updated by Martin Liska <marxin@gcc.gnu.org>:

https://gcc.gnu.org/g:877d8d63228579bd56f94e6c56fbfeb015da08e5

commit r9-8671-g877d8d63228579bd56f94e6c56fbfeb015da08e5
Author: Martin Liska <mliska@suse.cz>
Date:   Tue May 19 16:57:56 2020 +0200

    Add missing store in emission of asan_stack_free.
    
    gcc/ChangeLog:
    
    2020-05-19  Martin Liska  <mliska@suse.cz>
    
            PR sanitizer/94910
            * asan.c (asan_emit_stack_protection): Emit
            also **SavedFlagPtr(FakeStack, class_id) = 0 in order to release
            a stack frame.
    
    (cherry picked from commit 8b6731e674c76cb48a417f2eef74ced92a17f469)
Comment 15 CVS Commits 2020-06-12 08:30:15 UTC
The releases/gcc-8 branch has been updated by Martin Liska <marxin@gcc.gnu.org>:

https://gcc.gnu.org/g:5d746191e271949e530d9e5f46cde7e7bf08272f

commit r8-10307-g5d746191e271949e530d9e5f46cde7e7bf08272f
Author: Martin Liska <mliska@suse.cz>
Date:   Tue May 19 16:57:56 2020 +0200

    Add missing store in emission of asan_stack_free.
    
    gcc/ChangeLog:
    
    2020-05-19  Martin Liska  <mliska@suse.cz>
    
            PR sanitizer/94910
            * asan.c (asan_emit_stack_protection): Emit
            also **SavedFlagPtr(FakeStack, class_id) = 0 in order to release
            a stack frame.
    
    (cherry picked from commit 8b6731e674c76cb48a417f2eef74ced92a17f469)
Comment 16 Martin Liška 2020-06-12 08:30:32 UTC
Fixed on all active branches.