[Bug c/110878] New: -Wstringop-overread incorrectly warns about arguments to functions with static array parameter declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110878
Bug ID: 110878
Summary: -Wstringop-overread incorrectly warns about arguments
to functions with static array parameter declarations
Product: gcc
Version: 13.2.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: campbell+gcc-bugzilla at mumble dot net
Target Milestone: ---
Isolated from code passing a pointer into an array and the length of the array
as separate arguments, where each function has the minimum length of the array
encoded in its parameter declaration, and uses runtime conditionals to
guarantee the minimum is met:
// bar(p, n) may access p[0], p[1], ..., p[n-1], and requires n >= 128
void bar(unsigned char[static 128], unsigned);
// foo(p, n) may access p[0], p[1], ..., p[n-1], and requires n >= 16
void
foo(unsigned char p[static 16], unsigned n)
{
if (n % 128)
n -= n % 128;
if (n)
bar(p, n);
}
: In function 'foo':
:12:17: error: 'bar' accessing 128 bytes in a region of size 16
[-Werror=stringop-overflow=]
12 | bar(p, n);
| ^
:12:17: note: referencing argument 1 of type 'unsigned char[128]'
:2:6: note: in a call to function 'bar'
2 | void bar(unsigned char[static 128], unsigned n);
| ^~~
cc1: all warnings being treated as errors
Compiler returned: 1
Reproduced in GCC 10.5, 11.4, and 12.3. Not reproduced in any earlier versions
of GCC.
Using `if (n >= 128)' doesn't change anything, presumably because GCC doesn't
know the connection between p and n.
[Bug c/110878] -Wstringop-overflow incorrectly warns about arguments to functions with static array parameter declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110878 --- Comment #3 from Taylor R Campbell --- (In reply to Andrew Pinski from comment #1) > There is another bug report dealing with this. But IIRC this is an expected > warning as foo is being passed an array which is size 16 but then passed to > bar as size 128 which would be undefined. There is nothing undefined here. The caller's requirement as noted in the comment (which is not formally expressible in C, as far as I know, but is obviously extremely widespread practice) is that for foo(p, n) or bar(p, n), p must point to the first element of an array of at least n elements. foo additionally imposes the requirement that p have at least 16 elements. bar additionally imposes the requirement that p have at least 128 elements. When the caller meets foo's contract, foo meets bar's contract. So there is nothing undefined. >From C11, Sec. 6.7.6.3 `Function declarators (including prototypes)', paragraph 7, p. 133: > A declaration of a parameter as ``array of type'' shall be adjusted > to ``qualified pointer to type'', where the type qualifiers (if any) > are those specified within the [ and ] of the array type derivation. > If the keyword static also appears within the [ and ] of the array > type derivation, then for each call to the function, the value of the > corresponding actual argument shall provide access to the first > element of an array with at least as many elements as specified by > the size expression. Here, as required, the value of the corresponding actual argument does provide access to the first element of an array with at least as many elements as specified by the size expression. In other words, this states a requirement about run-time values, which the code meets, not about compile-time parameter declarations, which is what GCC appears to object to. (In reply to Andrew Pinski from comment #2) > This is basically a dup of bug 108154 I think. That one appears to be different: it trips -Wstringop-overread, not -Wstringop-overflow.
[Bug libstdc++/114879] New: std::ios::sync_with_stdio(false) triggers undefined behaviour of fflush(stdin)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114879
Bug ID: 114879
Summary: std::ios::sync_with_stdio(false) triggers undefined
behaviour of fflush(stdin)
Product: gcc
Version: 13.2.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: campbell+gcc-bugzilla at mumble dot net
Target Milestone: ---
std::ios::sync_with_stdio(false) creates a stdio_filebuf over stdin with mode
in:
182 new (&buf_cin) stdio_filebuf(stdin, ios_base::in);
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/src/c%2B%2B98/ios_init.cc;h=ace94b992b5ac7559352f5e7e94c67f64317bd9d;hb=c891d8dc23e1a46ad9f3e757d09e57b500d40044#l182
The stdio_filebuf constructor for these parameter types passes the arguments on
to __basic_file::sys_open:
158 this->_M_file.sys_open(__f, __mode);
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/ext/stdio_filebuf.h;h=98b8fa2a095acf190237887d4e21394212d1f38a;hb=c891d8dc23e1a46ad9f3e757d09e57b500d40044#l158
With these parameter types, __basic_file::sys_open will, in turn, pass
stdin to fflush in this case, and will only actually open the file if fflush
succeeds:
216 do
217 __err = fflush(__file);
218 while (__err && errno == EINTR);
219 errno = __save_errno;
220 if (!__err)
221 {
222 _M_cfile = __file;
223 _M_cfile_created = false;
224 __ret = this;
225 }
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/io/basic_file_stdio.cc;h=27c2ad2afe3ade7284ec9d487b74d3d04dd756f4;hb=c891d8dc23e1a46ad9f3e757d09e57b500d40044#l216
But stdin is an input stream, not an output or update stream. And calling
fflush on an input stream is undefined behaviour in standard C:
> If stream points to an output stream or an update stream in which
> the most recent operation was not input, the fflush function causes
> any unwritten data for that stream to be delivered to the host
> environment to be written to the file; otherwise, the behavior is
> undefined.
>
> (ISO C11 and ISO C17, Sec. 7.21.5.2 `The fflush function')
On NetBSD 9, what fflush(stdin) does depends on whether fd 0 is open for
writing or not:
- If fd 0 is open for writing (unlikely but possible), it will write a buffer's
worth of heap garbage to fd 0.
- If fd 0 is not open for writing (more likely), fflush will fail with EBADF,
causing __basic_file::sys_open to fail, after which although
std::cin.good() will initially return true, std::cin will otherwise be
nonfunctional (https://gnats.NetBSD.org/58206).
Fix: Don't call fflush if the mode is input.
(This bug first appeared no later than GCC 7, which NetBSD 9 ships with and
where I found the bug, and still appears in GCC 13.2.0, as quoted in the code
above.)
[Bug target/110592] [SPARC] GCC should default to TSO memory model when compiling for sparc32
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110592 Taylor R Campbell changed: What|Removed |Added CC||campbell+gcc-bugzilla@mumbl ||e.net --- Comment #5 from Taylor R Campbell --- (In reply to Eric Botcazou from comment #4) > Well, you need to elaborate a bit here, because the current configuration > has been there for a quarter of century and everybody had apparently > survived it until a couple of days ago. For most of that quarter century, memory ordering was limited to out-of-line barrier/fence subroutines implemented in assembly, like membar_sync in Solaris and NetBSD, or the thread-switch assembly routines in the kernel. It is only relatively recently, since C11 and C++11, that a lot of programs started using in-line barriers/fences and ordered memory operations like store-release/load-acquire. In that time, sparcv7 and sparcv8 haven't gotten a lot of attention, of course. But since they were introduced, NetBSD has had a common userland for sparcv7 and sparcv8, just called `NetBSD/sparc', with a special libc loaded on sparcv8 to use v8-only instructions like SMUL and UMUL for runtime multiplication subroutines to improve performance. (We could in principle do the same for LDSTUB in membar_sync on sparcv7, although we don't at the moment.) But now that programs rely on compiler-generated barriers, there's a conflict between gcc's v7 and v8 code generation: 1. `gcc -mcpu=v7' generates code that lacks LDSTUB where store-before-load barriers are needed, so anything that uses Dekker's algorithm with in-line barriers won't work correctly on a sparcv8 CPU (but it will only manifest in extremely rare, hard-to-diagnose scenarios, because Dekker's algorithm is so obscure). 2. `gcc -mcpu=v8' generates code that uses SMUL and UMUL and other instructions that don't exist on sparcv7. Evidently gcc can be made to generate SMUL/UMUL but omit LDSTUB barriers by using `gcc -mcpu=v8 -mmemory-model=sc', but the other way around doesn't work: `gcc -mcpu=v7 -mmemory-model=tso' still omits the LDSTUB barriers, because the code generation rules for barriers are all gated on TARGET_V8 || TARGET_V9. What we would like to do for NetBSD/sparc is use `-mcpu=v7 -mmemory-model=tso' -- that is, if it worked -- by default. The original submitter drafted a relatively small patch to achieve this, mostly by removing TARGET_V8 || TARGET_V9 conditionals or changing TARGET_V8 to !TARGET_V9 in membar-related code generation rules. But we'd also like to avoid diverging from gcc upstream. Could we convince you to take up an approach like this? Applications built to run on v7-only, of course, could omit the LDSTUBs by using `-mcpu=v7 -mmemory-model=sc' (or perhaps we could have the default be `-mcpu=v7 -mmemory-model=sc', but have bare `-mcpu=v7' imply `-mcpu=v7 -mmemory-model=sc' or something), and applications built to run on v8-only can still use `-mcpu=v8' to take advantage of `SMUL/UMUL'. I expect this would only affect a tiny fraction of programs in extremely rare scenarios -- those that actually rely on Dekker's algorithm (already rare), and hit problems with memory ordering (also rare, only under high contention), using in-line barriers or ordered memory operations (which wasn't the norm a quarter century ago when v7 and v8 were relevant). So you have to go out of your way to hit problems in practice, and any negative performance impact of the extra LDSTUBs on v7 CPUs that don't need them is likely negligible. But it's clear from code inspection and theory that the problem is there.
[Bug target/110592] [SPARC] GCC should default to TSO memory model when compiling for sparc32
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110592 --- Comment #7 from Taylor R Campbell --- > Sorry, no, NetBSD/sparc is too obscure a platform to justify changing the > default for the entire compiler. But you can do like Linux & Solaris and > add sparc/tso.h to the tm_file list of sparc-*-netbsdelf*) in config.gcc. I don't understand, how would that help? As I understand it, whenever `-mcpu=v7', the memory model is just ignored -- even if we set it to TSO -- because all rules that depend on it are gated on TARGET_V8 || TARGET_V9 or similar. I'm not asking for you to change defaults in Linux or Solaris -- I'm just asking to be _able_ to say `-mcpu=v7 -mmemory-model=tso' and get v7-only instruction streams with the LDSTUBs needed for TSO. Right now, with `-mcpu=v7', passing `-mmemory-model=tso' has no effect.
[Bug target/110592] [SPARC] GCC should default to TSO memory model when compiling for sparc32
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110592 --- Comment #10 from Taylor R Campbell --- (In reply to Eric Botcazou from comment #9) > > I don't understand, how would that help? As I understand it, whenever > > `-mcpu=v7', the memory model is just ignored -- even if we set it to TSO -- > > because all rules that depend on it are gated on TARGET_V8 || TARGET_V9 or > > simila > > Well, the subject of the PR is "GCC should default to TSO memory model when > compiling for sparc32" so you'll get exactly that. But defaulting to TSO doesn't seem to help with generating LDSTUB in sparcv7-only instruction streams, unless I misunderstand how this is different from trying to combine `-mcpu' and `-mmemory-model'? > So you want to mix memory models and synchronization instructions with > -mcpu=v7, although they were introduced in the V8 architecture? Correct. The idea is to have a way to generate code that works both on sparcv7 -- by avoiding v8-only instructions like SMUL/UMUL, as `-mcpu=v7' does -- and on sparcv8 -- by generating LDSTUB instructions where store-before-load ordering is needed, as `-mcpu=v8 -mmemory-model=tso' does. I tried to spell this request as `-mcpu=v7 -mmemory-model=tso' but that doesn't generate the LDSTUB instructions needed for store-before-load ordering. (Note that LDSTUB is available in v7 -- what's new in v8 is the relaxation of store-before-load order of TSO, in contrast to SC. So these requirements aren't contradictory.) Is that how Linux and Solaris work by default? I wasn't able to elicit that behaviour by combining explicit `-mcpu' and `-mmemory-model' options, so I assumed that it wouldn't be possible for it to be the default -- and I don't see how it could work given how the code generation rules for memory barriers are gated on TARGET_V8 || TARGET_V9 or similar.
