* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
@ 2023-02-04 23:40 ` pinskia at gcc dot gnu.org
2023-02-04 23:42 ` pinskia at gcc dot gnu.org
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-02-04 23:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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 .
GCC has already decided to never implement unsigned-integer-overflow even
because of how broken this is.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
2023-02-04 23:40 ` [Bug libstdc++/108674] " pinskia at gcc dot gnu.org
@ 2023-02-04 23:42 ` pinskia at gcc dot gnu.org
2023-02-04 23:56 ` lebedev.ri at gmail dot com
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-02-04 23:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
See Also| |https://gcc.gnu.org/bugzill
| |a/show_bug.cgi?id=91547,
| |https://gcc.gnu.org/bugzill
| |a/show_bug.cgi?id=81749
Status|UNCONFIRMED |RESOLVED
Resolution|--- |DUPLICATE
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Exact dup of bug 97844.
*** This bug has been marked as a duplicate of bug 97844 ***
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
2023-02-04 23:40 ` [Bug libstdc++/108674] " pinskia at gcc dot gnu.org
2023-02-04 23:42 ` pinskia at gcc dot gnu.org
@ 2023-02-04 23:56 ` lebedev.ri at gmail dot com
2023-02-05 0:04 ` pinskia at gcc dot gnu.org
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: lebedev.ri at gmail dot com @ 2023-02-04 23:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
Roman Lebedev <lebedev.ri at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|DUPLICATE |---
Status|RESOLVED |UNCONFIRMED
--- Comment #3 from Roman Lebedev <lebedev.ri at gmail dot com> ---
(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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (2 preceding siblings ...)
2023-02-04 23:56 ` lebedev.ri at gmail dot com
@ 2023-02-05 0:04 ` pinskia at gcc dot gnu.org
2023-02-05 0:06 ` pinskia at gcc dot gnu.org
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-02-05 0:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |DUPLICATE
--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Still is exact dup of bug 97844. Does not matter if it was not happening in gcc
11 or not. Still a dup.
*** This bug has been marked as a duplicate of bug 97844 ***
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (3 preceding siblings ...)
2023-02-05 0:04 ` pinskia at gcc dot gnu.org
@ 2023-02-05 0:06 ` pinskia at gcc dot gnu.org
2023-02-05 0:08 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-02-05 0:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> This is quite the hot take.
Hot take from 5 years ago. See te other bugs I referenced and even the mailing
list emails that are referenced from there. Rather clang is the one who decided
this breaking behavior.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (4 preceding siblings ...)
2023-02-05 0:06 ` pinskia at gcc dot gnu.org
@ 2023-02-05 0:08 ` pinskia at gcc dot gnu.org
2023-02-05 0:22 ` redi at gcc dot gnu.org
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-02-05 0:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
See https://gcc.gnu.org/legacy-ml/gcc/2016-07/msg00051.html .
Sorry when I said 5 years I meant 7 years.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (5 preceding siblings ...)
2023-02-05 0:08 ` pinskia at gcc dot gnu.org
@ 2023-02-05 0:22 ` redi at gcc dot gnu.org
2023-02-05 0:31 ` redi at gcc dot gnu.org
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2023-02-05 0:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(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 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.
> 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (6 preceding siblings ...)
2023-02-05 0:22 ` redi at gcc dot gnu.org
@ 2023-02-05 0:31 ` redi at gcc dot gnu.org
2023-02-05 1:15 ` lebedev.ri at gmail dot com
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2023-02-05 0:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Roman Lebedev from comment #3)
> 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.
But it still incorrectly claims there is undefined behaviour. Your own godbolt
link shows:
UndefinedBehaviorSanitizer: undefined-behavior ...
We've talked about this before, see PR 97844.
> 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.
Please stop asking other people to work around broken tools that they don't use
themselves.
You want a workaround, propose a patch.
> In fact, this is a regression.
> This was not happening in libstdc++-11, or ever before.
The code is nearly a decade old, and you even commented on PR 97844 about it
happening in gcc 10. So I don't know where you get the idea it's new in gcc-12.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (7 preceding siblings ...)
2023-02-05 0:31 ` redi at gcc dot gnu.org
@ 2023-02-05 1:15 ` lebedev.ri at gmail dot com
2023-02-05 1:16 ` lebedev.ri at gmail dot com
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: lebedev.ri at gmail dot com @ 2023-02-05 1:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #9 from Roman Lebedev <lebedev.ri at gmail dot com> ---
(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.
:)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (8 preceding siblings ...)
2023-02-05 1:15 ` lebedev.ri at gmail dot com
@ 2023-02-05 1:16 ` lebedev.ri at gmail dot com
2023-02-06 9:42 ` redi at gcc dot gnu.org
2023-02-06 9:54 ` redi at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: lebedev.ri at gmail dot com @ 2023-02-05 1:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #10 from Roman Lebedev <lebedev.ri at gmail dot com> ---
Created attachment 54409
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54409&action=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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (9 preceding siblings ...)
2023-02-05 1:16 ` lebedev.ri at gmail dot com
@ 2023-02-06 9:42 ` redi at gcc dot gnu.org
2023-02-06 9:54 ` redi at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2023-02-06 9:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Roman Lebedev from comment #9)
> (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.
Again no. A map of strings has always compared the strings, and that has always
used that function. Your godbolt example has exactly the same behaviour with
GCC 7 (I didn't bother testing anything older):
tmp$ cat wrap.cc
#include <string>
#include <map>
enum class BitOrder {
LSB, /* Memory order */
MSB, /* Input is added to stack byte by byte, and output is lifted
from top */
MSB16, /* Same as above, but 16 bits at the time */
MSB32, /* Same as above, but 32 bits at the time */
};
const std::map<std::string, BitOrder, std::less<>> order2enum = {
{"plain", BitOrder::LSB},
{"jpeg", BitOrder::MSB},
{"jpeg16", BitOrder::MSB16},
{"jpeg32", BitOrder::MSB32},
};
int main() {
return order2enum.find("plain") == order2enum.end();
}
tmp$ clang++ --gcc-toolchain=$HOME/gcc/7 wrap.cc -O3
-fsanitize=unsigned-integer-overflow
tmp$ ./a.out
/home/jwakely/gcc/7/lib/gcc/x86_64-pc-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/basic_string.h:392:51:
runtime error: unsigned integer overflow: 4 - 6 cannot be represented in type
'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
/home/jwakely/gcc/7/lib/gcc/x86_64-pc-linux-gnu/7.5.0/../../../../include/c++/7.5.0/bits/basic_string.h:392:51
in
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Bug libstdc++/108674] [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header
2023-02-04 23:29 [Bug libstdc++/108674] New: [wish] *Please* silence *intentional* (non-UB!) unsigned overflow in an libstdc++ header lebedev.ri at gmail dot com
` (10 preceding siblings ...)
2023-02-06 9:42 ` redi at gcc dot gnu.org
@ 2023-02-06 9:54 ` redi at gcc dot gnu.org
11 siblings, 0 replies; 13+ messages in thread
From: redi at gcc dot gnu.org @ 2023-02-06 9:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108674
--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Roman Lebedev from comment #10)
> I'm not at all familiar with the GCC's preferred patch protocol,
See https://gcc.gnu.org/contribute.html#patches
> this is the result of `git format-patch origin/master`,
That format is great.
> with commit message mimicking the ones of the recent commits.
> Please let me know what i got wrong this time.
The commit message should be in the form "libstdc++: ..."
I would just use "libstdc++: silence clang's Integer Sanitizer [PR108674]".
The no_sanitize attribute needs to be __no_sanitize__ instead, because this is
a valid program:
#define no_sanitize !!1!1
#include <string>
int main() { }
Otherwise this looks reasonable. The
__no_sanitize__("unsigned-integer-overflow") attribute seems to be supported by
all non-ancient versions of Clang.
Also, please note that the Signed-off-by: tag has the meaning described at
https://gcc.gnu.org/dco.html so please be sure that's what you intend when
using it :-)
Please make the changes above and post to the gcc-patches list, CCing the
libstdc++ list, thanks!
^ permalink raw reply [flat|nested] 13+ messages in thread