Bug 96934 - [9 Regression] Copy initialization of struct involving aggregate array initialization miscompiles in GCC 9
Summary: [9 Regression] Copy initialization of struct involving aggregate array initia...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 9.3.0
: P3 normal
Target Milestone: 9.4
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2020-09-04 09:56 UTC by Gustaw Smolarczyk
Modified: 2021-08-17 05:21 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work: 10.1.0, 8.1.0, 8.5.0, 9.4.0
Known to fail: 9.3.0
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gustaw Smolarczyk 2020-09-04 09:56:59 UTC
GCC 9 (but not <= 8 and not >= 10, including trunk) miscompiles the following piece of code. I have tested GCC 9.1 (which produces even worse code, as it assumes the const char* has only a single character) and GCC 9.2, 9.3 (explained below). I was not able to test the current GCC 9 branch.

Test case: https://godbolt.org/z/jPq4h7

The Code struct holds an array of chars that is meant to be always null-terminated. A simple constructor is provided to simulate how the array should be initialized (this is a reduced real world scenario). It uses aggregate initialization in order to store the "12" string.

There are two problems, probably originating from the same underlying issue:
1. Bogus -Wstringop-overflow warning saying the _buffer is unterminated, while it most certainly is (as you can see in the assembly).
2. std::strcmp call miscompiled as if _buffer == "1" (and not "12").

Switching the TEST define on line 17 into T1, T2, T3, T4 doesn't change the outcome. However, T5 and T6 fix the issue. What seems to be the difference is the copy-initialization [1] being involved in T1 and T2 (and T3, T4 "inheriting" the buggy state). T5 doesn't do copy-initialization, and T6 just copies it.

[1] https://en.cppreference.com/w/cpp/language/copy_initialization
Comment 1 Gustaw Smolarczyk 2020-09-04 10:56:24 UTC
It seems that part of this issue was already reported in another bug report (though the report is about flexible array members, the comment does not reference them):

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91490#c6

However, I don't see any mention of the miscompilation in the thread. Possibly the issue didn't surface in comment 6 as the tested string has only a single character (+ the terminating null character). Or strcmp is needed in order to trigger it.
Comment 2 Andrew Pinski 2021-08-17 05:15:46 UTC
Testcase:
#include <cstring>

struct Code
{
    constexpr Code(int) noexcept : _buffer{'1', '2', '\0'} {}

    char _buffer[3];
};

const Code T1 = {1};
const Code T2 = Code{1};
const Code T3 = T1;
const Code T4 = T2;
const Code T5{1};
const Code T6 = T5;

#define TEST T1

int
foo(const char* x)
{
  return std::strcmp(TEST._buffer, x);
}

size_t
bar()
{
  return std::strlen(TEST._buffer);
}
Comment 3 Andrew Pinski 2021-08-17 05:21:30 UTC
Fixed in GCC 9.4.0, most likely by the patch which fixed PR 91914.