Bug 90947 - [9 Regression] Simple lookup table of array of strings is miscompiled
Summary: [9 Regression] Simple lookup table of array of strings is miscompiled
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 9.1.0
: P2 normal
Target Milestone: 9.3
Assignee: Martin Sebor
URL:
Keywords: patch, wrong-code
: 93630 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-06-20 09:35 UTC by Hans-Kristian Arntzen
Modified: 2020-02-08 09:56 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work: 10.0
Known to fail:
Last reconfirmed: 2019-06-20 00:00:00


Attachments
gcc10-pr90947.patch (2.00 KB, patch)
2019-10-22 08:32 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hans-Kristian Arntzen 2019-06-20 09:35:26 UTC
On GCC 9.1.0, this snippet is miscompiled:

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>

static const char *vector_swizzle(int vecsize, int index)
{
	static const char *swizzle[4][4] = {
		{ ".x", ".y", ".z", ".w" },
		{ ".xy", ".yz", ".zw", nullptr },
		{ ".xyz", ".yzw", nullptr, nullptr },
		{ "", nullptr, nullptr, nullptr },
	};

	assert(vecsize >= 1 && vecsize <= 4);
	assert(index >= 0 && index < 4);
	assert(swizzle[vecsize - 1][index]);

	return swizzle[vecsize - 1][index];
}

int main(int argc, char **argv)
{
	puts(vector_swizzle(atoi(argv[1]), atoi(argv[2])));
}

Compiled with "g++ -o test test.cpp", and ran with ./test 4 0, and it asserts. The last array entry of swizzle[3] are all nullptr for some reason. No other compiler behaves like this.
Comment 1 Jonathan Wakely 2019-06-20 10:29:24 UTC
This started with r270155 so is a dup of PR 90938

*** This bug has been marked as a duplicate of bug 90938 ***
Comment 2 Martin Sebor 2019-06-20 15:24:05 UTC
This is actually a different bug in the same commit.  A simpler test case is this:

  void f (void)
  { 
    const char* a[2][2] = {
      { "0", "1" },
      { "", 0 }
   };

    if (!a[1][0])   // should be folded to false with -O
      __builtin_abort ();
  }

There is code in place to handle this kind of initialization:

      /* Pointers initialized to strings must be treated as non-zero
	 even if the string is empty.  */
      tree init_type = TREE_TYPE (elt_init);
      if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type))
	  || !initializer_zerop (elt_init))
	last_nonzero = index;

This was added in r270177 to fix pr89980.  Unfortunately, the handling is incomplete and the test insufficient to expose it.
Comment 3 Martin Sebor 2019-06-20 15:35:14 UTC
The root cause of the problem is that initializer_zerop (elt_init) doesn't differentiate between the empty string and a null pointer in the same initializer list and returns true for the CONSTRUCTOR ({"", 0}).
Comment 4 Martin Sebor 2019-06-22 00:09:34 UTC
Patch: https://gcc.gnu.org/ml/gcc-patches/2019-06/msg01365.html
Comment 5 Martin Sebor 2019-08-01 23:46:07 UTC
Author: msebor
Date: Thu Aug  1 23:45:36 2019
New Revision: 273989

URL: https://gcc.gnu.org/viewcvs?rev=273989&root=gcc&view=rev
Log:
PR c++/90947 - Simple lookup table of array of strings is miscompiled

gcc/cp/ChangeLog:

	PR c++/90947
	* decl.c (reshape_init_array_1): Avoid truncating initializer
	lists containing string literals.

gcc/testsuite/ChangeLog:

	PR c++/90947
	* c-c++-common/array-1.c: New test.
	* g++.dg/abi/mangle73.C: New test.
	* g++.dg/cpp2a/nontype-class23.C: New test.
	* g++.dg/init/array53.C: New test.

gcc/ChangeLog:

	PR c++/90947
	* tree.c (type_initializer_zero_p): Define.
	* tree.h (type_initializer_zero_p): New function.


Added:
    trunk/gcc/testsuite/c-c++-common/array-1.c
    trunk/gcc/testsuite/g++.dg/abi/mangle73.C
    trunk/gcc/testsuite/g++.dg/cpp2a/nontype-class23.C
    trunk/gcc/testsuite/g++.dg/init/array53.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree.c
    trunk/gcc/tree.h
Comment 6 Martin Sebor 2019-08-01 23:47:01 UTC
Fixed for GCC 10 in r273989.
Comment 7 Richard Biener 2019-08-02 10:01:20 UTC
Please consider backporting for GCC 9.2.
Comment 8 Jakub Jelinek 2019-08-02 10:23:10 UTC
The cp/decl.c change looks weird, it is more readable to use one if with || instead of if (cond1) something; else if (cond2) something; where something is the same in between.
Comment 9 Jakub Jelinek 2019-08-12 08:53:55 UTC
GCC 9.2 has been released.
Comment 10 Martin Sebor 2019-08-14 20:37:18 UTC
Author: msebor
Date: Wed Aug 14 20:36:46 2019
New Revision: 274494

