[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2021-09-28 Thread rafael at espindo dot la via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #45 from Rafael Avila de Espindola  ---
(In reply to niek from comment #43)
> Does this mean (and could you please reconfirm) that bug 95317 has
> disappeared in trunk (which will become GCC 12)?

Hi,

I am not working on a project using coroutines right now, so I can't easily do
any test other than trying the reduced testcase as you did.

[Bug target/97734] GCC using branches when a conditional move would be better

2020-11-06 Thread rafael at espindo dot la via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97734

--- Comment #3 from Rafael Avila de Espindola  ---
I just realized I made a mistake when producing the reduced testcase. The
argument x should have been an uint32_t. Unfortunately, with that fix gcc
always produces a branch.

ICC still produces a cmov:

https://gcc.godbolt.org/z/zszrPK

I would be very happy with a __builtin_branchless_select.

[Bug target/97734] New: GCC using branches when a conditional move would be better

2020-11-05 Thread rafael at espindo dot la via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97734

Bug ID: 97734
   Summary: GCC using branches when a conditional move would be
better
   Product: gcc
   Version: 11.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 49511
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49511=edit
graph

Playing with the code in https://github.com/patmorin/arraylayout I noticed that
I could not reproduce the results for the eytzinger layout. It turns out the
issue was with current gcc selecting moves instead of conditional moves for
that particular code.

A reduced testcase is

#include 
uint64_t branchfree_search(uint64_t x, uint64_t n, uint32_t *a) {
uint64_t i = 0;
while (i < n) {
i = (x <= a[i]) ? (2*i + 1) : (2*i + 2);
}
uint64_t j = (i+1) >> __builtin_ffsl(~(i+1));
return (j == 0) ? n : j-1;
}

I have placed it in

https://gcc.godbolt.org/z/Krqrz7

Results
* ICC: conditional move
* Clang: branches
* GCC 6.4: conditional move
* Newer GCCs with -O2:  branches
* GCC with -Os: conditional move

The attached graph shows how the conditional move is better for "small" array
sizes.

[Bug libstdc++/90295] Please define ~exception_ptr inline

2020-10-06 Thread rafael at espindo dot la via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90295

--- Comment #5 from Rafael Avila de Espindola  ---
> Fixed for GCC 11, and not plausible to backport.

Thank you so much! Looking forward to GCC 11.

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-07-21 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #31 from Rafael Avila de Espindola  ---
Hi Iain,

Any update on this? If there is any way I can help, please let me know. It has
been a decade since I looked into gcc, but I should still be able to test
patches or implement small side changes.

[Bug libstdc++/96036] New: Please make std::optinal noexcept constructible when possible

2020-07-02 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96036

Bug ID: 96036
   Summary: Please make std::optinal noexcept constructible when
possible
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Only the move constructor of std::optional is conditionally noexcept.

The standard in res.on.exception.handling says:

An implementation may strengthen the exception specification for a non-virtual
function by adding a non-throwing exception specification.

So it should be possible to also make

template < class U = value_type >
constexpr optional( U&& value );

conditionally noexcept.

libstdc++ already does that for std::tuple.

[Bug c++/95302] function attributed to be deprecated cannot include a typedef/using

2020-06-29 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95302

Rafael Avila de Espindola  changed:

   What|Removed |Added

 CC||rafael at espindo dot la

--- Comment #2 from Rafael Avila de Espindola  ---
The same problem happens with a namespace alias:

namespace foo {
void bar();
}
[[deprecated("zed")]] inline void zed() {
namespace a = foo;
a::bar();
}

Produces:

test.cc: In function ‘void zed()’:
test.cc:6:8: warning: ‘void zed()’ is deprecated: zed
[-Wdeprecated-declarations]
6 | a::bar();
  |^~~
test.cc:4:35: note: declared here
4 | [[deprecated("zed")]] inline void zed() {
  |   ^~~


If instead of the alias foo::bar() is used directly, no warning is produced.

The issue is still present on current master
(346bce6fe0cf1ed5f6a7ad732d2361d77b203c87).

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-06-22 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

Rafael Avila de Espindola  changed:

   What|Removed |Added

  Attachment #48723|0   |1
is obsolete||

--- Comment #29 from Rafael Avila de Espindola  ---
Created attachment 48771
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48771=edit
Testcase without lambda coroutines

I modified the testcase to also build with clang and not depend on async
lambdas. This still reproduces the problem with gcc with undefined behaviour
sanitizer, but works with with clang and sanitizers and gcc with valgrind.

[Bug c++/95703] New: Please backport 0998d2fd59e7a5eb3a3566c57625702bbdc6a05f to gcc 9

2020-06-16 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95703

Bug ID: 95703
   Summary: Please backport
0998d2fd59e7a5eb3a3566c57625702bbdc6a05f to gcc 9
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
CC: jason at redhat dot com
  Target Milestone: ---

We (seastar) recently hit a build error with gcc 9 because the fix in
0998d2fd59e7a5eb3a3566c57625702bbdc6a05f is missing.

Given that P1286R2 is about c++11 and newer, could the fix be backported to gcc
9?

The original commit message is:

Implement P1286R2, Contra CWG1778

The C++11 requirement that an explicit exception-specification on a
defaulted function match the implicit one was found to be problematic for
std::atomic.  This paper, adopted in February, simply removes that
requirement: if an explicitly defaulted function has a different
exception-specification, that now works just like a user-written function:
either it isn't noexcept when it could be, or it is noexcept and will call
terminate if an exception is thrown.

[Bug c++/95675] internal compiler error: in build_over_call

2020-06-14 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95675

Rafael Avila de Espindola  changed:

   What|Removed |Added

Version|unknown |11.0

--- Comment #1 from Rafael Avila de Espindola  ---
Sorry, gcc *10* has no problem with it.

[Bug c++/95675] New: internal compiler error: in build_over_call

2020-06-14 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95675

Bug ID: 95675
   Summary: internal compiler error: in build_over_call
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 48729
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48729=edit
testcase

With current master (06712fc68dc9843d9af7c7ac10047f49d305ad76), running

g++ -S test.ii  -std=c++20

In the attached file causes

test.ii:511:41: internal compiler error: in build_over_call, at cp/call.c:8971 

g++ 9 has no problems with it.

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-06-12 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

Rafael Avila de Espindola  changed:

   What|Removed |Added

  Attachment #48579|0   |1
is obsolete||

--- Comment #28 from Rafael Avila de Espindola  ---
Created attachment 48723
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48723=edit
new testcase

This testcase still fails with gcc 50ff02b534195c12298c64311d03a8b2d2dc261f

$ ~/gcc/inst/bin/g++ -std=gnu++20 -fcoroutines -g coroutines_test.cc -o t &&
valgrind ./t
...
==105582== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
% ~/gcc/inst/bin/g++ -std=gnu++20 -fcoroutines -g coroutines_test.cc 
-fsanitize=undefined -o t &&  ./t
/home/espindola/scylla/t/coroutines_test.cc:793: runtime error: member call on
misaligned address 0x0020d854 for type 'struct future', which requires 8
byte alignment
...

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-06-10 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #25 from Rafael Avila de Espindola  ---
(In reply to CVS Commits from comment #24)
> The master branch has been updated by Iain D Sandoe :

Thanks!

I can confirm that the reduced testcase is now fixed. On the original test I
still get

 runtime error: member call on misaligned address 0x0003 for type
'struct future', which requires 8 byte alignment

I will try reducing the testcase again and upload the results.

[Bug c++/95368] gcc things that a lambda capture is both const and mutable

2020-05-28 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95368

--- Comment #2 from Rafael Avila de Espindola  ---
I just tested a few compilers with the testcase from comment 1:

* clang version 11.0.0 (https://github.com/llvm/llvm-project.git
0796b170fb3bf38e6cc4e59746120b37c9a9cd9f):
  Accepts it.

* g++ (GCC) 10.1.1 20200507 (Red Hat 10.1.1-1):
  Rejects it:
  test.cc:17:14: error: passing ‘const foo’ as ‘this’ argument discards
qualifiers [-fpermissive]

* gcc (GCC) 9.3.1 20200528
  Rejects it:
  test.cc:17:14: error: passing ‘const foo’ as ‘this’ argument discards
qualifiers

[Bug c++/95368] New: gcc things that a lambda capture is both const and mutable

2020-05-27 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95368

Bug ID: 95368
   Summary: gcc things that a lambda capture is both const and
mutable
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

gcc accepts

#include 

struct foo {
void func();
};

void bar(foo& v) {
[v]() {
static_assert(std::is_same_v);
[v]() mutable {
static_assert(std::is_same_v);
//v.func(); 
}();
}();
}


But uncomment the call and you get

test.cc:12:20: error: passing ‘const foo’ as ‘this’ argument discards
qualifiers [-fpermissive]
   12 | v.func();

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-05-21 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #20 from Rafael Avila de Espindola  ---
The attached testcase also fails with just -fsanitize=undefined. I have tested
with gcc version

gcc (GCC) 10.1.1 20200507 (Red Hat 10.1.1-1)

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-05-21 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

Rafael Avila de Espindola  changed:

   What|Removed |Added

  Attachment #48547|0   |1
is obsolete||

--- Comment #19 from Rafael Avila de Espindola  ---
Created attachment 48579
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48579=edit
Single file testcase

$ g++ -g -fcoroutines -std=gnu++20 coroutines_test.cc -o t && valgrind ./t

no errors, exit value 42

$ g++ -g -fcoroutines -std=gnu++20 coroutines_test.cc -o t -fsanitize=address
-fsanitize=undefined  && ./t
coroutines_test.cc:561:26: runtime error: member access within misaligned
address 0x002231e6 for type 'struct promise_base', which requires 8 byte
alignment
0x002231e6: note: pointer points here
 5c 5d c3 cc 55 48  89 e5 41 57 41 56 41 55  41 54 53 48 83 ec 58 48  89 7d 88
48 8d 5d 90 49  89 df
 ^
AddressSanitizer:DEADLYSIGNAL
=
==112180==ERROR: AddressSanitizer: SEGV on unknown address 0x002231ee (pc
0x0021adf2 bp 0x7ffdf0604570 sp 0x7ffdf06044f0 T0)
==112180==The signal is caused by a WRITE memory access.
#0 0x21adf2 in seastar::internal::future_base::detach_promise()
/home/espindola/scylla/t/coroutines_test.cc:561
#1 0x21ac94 in seastar::internal::future_base::clear()
/home/espindola/scylla/t/coroutines_test.cc:556
#2 0x21acd2 in seastar::internal::future_base::~future_base()
/home/espindola/scylla/t/coroutines_test.cc:559
#3 0x21e3d4 in seastar::future<>::~future()
/home/espindola/scylla/t/coroutines_test.cc:645
#4 0x21d1e1 in seastar::internal::awaiter<>::~awaiter()
/home/espindola/scylla/t/coroutines_test.cc:862
#5 0x215d02 in main::{lambda()#1}::operator()() const [clone .actor]
/home/espindola/scylla/t/coroutines_test.cc:1172
#6 0x214d3b in operator() /home/espindola/scylla/t/coroutines_test.cc:1169
#7 0x216ab8 in apply /home/espindola/scylla/t/coroutines_test.cc:92
#8 0x216b8a in apply >
/home/espindola/scylla/t/coroutines_test.cc:97
#9 0x216d10 in operator() /home/espindola/scylla/t/coroutines_test.cc:680
#10 0x21765e in
satisfy_with_result_of::then_impl_nrvo, seastar::future<> >&&)> mutable:: >
/home/espindola/scylla/t/coroutines_test.cc:793
#11 0x216fee in operator() /home/espindola/scylla/t/coroutines_test.cc:678
#12 0x218881 in run_and_dispose
/home/espindola/scylla/t/coroutines_test.cc:376
#13 0x2143c2 in seastar::reactor::run_tasks(seastar::reactor::task_queue&)
/home/espindola/scylla/t/coroutines_test.cc:1130
#14 0x21463e in seastar::reactor::run()
/home/espindola/scylla/t/coroutines_test.cc:1138
#15 0x216852 in main /home/espindola/scylla/t/coroutines_test.cc:1173
#16 0x7faee1994041 in __libc_start_main (/lib64/libc.so.6+0x27041)
#17 0x21190d in _start (/home/espindola/scylla/t/t+0x21190d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/espindola/scylla/t/coroutines_test.cc:561
in seastar::internal::future_base::detach_promise()
==112180==ABORTING

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-05-20 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #18 from Rafael Avila de Espindola  ---
(In reply to Avi Kivity from comment #17)
> Is that the test were a lambda coroutine is called from future::then()? In
> that case it's a real use-after-free.

It was reduced from that to just

#include 
#include 
using namespace seastar;
int main(int argc, char** argv) {
seastar::app_template app;
app.run(argc, argv, [] () -> future<> {
future<> xyz = make_ready_future<>().then([] {});
co_await std::move(xyz);
});
return 0;
}

Which doesn't have a user after free.

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-05-20 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #16 from Rafael Avila de Espindola  ---
> @Rafael: Can you please append output with:
> export UBSAN_OPTIONS="print_stacktrace=1"

I also added halt_on_error=1:abort_on_error=1:

It is

../tests/unit/coroutines_test.cc:11:5: runtime error: member call on misaligned
address 0x41b58ab3 for type 'struct awaiter', which requires 8 byte
alignment
0x41b58ab3: note: pointer points here

Reactor stalled for 261 ms on shard 0.
Backtrace:
  /lib64/libasan.so.6+0x00045bad
...
#0 0xf3605a in main::{lambda()#1}::operator()() const [clone .actor]
../tests/unit/coroutines_test.cc:11
#1 0xf3b20b in std::__n4861::coroutine_handle::resume() const
/usr/include/c++/10/coroutine:126
#2 0xf3b5ed in
seastar::internal::coroutine_traits_base<>::promise_type::run_and_dispose()
../include/seastar/core/coroutine.hh:104
#3 0x11c8892 in seastar::reactor::run_tasks(seastar::reactor::task_queue&)
../src/core/reactor.cc:2151
#4 0x11cc8db in seastar::reactor::run_some_tasks()
../src/core/reactor.cc:2566
#5 0x11d1aa0 in seastar::reactor::run() ../src/core/reactor.cc:2721
#6 0xf4ef5e in seastar::app_template::run_deprecated(int, char**,
std::function&&) ../src/core/app-template.cc:202
#7 0xf4cf00 in seastar::app_template::run(int, char**,
std::function ()>&&) ../src/core/app-template.cc:115
#8 0xf4d3fa in seastar::app_template::run(int, char**,
std::function ()>&&) ../src/core/app-template.cc:130
#9 0xf36a70 in main ../tests/unit/coroutines_test.cc:8
#10 0x7fac0533d041 in __libc_start_main (/lib64/libc.so.6+0x27041)
#11 0xf3466d in _start
(/home/espindola/scylla/scylla/seastar/build-dbg/tests/unit/coroutines_test+0xf3466d)

I will try to reduce the test to not need seastar.

[Bug sanitizer/94910] detect_stack_use_after_return=1 is much slower than clang's

2020-05-19 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94910

--- Comment #8 from Rafael Avila de Espindola  ---
I can confirm that the proposed patch fixes the issue for me.

Thank you so much!

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-05-19 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #12 from Rafael Avila de Espindola  ---
(In reply to Martin Liška from comment #6)
> Thank you, can you please attach a pre-processed file (-E) so that one
> doesn't need to clone seastar repository?

The testcase that is attached doesn't require any seastar headers, just linking
with libseastar.a.

I will try to reduce it to a self contained test.

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-05-15 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #5 from Rafael Avila de Espindola  ---
With a seastar patched for c++ 20 (mostly dropping a few experimental/ from
includes and experimental:: from names), the following is all that is needed:

#include 
#include 
using namespace seastar;
int main(int argc, char** argv) {
seastar::app_template app;
app.run(argc, argv, [] () -> future<> {
future<> xyz = make_ready_future<>().then([] {});
co_await std::move(xyz);
});
return 0;
}

I have attached a partially preprocessed version that can be use with current
seastar from https://github.com/scylladb/seastar.

First build seastar in debug mode:

$ mkdir build
$ cd build
$ cmake -DCMAKE_BUILD_TYPE=Debug .. -GNinja
$ ninja

And the test can be built with

$ g++ test.cc -fcoroutines $(pkg-config --cflags --libs
/seastar/build-dbg/seastar.pc --static)  -o t-asan

It crashes like this:

$ export ASAN_OPTIONS=disable_coredump=0:abort_on_error=1
$ export UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1
$ gdb --args ./t-asan -c 1
...
run
...
test.cc:3749:5: runtime error: member call on misaligned address 0x41b58ab3
for type 'struct awaiter', which requires 8 byte alignment
0x41b58ab3: note: pointer points here


If seastar is build with clang (CC=clang CXX=clang++ cmake...) or if the
sanitizers are disabled (-DSeastar_SANITIZE=OFF) and valgrind is used instead,
the program doesn't crash.

[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines

2020-05-15 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

--- Comment #4 from Rafael Avila de Espindola  ---
Created attachment 48547
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48547=edit
testcase

[Bug sanitizer/94910] detect_stack_use_after_return=1 is much slower than clang's

2020-05-15 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94910

--- Comment #3 from Rafael Avila de Espindola  ---
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

[Bug c++/83028] Incorrect -Wsequence-point warning in correct C++17 code with new evaluation order rules

2020-05-15 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83028

Rafael Avila de Espindola  changed:

   What|Removed |Added

 CC||rafael at espindo dot la

--- Comment #3 from Rafael Avila de Espindola  ---
We just hit this and incorrectly assumed our codebase had a bug. It would be
really nice for this warning to take -std=XX into consideration and not warn
for code that is now valid.

[Bug sanitizer/95137] New: Sanitizers seem to be missing support for coroutines

2020-05-14 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137

Bug ID: 95137
   Summary: Sanitizers seem to be missing support for coroutines
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: sanitizer
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at 
gcc dot gnu.org
  Target Milestone: ---

In seastar (http://seastar.io/) we have experimental support for coroutines and
a simple test to check that it is working.

It currently passes with clang+asan and gcc+valgrind. Unfortunately it fails
with gcc + asan. It is not even that gcc's asan reports an error, it seems to
corrupt memory and the test hits a segmentation fault.

Sorry for the super generic bug report. I just wanted to check if it is a know
issue before I try to create a smallish testcase to add here.

[Bug other/12411] Missed -Wsequence-point on functions (example reduced from historical GCC source)

2020-05-12 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=12411

Rafael Avila de Espindola  changed:

   What|Removed |Added

 CC||rafael at espindo dot la

--- Comment #7 from Rafael Avila de Espindola  ---
I think we just hit a case similar to this bug where it would be nice get a
warning.

Given

-
struct foo {
void bar(int);
};
foo get_foo(int);
void g() {
int a = 0;
get_foo(a).bar(a++);
}


GCC will warn about the value of a. If instead of an integer we have an object,
like in


#include 
struct zed {
zed();
zed(const zed&);
zed(zed&&);
};
struct foo {
void bar(zed);
};
foo get_foo(zed);
void g() {
zed a;
get_foo(a).bar(std::move(a));
}


GCC produces no warning.

Let me know if I should report a new bug instead.

[Bug middle-end/95014] New: gcc fails to merge two identical returns

2020-05-08 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95014

Bug ID: 95014
   Summary: gcc fails to merge two identical returns
   Product: gcc
   Version: 10.1.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: middle-end
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Given

#include 
union any {
any(any&& x) noexcept {
if (x.st < 4) {
st = x.st;
x.st = 0;
} else {
ex = x.ex;
x.ex = nullptr;
}
}
long st;
void* ex;
};
void f(any* a, any* b) {
new (a) any(std::move(*b));
}

gcc produces

movq(%rsi), %rax
movq%rax, (%rdi)
movq$0, (%rsi)
cmpq$3, %rax
jg  .L2
ret
.L2:
ret


clang produces

movq(%rsi), %rax
cmpq$3, %rax
movq%rax, (%rdi)
movq$0, (%rsi)
retq

So clang has a redundant cmpq and gcc has the cmpq and two identical branch
destinations.

[Bug middle-end/94992] gcc thinks a member variable is uninitialised

2020-05-08 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94992

--- Comment #3 from Rafael Avila de Espindola  ---
For completeness, this is a reduction of a std::swap(x,x). The placement new
was originally in the move assignment operator.

I was able to reproduce this with gcc 9 by moving a few functions out of line.

As far as I can tell, the relevant standard line is from basic.life:

The lifetime of an object \placeholder{o} of type \tcode{T} ends when:
...
\item the storage which the object occupies is released,
or is reused by an object that is not nested within
\placeholder{o}\iref{intro.object}.

[Bug middle-end/94992] New: gcc thinks a member variable is uninitialised

2020-05-07 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94992

Bug ID: 94992
   Summary: gcc thinks a member variable is uninitialised
   Product: gcc
   Version: 10.1.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: middle-end
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 48477
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48477=edit
testcase

In the attached testcase gcc with -O1 (but not -O0 or -O2) produces the
following warning

futures_test.cc:57:22: warning: ‘f2.my_future::_promise’ is used uninitialized
in this function [-Wuninitialized]
   57 | : _promise(x._promise)

The produced assembly also uses uninitialized memory.

The problem is that _promise is initialized to nullptr in the default
constructor.

[Bug sanitizer/94910] New: detect_stack_use_after_return=1 is much slower than clang's

2020-05-01 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94910

Bug ID: 94910
   Summary: detect_stack_use_after_return=1 is much slower than
clang's
   Product: gcc
   Version: 9.3.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: sanitizer
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at 
gcc dot gnu.org
  Target Milestone: ---

The test I am using is from https://github.com/scylladb/seastar/. It can be
build with

$ cmake -DCMAKE_BUILD_TYPE=Debug -GNinja 
$ 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.

[Bug libstdc++/94811] Please make make_tuple noexcept when possible

2020-04-28 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94811

--- Comment #4 from Rafael Avila de Espindola  ---
(In reply to Jonathan Wakely from comment #3)
> (In reply to Rafael Avila de Espindola from comment #0)
> > So it should be possible to make the std::tuple constructor and
> 
> Isn't that already done?

It is :-)

I had tested this bit with gcc 9, but with 10 the following assert passes:

static_assert(std::is_nothrow_constructible_v, int>);

Given CTAD I am happy to call this bug fixed if you want.

[Bug c++/94811] New: Please make make_tuple noexcept when possible

2020-04-27 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94811

Bug ID: 94811
   Summary: Please make make_tuple noexcept when possible
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

The standard in res.on.exception.handling says:

An implementation may strengthen the exception specification for a non-virtual
function by adding a non-throwing exception specification.

So it should be possible to make the std::tuple constructor and std::make_tuple
noexcept when the arguments have noexcept copy or move constructors.

[Bug c++/94645] [10 Regression][concepts] incorrect concept evaluation with decltype, plus internal error since r10-7554-gf1ad7bac76b66257

2020-04-23 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94645

--- Comment #13 from Rafael Avila de Espindola  ---
Thank you so much. I can confirm that scylla now builds with gcc master with
just a few fixes on the scylla side (we build with -Werror).

There is a couple of test failures. I will try to reduce those and open new
bugs as appropriate.

[Bug c++/94645] [10 Regression][concepts] incorrect concept evaluation with decltype, plus internal error since r10-7554-gf1ad7bac76b66257

2020-04-21 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94645

--- Comment #8 from Rafael Avila de Espindola  ---
The internal compiler error reduces to

struct unordered_map {
int cend() const noexcept;
};
template  concept HasMapInterface = requires(a t) { t.cend(); };
template 
requires HasMapInterface struct l {
friend void foo(l opt) {
  ([]() {})();
  }
};
struct p {
  static unordered_map map();
};
void g(l *y) { foo(*y); }

[Bug c++/94645] incorrect concept evaluation with decltype, plus internal erropr

2020-04-19 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94645

--- Comment #2 from Rafael Avila de Espindola  ---
This reduces to just

template  concept HasMapInterface = requires(a t) { t.cend; };
template 
requires HasMapInterface struct l {};
struct mymap {
  int cend();
};
struct p {
  static mymap map();
};
l q;

[Bug c++/94644] New: Wrong is_nothrow_move_constructible result if used in a template first

2020-04-17 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94644

Bug ID: 94644
   Summary: Wrong is_nothrow_move_constructible result if used in
a template first
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Tested with c5bac7d127f288fd2f8a1f15c3f30da5903141c6.

Given

---
#include 
struct future_state_base {
protected:
~future_state_base() noexcept;
};
#ifdef foo
static_assert(std::is_nothrow_move_constructible::value,
"future_state_base's move constructor must not throw");
#endif
template 
struct future_state : public future_state_base {
static_assert(std::is_nothrow_move_constructible::value,
"future_state_base's move constructor must not throw");
};
void f(future_state x) {
}
---

g++ produces no error with just -c, but if I add -Dfoo both static asserts
fails. With clang, and libstdc++, I get one or two errors depending on whether
-Dfoo is used.

[Bug c++/94418] Please make reverse_iterator nothrow constructible when possible

2020-03-30 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94418

--- Comment #1 from Rafael Avila de Espindola  ---
For what it is worth, libc++ implements this. Given

static_assert(std::is_nothrow_copy_constructible_v::reverse_iterator>);


With libstdc++:

$ clang -S test3.cc -std=c++17
test3.cc:3:1: error: static_assert failed due to requirement...

with libc++

$ clang clang -S test3.cc -std=c++17 -stdlib=libc++


[Bug c++/94418] New: Please make reverse_iterator nothrow constructible when possible

2020-03-30 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94418

Bug ID: 94418
   Summary: Please make reverse_iterator nothrow constructible
when possible
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

The standard in res.on.exception.handling says:

An implementation may strengthen the exception specification for a non-virtual
function by adding a non-throwing exception specification.

So, as far as I understand, libstdc++ could make the constructors of
std::reverse_iterator noexcept when the corresponding constructors of the
underlying iterators are noexcept.

[Bug c++/94112] Please add a warning for potentially throwing code in noexcept function

2020-03-10 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94112

--- Comment #2 from Rafael Avila de Espindola  ---
(In reply to Martin Sebor from comment #1)
> Confirmed with the output below.  -Wterminate is fully implemented in the
> C++ front-end so it doesn't know about what might happen in called
> functions.  Implementing the detection across inlined functions requires
> moving the warning to the middle-end and deciding how to deal with
> situations where the called function throws only under some non-constant
> conditions (e.g., issue -Wterminate regardless, or a -Wmaybe-terminate kind
> of a warning, or none).


I would be happy even with a "noexcept function calls a non-noexcept one"
warning.

[Bug c++/94112] New: Please add a warning for potentially throwing code in noexcept function

2020-03-09 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94112

Bug ID: 94112
   Summary: Please add a warning for potentially throwing code in
noexcept function
   Product: gcc
   Version: 10.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

GCC produces a warning for

void foo() noexcept {
throw 42;
}

But not for

static void bar() {
throw 42;
}
void foo() noexcept {
bar();
}

Such a warning would be quite handy for checking the usage of noexcept in a
codebase.

[Bug libstdc++/94033] is_trivially_copy_constructible<> fails with compiler error on complicated object with private default constructor

2020-03-07 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94033

--- Comment #5 from Rafael Avila de Espindola  ---
The regression (at least with the reduced testcase I uploaded) is from:

commit 58487c21b6a47c3fff6c6958684de866216a5593 (HEAD)
Author: Jonathan Wakely 
Date:   Mon May 20 12:32:51 2019 +0100

PR c++/90532 Ensure __is_constructible(T[]) is false

[Bug libstdc++/94033] is_trivially_copy_constructible<> fails with compiler error on complicated object with private default constructor

2020-03-06 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94033

--- Comment #5 from Rafael Avila de Espindola  ---
Created attachment 47994
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47994=edit
reduced testcase

I have reduced it a bit further.

[Bug libstdc++/94032] New: Please provide std::string::__resize_default_init

2020-03-04 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94032

Bug ID: 94032
   Summary: Please provide std::string::__resize_default_init
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

There is a c++ proposal at

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1072r5.html

and libc++ already has an implementation that is used by abseil:

https://github.com/abseil/abseil-cpp/blob/12bc53e0318d80569270a5b26ccbc62b52022b89/absl/strings/internal/resize_uninitialized.h#L39

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2020-02-19 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

--- Comment #15 from Rafael Avila de Espindola  ---
In gcc 9 it is pretty easy to avoid this warning by adding an assert or
builtin_unreachable and we have done that in seastar.

Unfortunately the warning still shows up with gcc 8. Is there a known way to
work around this warning with gcc 8 or should we should disable the warning?

[Bug libstdc++/93584] New: std::string::find_first_not_of is about 9X slower than strspn

2020-02-04 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93584

Bug ID: 93584
   Summary: std::string::find_first_not_of is about 9X slower than
strspn
   Product: gcc
   Version: 9.2.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 47779
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47779=edit
testcase

In the attached test program the benchmark function takes 0.26837 seconds if
calling std::string::find_first_not_of and 0.0291547 seconds if calling strspn.

[Bug c++/93437] On protobuf generated code, -Warray-bounds is brittle and not very helpful

2020-01-27 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93437

Rafael Avila de Espindola  changed:

   What|Removed |Added

 CC||msebor at gcc dot gnu.org

--- Comment #1 from Rafael Avila de Espindola  ---
The warning is no longer issued after 6889a3acfeed47265886676c6d43b04ef799fb82.

Martin, is this a duplicate of middle-end/91631?

[Bug c++/93437] New: On protobuf generated code, -Warray-bounds is brittle and not very helpful

2020-01-25 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93437

Bug ID: 93437
   Summary: On protobuf generated code, -Warray-bounds is brittle
and not very helpful
   Product: gcc
   Version: 9.2.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 47710
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47710=edit
testcase

In the attached testcase, "g++ -S metrics2.pb.ii -O2 -Wall" produces no
warning, but "g++ -S metrics2.pb.ii -O3 -Wall" produces

In constructor ‘Bucket::Bucket()’,
inlined from ‘void InitDefaultsBucket()’ at metrics2.pb.ii:30:5:
metrics2.pb.ii:24:13: warning: ‘void* memset(void*, int, size_t)’ offset [9,
12] from the object at ‘_Bucket_default_instance_’ is out of the bounds of
referenced subobject ‘Bucket::cumulative_count_’ with type ‘unsigned int’ at
offset 4 [-Warray-bounds]

Small modifications to the source avoid the warning. For instance, replacing
the placement new

new (ptr)::Bucket();

With a regular new avoids the warning. Similar for removing memset:

  memset(&_has_bits_, 0, sizeof(_has_bits_));

So GCC should at least be a bit more consistent about when the warning is
produced, but why is it needed? Is it invalid to initialized multiple members
with a memset? Should the protobuf compiler be disabling this warning with a
pragma around the memset?

[Bug middle-end/92957] gcc produces a duplicated load, clang doesn't

2019-12-16 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92957

Rafael Avila de Espindola  changed:

   What|Removed |Added

Version|9.2.1   |10.0

--- Comment #1 from Rafael Avila de Espindola  ---
gcc trunk (r279443) still produces the same code.

[Bug middle-end/92957] New: gcc produces a duplicated load, clang doesn't

2019-12-16 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92957

Bug ID: 92957
   Summary: gcc produces a duplicated load, clang doesn't
   Product: gcc
   Version: 9.2.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: middle-end
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 47507
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47507=edit
testcase

In the attached testcase gcc produces

_Z3fooPN7seastar12future_stateIJiEEES2_:
movq(%rsi), %rax
movq%rax, (%rdi)
movq$0, (%rsi)
movq(%rdi), %rax
subq$2, %rax
cmpq$1, %rax
ja  .L1
movl8(%rsi), %eax
movl%eax, 8(%rdi)
.L1:
ret

The "movq   (%rdi), %rax" load is redundant and clang avoids it.

[Bug c++/92722] New: gcc considers "padding" byte of empty lambda to be uninitialized

2019-11-28 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92722

Bug ID: 92722
   Summary: gcc considers "padding" byte of empty lambda to be
uninitialized
   Product: gcc
   Version: 9.2.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 47394
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=47394=edit
testcase

seastar has a variant of std::function that cannot be copied and is called
noncopyable_function. It also has a small buffer so that small functions don't
need an indirection.

The implementation is careful to only move sizeof(Func) bytes to avoid using
uninitialized memory. It looks like gcc is careful enough to consider padding
bytes to be initialized, but when given an empty lambda, sizeof(Func) is 1 but
no bytes are actually used and gcc ends up issuing a warning.

A reduced testcase is attached. Compiling with g++ -S failure_injector_test.ii
-O1 -Wall produces the warning:

failure_injector_test.ii:22:27: warning: ‘a.noncopyable_function::direct[0]’ is
used uninitialized in this function [-Wuninitialized]

This seems to be generated because

noncopyable_function(Func&& func) {
static_assert(sizeof(Func) == 1);
new (reinterpret_cast(direct)) Func(std::move(func));
}

Doesn't initialize any bytes, but sizeof(Func) is 1, so trivial_direct_move
still uses direrct[0]

[Bug libstdc++/91997] pretty printers: The __node_type type alias in _Hashtable is not available

2019-11-28 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91997

--- Comment #3 from Rafael Avila de Espindola  ---
(In reply to Jonathan Wakely from comment #2)
> Rafael, I'm unable to reproduce this with unordered containers. Do you have
> a testcase?

I was able to reproduce it with 2 files:

$ cat test.cc
#include 
void foo(std::unordered_map );
int main() {
  std::unordered_map map;
  map[42] = 1;
  foo(map);
  return 0;
}
$ cat test2.cc
#include 
#include 
void foo(std::unordered_map ) {
  auto it = map.begin();
  printf("%d\n", *it);
}
$ g++ test.cc test2.cc -o t -g
$ /usr/bin/gdb -q -ex "b printf" -ex r -ex bt ./t
Reading symbols from ./t...
Breakpoint 1 at 0x204b10
Starting program: /home/espindola/scylla/t
warning: Loadable section ".note.gnu.property" outside of ELF segments
warning: Loadable section ".note.gnu.property" outside of ELF segments
warning: Loadable section ".note.gnu.property" outside of ELF segments

Breakpoint 1, __printf (format=0x20081b "%d\n") at printf.c:28
28  {
#0  __printf (format=0x20081b "%d\n") at printf.c:28
#1  0x0020494a in foo (Traceback (most recent call last):
  File "/lib64/../share/gcc-9/python/libstdcxx/v6/printers.py", line 957, in
children
data = self.flatten (imap (self.format_one, StdHashtableIterator
(self.hashtable(
  File "/lib64/../share/gcc-9/python/libstdcxx/v6/printers.py", line 880, in
__init__
self.node_type = find_type(hash.type, '__node_type').pointer()
  File "/lib64/../share/gcc-9/python/libstdcxx/v6/printers.py", line 97, in
find_type
field = typ.fields()[0]
IndexError: list index out of range
map=
std::unordered_map with 1 element) at test2.cc:5
#2  0x00203017 in main () at test.cc:6

[Bug middle-end/92183] New: gcc tries to create a relocation in a mergeable section

2019-10-22 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92183

Bug ID: 92183
   Summary: gcc tries to create a relocation in a mergeable
section
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: middle-end
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Given

struct foo {
  const char *bar;
  const char *zed;
};
void g(struct foo *r);
void f() {
  struct foo t = {"bar", "zed"};
  g();
}


gcc -O3 produces

.section.rodata.str1.1,"aMS",@progbits,1
.LC0:
.string "bar"
.LC1:
.string "zed"



.section.rodata.cst8,"aM",@progbits,8
.align 8
.LC2:
.quad   .LC1


While it is not illegal to have a relocation in a mergeable section, I don't
know of any linker that supports that, so gcc is being over aggressive.

[Bug libstdc++/91997] New: pretty printers: The __node_type type alias _Hashtable is not available

2019-10-04 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91997

Bug ID: 91997
   Summary: pretty printers: The __node_type type alias _Hashtable
is not available
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

At least when compiling with optimizations with

gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)

the debug info doesn't include the type alias

 using __node_type = __detail::_Hash_node<_Value, __hash_cached::value>

Which causes the pretty printer to fail with

-
Traceback (most recent call last):
  File "/home/espindola/scylla/gdb-printers/libstdcxx/v6/printers.py", line
966, in children
data = self.flatten (imap (self.format_one, StdHashtableIterator
(self.hashtable(
  File "/home/espindola/scylla/gdb-printers/libstdcxx/v6/printers.py", line
889, in __init__
self.node_type = find_type(hash.type, '__node_type').pointer()
  File "/home/espindola/scylla/gdb-printers/libstdcxx/v6/printers.py", line 97,
in find_type
field = typ.fields()[0]
IndexError: list index out of range
-

To work around that the pretty printer should reconstruct the type.
The best I was able to come up with was 

-self.node_type = find_type(hash.type, '__node_type').pointer()
+pair_name = hash.type.template_argument(1).name
+traits_type = hash.type.template_argument(9)
+cached = str(traits_type.template_argument(0)).lower()
+node_name = '::std::__detail::_Hash_node<%s,%s>' % (pair_name, cached)
+self.node_type = gdb.lookup_type(node_name).pointer()

[Bug libstdc++/91827] New: const std::experimental::optional is not std::is_nothrow_move_constructible

2019-09-19 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91827

Bug ID: 91827
   Summary: const std::experimental::optional is not
std::is_nothrow_move_constructible
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Only the first static_assert in the following example fails.

#include 
#include 

void foo(const std::experimental::optional timeout) {
static_assert(std::is_nothrow_move_constructible>::value, "foobar");
static_assert(std::is_nothrow_move_constructible>::value, "foobar");
}

[Bug libstdc++/91516] Please also export the base object constructor for __shared_ptr;

2019-08-21 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91516

--- Comment #1 from Rafael Avila de Espindola  ---
Corresponding clang bug:

https://bugs.llvm.org/show_bug.cgi?id=43079

[Bug libstdc++/91516] New: Please also export the base object constructor for __shared_ptr;

2019-08-21 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91516

Bug ID: 91516
   Summary: Please also export the base object constructor for
__shared_ptr;
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Given 

#include 
void foo() {
for (auto& dev : std::filesystem::directory_iterator("slaves")) {
}
}


gcc calls
_ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC1EOS5_,
but clang calls
_ZNSt12__shared_ptrINSt10filesystem7__cxx114_DirELN9__gnu_cxx12_Lock_policyE2EEC2EOS5_.

In the libstdc++.so, only the C1 variant is exported, so a program compiled
with clang and libstdc++ fails to link.

[Bug tree-optimization/91026] New: switch expansion produces a jump table with trivial entries

2019-06-27 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91026

Bug ID: 91026
   Summary: switch expansion produces a jump table with trivial
entries
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: tree-optimization
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 46531
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46531=edit
testcase

In the attached testcase gcc lowers the switch to a jump table where each entry
in the table just sets a value. The two labels in the table are

.L5:
movl$1, %eax
ret

.L2:
xorl%eax, %eax
ret

gcc could have replaced the jump table with a constant table, with L5 replaced
by 1 and L2 by 0.

clang produces:

 <_Z3foo4kind>:
   0:   40 80 c7 ff add$0xff,%dil
   4:   40 80 ff 11 cmp$0x11,%dil
   8:   77 0c   ja 16 <_Z3foo4kind+0x16>
   a:   b8 b9 a7 02 00  mov$0x2a7b9,%eax
   f:   89 f9   mov%edi,%ecx
  11:   d3 e8   shr%cl,%eax
  13:   24 01   and$0x1,%al
  15:   c3  retq
  16:   31 c0   xor%eax,%eax
  18:   c3  retq

[Bug libstdc++/90415] std::is_copy_constructible> is incomplete

2019-05-22 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415

--- Comment #3 from Rafael Avila de Espindola  ---
I see now that the corresponding commit on trunk was
31011b9a94fed33170c009292e82558336d1c4d7 (r261146).

At that revision, the test in this bug passes. There was a more recent
regression on trunk on revision a9b768f8f4fd471e315623b23c4f9e83463bf92e
(r270433).

[Bug libstdc++/90415] std::is_copy_constructible> is incomplete

2019-05-22 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415

--- Comment #2 from Rafael Avila de Espindola  ---
The bug is still present on trunk.

[Bug libstdc++/90415] std::is_copy_constructible> is incomplete

2019-05-22 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90415

Rafael Avila de Espindola  changed:

   What|Removed |Added

 CC||jason at redhat dot com

--- Comment #1 from Rafael Avila de Espindola  ---
This bug was present when gcc 8 branched. It was fixed in the gcc 8 branch, but
I guess it was never fixed on trunk.

On the gcc 8 branch it was fixed by r261463
(d26c6b8b0c6abba9a67b87a1d48f0c3165d021cc).

[Bug c++/90493] New: const variable template specialization is always local

2019-05-15 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90493

Bug ID: 90493
   Summary: const variable template specialization is always local
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Without templates, things are simple:

// this is local
const int bar = 42;

// this is global
extern const int bar = 42;

Just adding a template is also OK

// this is local
template  const int foo = 42;

// this is global
template  extern const int foo = 42;

But with specializations there doesn't seem to be any way of getting a global
symbol

template  extern const int foo = 41;
// this is local
template <> const int foo = 42;

// this is an error:
// error: explicit template specialization cannot have a storage class
template <> extern const int foo = 42;


clang both accept the storage class in the specialization and always produces a
global symbol for template variables. Maybe doing both is a bug in clang, but
we should have some way of defining a global variable template specialization.

[Bug c++/90307] New: -Wuninitialized only at -O1, not at -O2

2019-05-01 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90307

Bug ID: 90307
   Summary: -Wuninitialized only at -O1, not at -O2
   Product: gcc
   Version: 9.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
CC: mjambor at suse dot cz
  Target Milestone: ---

Created attachment 46271
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46271=edit
testcase

Starting at 00ee3e3e4fea0457ef84bada636ae26ffcc95fff -Wall -O1 on the attached
testcase will print

distributed_test.ii:27:24: warning: ‘*((void*)& s +8)’ is used uninitialized in
this function [-Wuninitialized]

But the warning goes away at -O2 an higher. Should this perhaps be in
-Wmaybe-uninitialized?

[Bug libstdc++/90295] New: Please define ~exception_ptr inline

2019-04-30 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90295

Bug ID: 90295
   Summary: Please define ~exception_ptr inline
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: libstdc++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Currently ~exception_ptr is defined out of line. Its definition just calls
_M_release which looks like

 if (_M_exception_object)
{
...
}

Which means that there is nothing to do for an exception_ptr that has been
moved, but the user code has no way of knowing that.

If the ~exception_ptr was defined inline as

~exception_ptr() {
  if (_M_exception_object)
 {
 out_of_line_destructor();
 }
}

Then the compiler would be able to omit the call to out_of_line_destructor when
it knows _M_exception_object is null.

This comes up in seastar (https://github.com/scylladb/seastar/) where we have a
struct (future_state) that holds a exception_ptr and is frequently moved.

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2019-02-15 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

--- Comment #10 from Rafael Avila de Espindola  ---
(In reply to Martin Sebor from comment #9)
> The warning is very simple: it just looks for excessive sizes in calls
> emitted in the optimized IL.  When the call is there (either because it's in
> the source code as is or because it's been synthesized by GCC based on the
> invariants it can infer from the code) it triggers.  It runs after all
> high-level optimizations, including DCE, and assumes that if the statement
> is in the IL it is reachable.  Compiling the test case with
> -fdump-tree-optimized=/dev/stdout shows the GIMPLE the warning works with:
> 
>[local count: 233860936]:
>   # iftmp.6_113 = PHI 
>   memset (iftmp.6_113, 0, 18446744073709551613);
> 
> I think the issue can essentially be reduced to the following:
> 
> $ cat z.c && gcc -O2 -S -fdump-tree-optimized=/dev/stdout z.c
> void f (char *d, const char *s)
> {
>   if (__builtin_strstr (s, "ABC"))
> {
>   __SIZE_TYPE__ n = __builtin_strlen (s) - 3;
> 
>   if (n > __builtin_strlen (s))   // cannot be true
> __builtin_memset (d, 0, n - __builtin_strlen (s));
>   }
> }
> 
> ;; Function f (f, funcdef_no=0, decl_uid=1907, cgraph_uid=1, symbol_order=0)
> 
> Removing basic block 6
> Removing basic block 7
> f (char * d, const char * s)
> {
>   char * _1;
>   long unsigned int _2;
> 
>[local count: 1073741824]:
>   _1 = __builtin_strstr (s_5(D), "ABC");
>   if (_1 != 0B)
> goto ; [70.00%]
>   else
> goto ; [30.00%]
> 
>[local count: 751619278]:
>   _2 = __builtin_strlen (s_5(D));
>   if (_2 <= 2)
> goto ; [33.00%]
>   else
> goto ; [67.00%]
> 
>[local count: 248034361]:
>   __builtin_memset (d_6(D), 0, 18446744073709551613); [tail call]
> 
>[local count: 1073741824]:
>   return;
> 
> }
> 
> 
> z.c: In function ‘f’:
> z.c:8:9: warning: ‘__builtin_memset’ specified size 18446744073709551613
> exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
> 8 | __builtin_memset (d, 0, n - __builtin_strlen (s));
>   | ^
> 
> GCC doesn't have the smarts to figure out that when s contains a substring
> of length 3 then strlen(s) must be at least 3.  As a result, it doesn't
> eliminate the memset call in the function and the warning triggers.  Suppose
> we teach GCC how to figure this out from the strstr call (which might be a
> useful optimization) and then someone comes along with a test case that uses
> strspn() instead of strstr().  We can also teach GCC about strstr() but then
> someone else might use regcomp() or some user-defined function that we won't
> have access to.  At some point we'll have to call it good enough.

Absolutely, this is the halting problem after all.

The question is not to give up or not, it is whether to warn when we do. If we
do, we get potential warnings every time gcc gives up on solving the halting
problem.

The uninitialized variable warning was at least split in two, one for the cases
where gcc is not sure.

Maybe we should have a general flag that disables all warnings where gcc cannot
prove that there is a path from a function entry to the broken statement?

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2019-02-14 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

Rafael Avila de Espindola  changed:

   What|Removed |Added

 Status|RESOLVED|UNCONFIRMED
 Resolution|DUPLICATE   |---

--- Comment #8 from Rafael Avila de Espindola  ---
The previous testcase passes on trunk, but the original doesn't.

The original has a "if (boost::algorithm::ends_with(name, "%2A"))" instead of a
direct "if(name.size() > 3)".

The reduced testcase uses a find to avoid boost::algorithm and keep the
testcase size sane.

In the end it seems that gcc is doing an heroic effort to understand the code
and issue an warning if it can't prove that the bogus memset is not reached. I
don't think that is workable in practice (for sure not in theory): There is
always something that is obvious to the developer but not to gcc.

IMHO gcc should be issuing warnings only when it shows that a statement can be
reached, not when it fails to show the statement cannot be reached.

[Bug tree-optimization/88443] [meta-bug] bogus/missing -Wstringop-overflow warnings

2019-02-14 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88443
Bug 88443 depends on bug 89337, which changed state.

Bug 89337 Summary: Bogus "exceeds maximum object size" on unreachable code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

   What|Removed |Added

 Status|RESOLVED|UNCONFIRMED
 Resolution|DUPLICATE   |---

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2019-02-14 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

Rafael Avila de Espindola  changed:

   What|Removed |Added

  Attachment #45710|0   |1
is obsolete||

--- Comment #7 from Rafael Avila de Espindola  ---
Created attachment 45726
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45726=edit
testcase that fails on trunk

[Bug tree-optimization/88443] [meta-bug] bogus/missing -Wstringop-overflow warnings

2019-02-13 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88443
Bug 88443 depends on bug 89337, which changed state.

Bug 89337 Summary: Bogus "exceeds maximum object size" on unreachable code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

   What|Removed |Added

 Status|RESOLVED|UNCONFIRMED
 Resolution|INVALID |---

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2019-02-13 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

Rafael Avila de Espindola  changed:

   What|Removed |Added

 Status|RESOLVED|UNCONFIRMED
 Resolution|INVALID |---

--- Comment #5 from Rafael Avila de Espindola  ---
Reopening with an expanded testcase.

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2019-02-13 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

Rafael Avila de Espindola  changed:

   What|Removed |Added

  Attachment #45704|0   |1
is obsolete||

--- Comment #4 from Rafael Avila de Espindola  ---
Created attachment 45710
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45710=edit
testcase where the string size should still be visible to the compiler

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2019-02-13 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

--- Comment #3 from Rafael Avila de Espindola  ---

> GCC can't see that drop3() cannot be called with name.size() < 3, and in
> resize, the condition (n > size()) can only be true only when name.size() <
> 3 so n - size() is unavoidably too large.

That is why gcc should not warn about it :-)

The original testcase had

  if (boost::algorithm::ends_with(name, "%2A")) {
  name.resize(name.length() - 3);

I think it is fair to say that:

* The code is valid, if something ends with "%2A" the size is >= 3.
* GCC should not be required to figure out the above implication.
* The developer should not be required to add a __builtin_unreachable() to
avoid a warning in the above code.

I will re reduce the testcase keeping the ends_with and attach.

[Bug middle-end/89337] Bogus "exceeds maximum object size" on unreachable code

2019-02-13 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

--- Comment #1 from Rafael Avila de Espindola  ---
The original testcase is from https://github.com/scylladb/seastar/issues/598

[Bug middle-end/89337] New: Bogus "exceeds maximum object size" on unreachable code

2019-02-13 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89337

Bug ID: 89337
   Summary: Bogus "exceeds maximum object size" on unreachable
code
   Product: gcc
   Version: 9.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: middle-end
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 45704
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45704=edit
testcase

In the attached testcase the function drop3 has the precondition that the
provided string has a size of at least 3.

Given that, the body of the resize function is dead, but gcc doesn't realize it
and warns that we are passing -3 to memcpy.

[Bug middle-end/88897] Bogus maybe-uninitialized warning on class field

2019-01-21 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88897

Rafael Avila de Espindola  changed:

   What|Removed |Added

 CC||rguenther at suse dot de

--- Comment #5 from Rafael Avila de Espindola  ---
This is a regression introduced by 4d2b9d1e3c794a05c00708035519290e920768e8.

[Bug middle-end/88897] Bogus maybe-uninitialized warning on class field

2019-01-20 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88897

Rafael Avila de Espindola  changed:

   What|Removed |Added

  Attachment #45452|0   |1
is obsolete||
  Attachment #45453|0   |1
is obsolete||

--- Comment #4 from Rafael Avila de Espindola  ---
Created attachment 45475
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45475=edit
testcase

Now that I know what to look for I reduced the testcase again trying to make
sure to not introduce uninitialized uses in the process.

As far as I can tell the warning in the attached testcase is wrong.

First an optional is constructed, but _M_engaged is false since it doesn't hold
a value.

It is then moved, but move constructor checks __other._M_engaged, so the
temporary_buffer move constructor should never be called.

[Bug middle-end/88897] Bogus maybe-uninitialized warning on class field

2019-01-18 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88897

--- Comment #3 from Rafael Avila de Espindola  ---
(In reply to Andrew Pinski from comment #2)
> Some of the time, the uninitialized is due to using the object after the
> lifetime of the object has gone out of scope.  I have not checked if that is
> the case here but I would not be suprised.

Thanks! This may very well be the case.

This reduces further to

struct deleter {
  deleter(deleter &) {}
  deleter(deleter &) = default;
};
struct temporary_buffer {
  temporary_buffer(temporary_buffer &) = default;
  char *_buffer = nullptr;
  deleter _deleter;
};
void foo123(temporary_buffer);
struct future_state {
  union any {
any() {}
temporary_buffer value;
  } _u;
  void forward_to() { foo123(_u.value); }
};
void then3() {
  future_state _local_state;
  _local_state.forward_to();
}


which has a real uninitialized use. I am looking again at the unreduced test
case to see if that was the original issue.

More context in the warning message would have been awesome! :-)

[Bug c++/88897] Bogus maybe-uninitialized warning on class field

2019-01-17 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88897

--- Comment #1 from Rafael Avila de Espindola  ---
Created attachment 45453
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45453=edit
reduced the test a bit more

It now compiles with older gcc too.

The warning is there in gcc 7, but not gcc 6.

[Bug c++/88897] New: Bogus maybe-uninitialized warning on class field

2019-01-17 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88897

Bug ID: 88897
   Summary: Bogus maybe-uninitialized warning on class field
   Product: gcc
   Version: 8.2.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Created attachment 45452
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45452=edit
testcase

Compiling the attached testcase with "g++ -c -O1 -Wall test.ii" produces

test.ii:631:7: warning: ‘.seastar::temporary_buffer::_buffer’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
 class temporary_buffer {

But in the reduced testcase the only constructor of temporary_buffer is out of
line, so the compiler has no way of knowing if it is initialized or not.

The warning goes away with -O2 or with very minor changes to the source code.

[Bug sanitizer/88684] Please make SANITIZER_NON_UNIQUE_TYPEINFO a runtime flag (or always true)

2019-01-16 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88684

--- Comment #11 from Rafael Avila de Espindola  ---
(In reply to Martin Liška from comment #10)
> > That said, I'm willing to ack it for GCC9 even then if upstream comes up
> > with something or if they don't care, eventually as a GCC only tweak.
> 
> Works for me. Note that so far there has been no reply to my patch.

You might want to CC:
 Filipe Cabecinhas 

[Bug sanitizer/88684] New: Please make SANITIZER_NON_UNIQUE_TYPEINFO a runtime flag (or always true)

2019-01-03 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88684

Bug ID: 88684
   Summary: Please make SANITIZER_NON_UNIQUE_TYPEINFO a runtime
flag (or always true)
   Product: gcc
   Version: unknown
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: sanitizer
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
jakub at gcc dot gnu.org, kcc at gcc dot gnu.org, marxin at 
gcc dot gnu.org
  Target Milestone: ---

Even on ABIs that normally unique typeinfo names, it is easy to end up in
situations where that fails.

Consider a shared library implemented with

lib.hh:
struct foo {
virtual ~foo(){}
};
struct bar : public foo {
virtual void zed();
};

lib.cc:
#include "lib.hh"
void bar::zed() {}

and being used by the program (could be another library):

test.cc:
#include "lib.hh"
int main(int argc, char** argv) { bar t; }

if the program is compiled with -fvisibility=hidden, it will have a hidden
_ZTI3foo which isDerivedFromAtOffset will think doesn't match the _ZTI3foo in
the library.

The above test is a reduction of

#include 
int main(int argc, char **argv) {
return 0;
}

compiled with -fvisibility=hidden, which complains that

/usr/include/boost/test/unit_test_log.hpp:112:23: runtime error: member call on
address 0x06583060 which does not point to an object of type
'test_observer'

[Bug c++/88509] Missing optimization of tls initialization

2018-12-15 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88509

--- Comment #3 from Rafael Avila de Espindola  ---
(In reply to Jakub Jelinek from comment #2)
> I must say I don't understand your suggestion.  bar is not a pointer and its
> address is non-NULL no matter whether it has been already initialized or not.
> What kind of assembly you'd like to see for f?

Bar is not a pointer, but a guard variable for bar can be. It is initially null
and is set to bar's address once bar is initialized.

I think the assembly for f could be:

_Z1fv:
movq%fs:__guard_for_bar@tpoff, %rax 
testq   %rax, %rax
je  .L15
ret
.L15:
pushq   %rbx
movq%fs:0, %rbx
leaq_ZL3bar@tpoff(%rbx), %rdi
call_ZN3fooC1Ev
leaq_ZL3bar@tpoff(%rbx), %rax
movq%rax, %fs:__guard_for_bar@tpoff
popq%rbx
ret

The .L15 label could even be moved to a cold section.

[Bug c++/88509] New: Missing optimization of tls initialization

2018-12-14 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88509

Bug ID: 88509
   Summary: Missing optimization of tls initialization
   Product: gcc
   Version: 8.2.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

Given

struct foo {
  foo();
};
static thread_local foo bar;
foo *f() { return  }
foo *g() {
  static thread_local foo *bar_ptr;
  if (bar_ptr == nullptr) {
[&]() { bar_ptr =  }();
  }
  return bar_ptr;
}

GCC has to make sure bar is only initialized once. For the function f it
produces

pushq   %rbx
cmpb$0, %fs:__tls_guard@tpoff
movq%fs:0, %rbx
je  .L6
leaq_ZL3bar@tpoff(%rbx), %rax
popq%rbx
ret
.L6:
   

for g, the common code path is somewhat simpler:

movq%fs:_ZZ1gvE7bar_ptr@tpoff, %rax
testq   %rax, %rax
je  .L15
ret
.L15:
   


The optimization is to use the a pointer to the object as a guard instead of
using a boolean. As far as I can tell this can be applied to any static tls
variable (where the ABI is not a problem).

[Bug c++/88232] New: Please implement -Winfinite-recursion

2018-11-27 Thread rafael at espindo dot la
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88232

Bug ID: 88232
   Summary: Please implement -Winfinite-recursion
   Product: gcc
   Version: 8.2.1
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: c++
  Assignee: unassigned at gcc dot gnu.org
  Reporter: rafael at espindo dot la
  Target Milestone: ---

This may sound like a silly warning, but it actually very useful in finding
missing member functions in CRTP. Given the testcase

template  struct C {
  void foo() { static_cast(this)->foo(); }
};
struct D : C {
// this is missing:
// void foo() {}
};
void f(D *d) { d->foo(); }

gcc is silent, but clang prints:

test.cpp:2:14: warning: all paths through this function will call itself
[-Winfinite-recursion]
  void foo() { static_cast(this)->foo(); }