[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(); }