[Bug driver/110522] `-fdiagnostics-format=sarif-file`: file name conflicts / races
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110522 Roman Lebedev changed: What|Removed |Added CC||dmalcolm at redhat dot com, ||dseketel at redhat dot com, ||jwakely at redhat dot com --- Comment #2 from Roman Lebedev --- Would it please be possible to get an acknowledgement/rejection on this, please? It seems like a rather unexpected usability roadblock. Also, Dodji Seketeli's email from MAINTAINERS is unknown to bugzilla.
[Bug driver/110522] `-fdiagnostics-format=sarif-file`: file name conflicts / races
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110522 --- Comment #1 from Roman Lebedev --- To spell it out explicitly, not storing the resulting `.sarif` next to the produced object file itself, like it's done in (all?) other cases, very much looks like a not-a-feature, basically making the feature to be borderline unusable in general. There can be some edge-cases (`-o /dev/null`?), but they shouldn't dictate how normal cases are handled.
[Bug driver/110522] New: `-fdiagnostics-format=sarif-file`: file name conflicts / races
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110522
Bug ID: 110522
Summary: `-fdiagnostics-format=sarif-file`: file name conflicts
/ races
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: driver
Assignee: unassigned at gcc dot gnu.org
Reporter: lebedev.ri at gmail dot com
Target Milestone: ---
The sarif-file is stored into PWD of the compiler invocation.
Worse yet, it uses only a basename of the target object file,
but with a `.c.sarif` suffix.
What happens when the same object file is created multiple times,
in different directories? The results will be overwritten.
And in some cases, you may even lose the whole log of the failed compilation,
(especially because the moment you specify `-fdiagnostics-format=sarif-file`,
there's *NOTHING* in stderr, and you can't even specify `-fdiagnostics-format=`
twice)
if it will later be overwritten by the successful compilation
of some different TU that happened to produce object file with the same base
name.
I'm not sure how this was not pointed out during the initial implementation,
this seems problematic.
```
$ cat Makefile
bad:
gcc-13 -fdiagnostics-format=sarif-file -c common/a.c -o common/file.o
good:
gcc-13 -fdiagnostics-format=sarif-file -c b.c -o file.o
all: good bad
clean:
rm common/file.o file.o
$ cat common/a.c
bad
$ cat b.c
// good
$ VERBOSE=1 make bad good
gcc-13 -fdiagnostics-format=sarif-file -c common/a.c -o common/file.o
make: *** [Makefile:2: bad] Error 1
$ ls
Makefile b.c common file.c.sarif file.o
$ cat file.c.sarif
{"$schema":
"https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json;,
"version": "2.1.0", "runs": [{"tool": {"driver": {"name": "GNU C17",
"fullName": "GNU C17 (Debian 13.1.0-7) version 13.1.0 (x86_64-linux-gnu)",
"version": "13.1.0", "informationUri": "https://gcc.gnu.org/gcc-13/;, "rules":
[]}}, "invocations": [{"executionSuccessful": true,
"toolExecutionNotifications": []}], "originalUriBaseIds": {"PWD": {"uri":
"file:///tmp/test/"}}, "artifacts": [{"location": {"uri": "common/a.c",
"uriBaseId": "PWD"}, "contents": {"text": "bad\n"}, "sourceLanguage": "c"}],
"results": [{"ruleId": "error", "level": "error", "message": {"text": "expected
‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ at end of input"}, "locations":
[{"physicalLocation": {"artifactLocation": {"uri": "common/a.c", "uriBaseId":
"PWD"}, "region": {"startLine": 1, "startColumn": 1, "endColumn": 4},
"contextRegion": {"startLine": 1, "snippet": {"text": "bad\n"]}]}]}
$ VERBOSE=1 make -k bad good
gcc-13 -fdiagnostics-format=sarif-file -c common/a.c -o common/file.o
make: *** [Makefile:2: bad] Error 1
gcc-13 -fdiagnostics-format=sarif-file -c b.c -o file.o
$ cat file.c.sarif
{"$schema":
"https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json;,
"version": "2.1.0", "runs": [{"tool": {"driver": {"name": "GNU C17",
"fullName": "GNU C17 (Debian 13.1.0-7) version 13.1.0 (x86_64-linux-gnu)",
"version": "13.1.0", "informationUri": "https://gcc.gnu.org/gcc-13/;, "rules":
[]}}, "invocations": [{"executionSuccessful": true,
"toolExecutionNotifications": []}], "artifacts": [], "results": []}]}
```
[Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674 --- Comment #10 from Roman Lebedev --- Created attachment 54409 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54409=edit the patch I'm not at all familiar with the GCC's preferred patch protocol, this is the result of `git format-patch origin/master`, with commit message mimicking the ones of the recent commits. Please let me know what i got wrong this time.
[Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #9 from Roman Lebedev ---
(In reply to Jonathan Wakely from comment #7)
> (In reply to Roman Lebedev from comment #0)
> > I believe in the version 12, a new instance of such intentional wraparound
> > was introduced into libstdc++: https://godbolt.org/z/rq153fxKW
>
> No, that code is from 2014-12-19.
I agree that the _S_compare is rather old.
What i'm saying is that i have never seen this issue before v12,
so std::map implementation must have been refactored
to use the function in question, and that "introduced" the issue.
> > I understand that there is no UB there.
> > I understand that you are doing this intentionally.
> > The problem is that it is happening in a header,
> > so it's effectively dictating everyone
> > that they should not use that sanitizer.
>
> Well, they shouldn't use it *and expect everybody else's code to never rely
> on unsigned wraparound*. If they want to write their own code to never rely
> on that guaranteed feature of the language, that's fine. They don't get to
> force that choice on everybody else.
Right, of course. As i have said, if i was compiling gcc/libstdc++ itself
with that "broken" sanitizer, closing as WONTFIX would be totally justified,
but here we are in a bit of a gray area, because while that is for sure
a part of implementation, it's rather a user-facing one.
> > Silencing this kind of thing from user side is possible,
> > but it's somewhat cumbersome: it requires compiling with
> > `-fsanitize-recover=integer`, and supplying a run-time suppressions file.
> >
> > On the other hand, suppressing this in-source is trivial:
> > https://godbolt.org/z/E7sEnvvrT
> > ... all it would take is applying
> > `__attribute__((no_sanitize("unsigned-integer-overflow")))`
> > to `_S_compare` on line 483 in `basic_string.h`.
> >
> > I have tried that locally, and it works, but it seems it needs to be
> > wrapped into `#if defined(__clang__)` preprocessor check:
> > https://godbolt.org/z/5a7ox4EWv
> >
> > Forwarded from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029970
>
> So provide a patch then, instead of asking other people to work around this
> sanitizer for you.
:)
[Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674 Roman Lebedev changed: What|Removed |Added Resolution|DUPLICATE |--- Status|RESOLVED|UNCONFIRMED --- Comment #3 from Roman Lebedev --- (In reply to Andrew Pinski from comment #1) > I think this is a bug in clang in the first place for enabling > unsigned-integer-overflow at all. > I would file a bug with clang to disable unsigned-integer-overflow by > default when using -fsanitize=undefined . This is incorrect. unsigned-integer-overflow is *NOT* enabled by -fsanitize=undefined It is enabled by -fsanitize=integer, separately. I'm not enabling it erroneously, but very intentionally. > GCC has already decided to never implement unsigned-integer-overflow even > because of how broken this is. This is quite the hot take. I understand that the behaviours it diagnoses are well-defined by the C and C++ standards, and are well-used in various codebases, however not all of those behaviors are desired by everyone. For example, ((signed char)127)+1 is not a signed overflow, even though you really can't tell me that the effective wraparound is the semantics *everyone* expects there. :) However, that is not the question here. I really don't care whether or not you rely on the wraparound semantics of the unsigned types in the library. I really don't. This is only a problem because it happens in a public header. All i'm asking is to improve the UX of the user-facing side of the C++ standard library implementation, and make it more usable by wider variety of the scenarios. In fact, this is a regression. This was not happening in libstdc++-11, or ever before.
[Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
Bug ID: 108674
Summary: [wish] *Please* silence *intentional* (non-UB!)
unsigned overflow in an libstdc++ header
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: lebedev.ri at gmail dot com
Target Milestone: ---
Dear maintainer.
As everyone knows, unsigned integer overflow is well-defined in C and C++.
However, there are situations where you *know* that a particular code
should not have any overflows. To catch them, there's Integer Sanitizer
in clang (`-fsanitize=integer`).
Unfortunately as one would expect, while some might want to have no
unsigned overflows, others may very well depend on the defined behavior.
As is the case, the GCC, and in particular libstdc++ fall into the
latter category.
I believe in the version 12, a new instance of such intentional wraparound
was introduced into libstdc++: https://godbolt.org/z/rq153fxKW
Running this on a debian machine, we get:
```
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:483:51:
runtime error: unsigned integer overflow: 4 - 6 cannot be represented in type
'size_type' (aka 'unsigned long')
#0 0x55e69e5b6818 in std::__cxx11::basic_string, std::allocator>::_S_compare(unsigned long,
unsigned long)
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:483:51
#1 0x55e69e5b6818 in std::__cxx11::basic_string,
std::allocator>::compare(std::__cxx11::basic_string, std::allocator> const&) const
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:3150:10
<...>
```
I understand that there is no UB there.
I understand that you are doing this intentionally.
The problem is that it is happening in a header,
so it's effectively dictating everyone
that they should not use that sanitizer.
Silencing this kind of thing from user side is possible,
but it's somewhat cumbersome: it requires compiling with
`-fsanitize-recover=integer`, and supplying a run-time suppressions file.
On the other hand, suppressing this in-source is trivial:
https://godbolt.org/z/E7sEnvvrT
... all it would take is applying
`__attribute__((no_sanitize("unsigned-integer-overflow")))`
to `_S_compare` on line 483 in `basic_string.h`.
I have tried that locally, and it works, but it seems it needs to be
wrapped into `#if defined(__clang__)` preprocessor check:
https://godbolt.org/z/5a7ox4EWv
Forwarded from https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1029970
[Bug c++/107763] -Wreturn-type false-positive with fully-covered switch over enum
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107763 --- Comment #5 from Roman Lebedev --- Thank you. Forwarded to https://github.com/llvm/llvm-project/issues/59085
[Bug c++/107763] -Wreturn-type false-positive with fully-covered switch over enum
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107763 Roman Lebedev changed: What|Removed |Added Component|c |c++ --- Comment #2 from Roman Lebedev --- (In reply to Andrew Pinski from comment #1) > >The `a` can only have value `a::b`, > > NO, it has a full range of the underlying type. For GCC, see > https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Structures-unions-enumerations- > and-bit-fields-implementation.html#Structures-unions-enumerations-and-bit- > fields-implementation > > "Normally, the type is unsigned int if there are no negative values in the > enumeration, otherwise int. If -fshort-enums is specified, then if there are > negative values it is the first of signed char, short and int that can > represent all the values, otherwise it is the first of unsigned char, > unsigned short and unsigned int that can represent all the values. > > On some targets, -fshort-enums is the default; this is determined by the ABI. > > " Is this situation different in C++? looks like i set the component wrong. Is this implementation-defined behavior, or are you saying that clang is wrong here? At run time, `a` can not have any other value other than `a::b`: https://godbolt.org/z/hnc665755 so this really does seem like a bug.
[Bug c/107763] New: -Wreturn-type false-positive with fully-covered switch over enum
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107763
Bug ID: 107763
Summary: -Wreturn-type false-positive with fully-covered switch
over enum
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: lebedev.ri at gmail dot com
Target Milestone: ---
enum a { b };
int foo(a a) {
switch(a) {
case b: return 42;
}
}
The `a` can only have value `a::b`,
so the function always returns 42, yet gcc complains:
> warning: control reaches end of non-void function [-Wreturn-type]
https://godbolt.org/z/j8ezrr5h3
[Bug c/101089] New: [OpenMP] Diagnostic difference with clang
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101089 Bug ID: 101089 Summary: [OpenMP] Diagnostic difference with clang Product: gcc Version: unknown Status: UNCONFIRMED Severity: normal Priority: P3 Component: c Assignee: unassigned at gcc dot gnu.org Reporter: lebedev.ri at gmail dot com Target Milestone: --- https://godbolt.org/z/KPc5hM3f9 clang: :4:39: error: variable 'c' must have explicitly specified data sharing attributes #pragma omp parallel for simd aligned(c) firstprivate(a) default(none) ^ :4:66: note: explicit data sharing attribute requested here #pragma omp parallel for simd aligned(c) firstprivate(a) default(none) ^ 1 error generated. Compiler returned: 1 gcc: no diagnostic.
[Bug libstdc++/97844] Unsigned Integer Overflow when comparing strings (|s1|<|s2|)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97844 Roman Lebedev changed: What|Removed |Added CC||lebedev.ri at gmail dot com --- Comment #2 from Roman Lebedev --- (In reply to Andrew Pinski from comment #1) > unsigned integer overflow DOES NOT EXIST. It is called wrapping only. > This is NOT undefined at all. I agree, and no one said it's undefined. > clang is broken. It's not, actually. `-fsanitize=undefined` does *NOT* warn on that, `-fsanitize=integer` does, and it's explicitly marketed as sanitizing some behavior that is explicitly *NOT* UB, but very often unintentional. So the bugreport is correct in a way, libstdc++ for -std=c++20 contains `-fsanitize=integer` issues.
