Bug 86153 - [8 regression] test case g++.dg/pr83239.C fails starting with r261585
Summary: [8 regression] test case g++.dg/pr83239.C fails starting with r261585
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: testsuite (show other bugs)
Version: 9.0
: P2 normal
Target Milestone: 8.3
Assignee: Alexandre Oliva
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-14 14:41 UTC by seurer
Modified: 2019-04-17 02:31 UTC (History)
11 users (show)

See Also:
Host: powerpc64*-*-*
Target: powerpc64*-*-* aarch64 alpha*-*-* x86
Build: powerpc64*-*-*
Known to work:
Known to fail:
Last reconfirmed: 2018-06-15 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description seurer 2018-06-14 14:41:08 UTC
make -k check-gcc RUNTESTFLAGS=dg.exp=g++.dg/pr83239.C
. . .
# of expected passes		7
# of unexpected failures	2
FAIL: g++.dg/pr83239.C  -std=gnu++11  scan-tree-dump-not optimized "_ZNSt6vectorIiSaIiEE17_M_default_appendEm"
FAIL: g++.dg/pr83239.C  -std=gnu++14  scan-tree-dump-not optimized "_ZNSt6vectorIiSaIiEE17_M_default_appendEm"


The revision changed the trees a bit and those checks now fail.  If this is working as expected the test cases need to be updated.


r261585 | redi | 2018-06-14 04:26:51 -0500 (Thu, 14 Jun 2018) | 14 lines

PR libstdc++/83982 fix exception-safety guarantee of std::vector::resize

Construct new elements before moving existing ones, so that if a default
constructor throws, the existing elements are not left in a moved-from
state.

2018-06-14  Daniel Trebbien <dtrebbien@gmail.com>
	    Jonathan Wakely  <jwakely@redhat.com>

	PR libstdc++/83982
	* include/bits/vector.tcc (vector::_M_default_append(size_type)):
	Default-construct new elements before moving existing ones.
	* testsuite/23_containers/vector/capacity/resize/strong_guarantee.cc:
	New.
Comment 1 Jonathan Wakely 2018-06-14 14:47:06 UTC
It seems unfortunate that _M_default_append is no longer inlined, but the function was incorrect and had to be changed.

I don't know enough about the failing test to know what can or should be done to fix it.
Comment 2 Richard Biener 2018-06-15 08:04:19 UTC
Confirmed.

The testcase says the absence of the warning depends on the inlining but it seems it doesn't (no extra diagnostic reported).  So maybe just remove this dump-scanning.
Comment 3 Christophe Lyon 2018-06-15 12:10:35 UTC
Seen on aarch64 too.
Comment 4 Steve Ellcey 2018-06-25 18:37:08 UTC
If we wanted to restore the previous inlining behavior, adding
the option '--param early-inlining-insns=30' seems to fix the
failure for me on aarch64.
Comment 5 Andrey Guskov 2018-07-05 11:12:38 UTC
Confirmed on Haswell and Silvermont.
Comment 6 Richard Biener 2018-07-16 11:05:09 UTC
Same on the 8 branch now, didn't yet check the GCC 7 branch (patch was also backported there).
Comment 7 Jakub Jelinek 2018-07-26 11:21:12 UTC
GCC 8.2 has been released.
Comment 8 Uroš Bizjak 2018-08-01 12:15:15 UTC
Patch at [1].

[1] https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00034.html
Comment 9 uros 2018-08-04 10:02:25 UTC
Author: uros
Date: Sat Aug  4 10:01:54 2018
New Revision: 263306

URL: https://gcc.gnu.org/viewcvs?rev=263306&root=gcc&view=rev
Log:
	PR testsuite/86153
	* g++.dg/pr83239.C (dg-options): Add -finline-limit=500.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/pr83239.C
Comment 10 uros 2018-08-04 10:09:54 UTC
Author: uros
Date: Sat Aug  4 10:09:21 2018
New Revision: 263307

URL: https://gcc.gnu.org/viewcvs?rev=263307&root=gcc&view=rev
Log:
	PR testsuite/86153
	* g++.dg/pr83239.C (dg-options): Add -finline-limit=500.


Modified:
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
    branches/gcc-8-branch/gcc/testsuite/g++.dg/pr83239.C
Comment 11 seurer 2018-08-06 15:42:58 UTC
<previous run
>this run
> FAIL: g++.dg/pr83239.C  -std=gnu++98 (test for excess errors)
< FAIL: g++.dg/pr83239.C  -std=gnu++11  scan-tree-dump-not optimized "_ZNSt6vectorIiSaIiEE17_M_default_appendEm"
< FAIL: g++.dg/pr83239.C  -std=gnu++14  scan-tree-dump-not optimized "_ZNSt6vectorIiSaIiEE17_M_default_appendEm"