URL: https://gcc.gnu.org/viewcvs?rev=274494&root=gcc&view=rev
Log:
	Backport from mainline

	2019-08-01  Martin Sebor  <msebor@redhat.com>

	PR c++/90947
	* tree.c (type_initializer_zero_p): Define.
	* tree.h (type_initializer_zero_p): New function.

	2019-08-05  Martin Sebor  <msebor@redhat.com>

	* doc/extend.texi (Common Variable Attributes): Document alias
	attribute.

	2019-08-01  Martin Sebor  <msebor@redhat.com>

	PR c++/90947
	* decl.c (reshape_init_array_1): Avoid truncating initializer
	lists containing string literals.

	2019-08-14  Martin Sebor  <msebor@redhat.com>

	PR tree-optimization/91294
	* gcc.dg/strlenopt-44.c: Adjust tested result.
	* gcc.dg/strlenopt-70.c: Avoid exercising unimplemnted optimization.
	* gcc.dg/strlenopt-73.c: New test.
	* gcc.dg/strlenopt-74.c: New test.
	* gcc.dg/strlenopt-75.c: New test.
	* gcc.dg/strlenopt-76.c: New test.
	* gcc.dg/strlenopt-77.c: New test.

Added:
    branches/gcc-9-branch/gcc/testsuite/c-c++-common/array-1.c
    branches/gcc-9-branch/gcc/testsuite/g++.dg/abi/mangle73.C
    branches/gcc-9-branch/gcc/testsuite/g++.dg/cpp2a/nontype-class23.C
    branches/gcc-9-branch/gcc/testsuite/g++.dg/init/array53.C
Modified:
    branches/gcc-9-branch/gcc/ChangeLog
    branches/gcc-9-branch/gcc/cp/ChangeLog
    branches/gcc-9-branch/gcc/cp/decl.c
    branches/gcc-9-branch/gcc/doc/extend.texi
    branches/gcc-9-branch/gcc/testsuite/ChangeLog
    branches/gcc-9-branch/gcc/tree.c
    branches/gcc-9-branch/gcc/tree.h
Comment 11 Jason Merrill 2019-08-15 04:39:31 UTC
Fixed for 9.3.
Comment 12 Dimitar Yordanov 2019-10-22 07:46:36 UTC
Hi,

after the fix we hit a regression with:

$ cat > main.cpp << EOF

#include <atomic>

static std::atomic<int> a[1] { {1} };

int main(void)
{
  if (!a[0].load())
    __builtin_abort ();
  return 0;
}

EOF

$ g++ main.cpp && ./a.out

Best wishes
Dimitar
Comment 13 Jakub Jelinek 2019-10-22 08:32:13 UTC
Created attachment 47082 [details]
gcc10-pr90947.patch

Here is an untested patch that fixes it for -std=c++17/-std=c++2a, though not for C++11/14, next_initialized_field (TYPE_FIELDS (type)) for std::atomic<int>
is NULL in those cases, as that class contains no non-static data members directly, just has __atomic_base as a base class.  The initializer has init list
type in both cases, dunno if we don't need to treat those specially or how else to fix C++11/14.
Comment 14 Jakub Jelinek 2019-10-31 07:11:28 UTC
Author: jakub
Date: Thu Oct 31 07:10:57 2019
New Revision: 277656

URL: https://gcc.gnu.org/viewcvs?rev=277656&root=gcc&view=rev
Log:
	PR c++/90947
	* tree.h (type_initializer_zero_p): Remove.
	* tree.c (type_initializer_zero_p): Remove.
cp/
	* cp-tree.h (type_initializer_zero_p): Declare.
	* decl.c (reshape_init_array_1): Formatting fix.
	* tree.c (type_initializer_zero_p): New function.  Moved from
	../tree.c, use next_initializable_field, formatting fix.  Return
	false for TYPE_NON_AGGREGATE_CLASS types.

Added:
    trunk/gcc/testsuite/g++.dg/init/array54.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/tree.c
    trunk/gcc/tree.c
    trunk/gcc/tree.h
Comment 15 Jakub Jelinek 2019-11-08 18:57:13 UTC
Author: jakub
Date: Fri Nov  8 18:56:42 2019
New Revision: 277985

URL: https://gcc.gnu.org/viewcvs?rev=277985&root=gcc&view=rev
Log:
	Backported from mainline
	2019-10-31  Jakub Jelinek  <jakub@redhat.com>

	PR c++/90947
	* tree.h (type_initializer_zero_p): Remove.
	* tree.c (type_initializer_zero_p): Remove.

	* cp-tree.h (type_initializer_zero_p): Declare.
	* decl.c (reshape_init_array_1): Formatting fix.
	* tree.c (type_initializer_zero_p): New function.  Moved from
	../tree.c, use next_initializable_field, formatting fix.  Return
	false for TYPE_NON_AGGREGATE_CLASS types.

Added:
    branches/gcc-9-branch/gcc/testsuite/g++.dg/init/array54.C
Modified:
    branches/gcc-9-branch/gcc/ChangeLog
    branches/gcc-9-branch/gcc/cp/ChangeLog
    branches/gcc-9-branch/gcc/cp/cp-tree.h
    branches/gcc-9-branch/gcc/cp/decl.c
    branches/gcc-9-branch/gcc/cp/tree.c
    branches/gcc-9-branch/gcc/tree.c
    branches/gcc-9-branch/gcc/tree.h
Comment 16 Jakub Jelinek 2020-02-08 09:56:18 UTC
*** Bug 93630 has been marked as a duplicate of this bug. ***