the patch fixed the original failures but now there's a new one

spawn -ignore SIGHUP /home/seurer/gcc/build/gcc-trunk/gcc/testsuite/g++3/../../xg++ -B/home/seurer/gcc/build/gcc-trunk/gcc/testsuite/g++3/../../ /home/seurer/gcc/gcc-trunk/gcc/testsuite/g++.dg/pr83239.C -fno-diagnostics-show-caret -fdiagnostics-color=never -nostdinc++ -I/home/seurer/gcc/build/gcc-trunk/powerpc64le-unknown-linux-gnu/libstdc++-v3/include/powerpc64le-unknown-linux-gnu -I/home/seurer/gcc/build/gcc-trunk/powerpc64le-unknown-linux-gnu/libstdc++-v3/include -I/home/seurer/gcc/gcc-trunk/libstdc++-v3/libsupc++ -I/home/seurer/gcc/gcc-trunk/libstdc++-v3/include/backward -I/home/seurer/gcc/gcc-trunk/libstdc++-v3/testsuite/util -fmessage-length=0 -std=gnu++98 -O3 -finline-limit=500 -Wall -fdump-tree-optimized -S -o pr83239.s
In function 'void test_loop() [with T = int]':
cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551608 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
In function 'void test_if(std::vector<T>&, int) [with T = long int]':
cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551600 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
FAIL: g++.dg/pr83239.C  -std=gnu++98 (test for excess errors)
Excess errors:
cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551608 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551600 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]

PASS: g++.dg/pr83239.C  -std=gnu++98  scan-tree-dump-not optimized "_ZNSt6vectorIiSaIiEE17_M_default_appendEm"
PASS: g++.dg/pr83239.C  -std=gnu++98  scan-tree-dump-not optimized "_ZNSt6vectorIPvSaIS0_EE17_M_default_appendEm"
Comment 13 Alexandre Oliva 2018-12-19 06:52:13 UTC
Author: aoliva
Date: Wed Dec 19 06:51:41 2018
New Revision: 267252

URL: https://gcc.gnu.org/viewcvs?rev=267252&root=gcc&view=rev
Log:
[PR86153] simplify more overflow tests in VRP

PR 86153 was originally filed when changes to the C++11's
implementation of vector resize(size_type) limited inlining that were
required for testsuite/g++.dg/pr83239.C to verify that we did not
issue an undesired warning.

That was worked by increasing the limit for inlining, but that in turn
caused the C++98 implementation of vector resize, that is
significantly different, to also be fully inlined, and that happened
to issue the very warnings the test was meant to verify we did NOT
issue.

The reason we issued the warnings was that we failed to optimize out
some parts of _M_fill_insert, used by the C++98 version of vector
resize, although the call of _M_fill_insert was guarded by a test that
could never pass: test testcase only calls resize when the vector size
is >= 3, to decrement the size by two.  The limitation we hit in VRP
was that the compared values could pass as an overflow test, if the
vector size was 0 or 1 (we knew it wasn't), but even with dynamic
ranges we failed to decide that the test result could be determined at
compile time, even though after the test we introduced ASSERT_EXPRs
that required a condition known to be false from earlier ones.

I pondered turning ASSERT_EXPRs that show impossible conditions into
traps, to enable subsequent instructions to be optimized, but I ended
up finding an earlier spot in which an overflow test that would have
introduced the impossible ASSERT_EXPR can have its result deduced from
earlier known ranges and resolved to the other path.

Although such overflow tests could be uniformly simplified to compares
against a constant, the original code would only perform such
simplifications when the test could be resolved to an equality test
against zero.  I've thus avoided introducing compares against other
constants, and instead added code that will only simplify overflow
tests that weren't simplified before when the condition can be
evaluated at compile time.


for  gcc/ChangeLog

	PR testsuite/86153
	PR middle-end/83239
	* vr-values.c
	(vr_values::vrp_evaluate_conditional_warnv_with_ops): Extend
	simplification of overflow tests to cover cases in which we
	can determine the result of the comparison.

for  gcc/testsuite/ChangeLog

	PR testsuite/86153
	PR middle-end/83239
	* gcc.dg/vrp-overflow-1.c: New.

Added:
    trunk/gcc/testsuite/gcc.dg/vrp-overflow-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/vr-values.c
Comment 14 Alexandre Oliva 2018-12-19 07:02:17 UTC
Fixed in the trunk
Comment 15 Martin Sebor 2019-02-14 03:52:05 UTC
*** Bug 89337 has been marked as a duplicate of this bug. ***
Comment 16 bin cheng 2019-04-17 02:31:39 UTC
Should this be backported to GCC8?