[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2023-10-08 Thread llvm at rifkin dot dev via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #21 from Jeremy R.  ---
Another option might be just do nothing and don't set the badbit, just pretend
it's an empty string. This shouldn't break existing programs and would at least
be something a programmer could more easily track down.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2023-10-08 Thread llvm at rifkin dot dev via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #20 from Jeremy R.  ---
Silently ruining the behavior of the rest of a program and leaving the
programmer to pull their hair out over what on earth is happening seems very
un-ideal behavior.

This is a very easy mistake to make and the current behavior makes tracking
down that mistake exceedingly difficult.

I do recognize the concerns here about existing programs and crashing being
arguably less friendly, but something should be done here. 

Libc++ and msvc's stl both segfault and there's some value in making the
behavior here consistent.

(In reply to Jonathan Wakely from comment #13)
> I'm not suggesting we should print "(null)", that would be crazy
Maybe, but it's also a nice middle ground between silently corrupting the
stream and crashing the program.

Throwing an exception, even only in debug, also seems reasonable as existing
programs may be able to recover from the exception without crashing.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2023-10-08 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

Andrew Pinski  changed:

   What|Removed |Added

 CC||llvm at rifkin dot dev

--- Comment #19 from Andrew Pinski  ---
*** Bug 111729 has been marked as a duplicate of this bug. ***

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2023-07-10 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

Jonathan Wakely  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
 Ever confirmed|0   |1
   Last reconfirmed||2023-07-10

--- Comment #18 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #13)
> I think there would need to be a period of deprecation and an optional
> assertion enabled by _GLIBCXX_ASSERTIONS before we can do that (assuming we
> want to).

As I said in the PR 109891 dup, _GLIBCXX_DEBUG_PEDANTIC exists for precisely
this kind of case:
https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode_semantics.html

So let's add that for now.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2023-07-09 Thread pinskia at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

Andrew Pinski  changed:

   What|Removed |Added

 CC||mimomorin at gmail dot com

--- Comment #17 from Andrew Pinski  ---
*** Bug 109891 has been marked as a duplicate of this bug. ***

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread p.sanders at alpinesoft dot co.uk
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #16 from Paul Sanders  ---
Interesting, I can see why you don't want to change the behaviour again.  It's
just a shame it ever did anything other than SEGFAULT in the first place but as
you point out, it's been the way it is for a long time now and changing it
would be foolhardy.

Just to finish off (I see that the decision is made):

> this code has been present since r53839 in 2002 ... [and] it's far too late 
> to just turn this into a segfault because somebody on stackoverflow found it 
> surprising and doesn't know how to check iostreams for errors.

I'm sure myself and the OP aren't the only ones ever to have been 'surprised'
by this, and more will follow.  I will update my answer at SO to explain what
is really happening and how to fix it (the original code wasn't my own).  

Thank you for taking the time to lay things out for me, I learnt a lot here,
even if I still don't like the way ostream behaves under these circumstances. 
It seems to me that badbit's job is to reflect whether or not the stream is in
a usable state, and not get set just because something strange has been passed
to the stream which does it no actual harm.  It (badbit) just got abused back
in 2002, so to speak, and now we're stuck with it.

And finally, I owe you an apology for ever thinking that ostream would silently
terminate your program like that; I posted one on SO. I hope it does some good,
I don't set out to aggravate you guys.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #15 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #13)
> This is not an accident, it's a supported feature (and arguably documented,
> by the code and the test).

See also Bug 6518 which changed the segfault to setting failbit (and introduced
a regression) and Bug 6750 which introduced the current behaviour.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #14 from Jonathan Wakely  ---
(In reply to Paul Sanders from comment #12)
> Any interest?

Good grief, no.

That would generate even worse code than what we have now, and it's possible to
test for badbit without enabling exceptions. You keep making assumptions about
what everyone else's programs do, and we have no way of knowing what the
majority of libstdc++ users expect from this code, or how much code relies on
the current behaviour.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #13 from Jonathan Wakely  ---
(In reply to Martin Sebor from comment #10)
> As a data point, calling printf ("%s", p) does lead to a segfault in Glibc
> for a null p because GCC turns the call into puts(p)

Only when optimization is enabled.

I'm not suggesting we should print "(null)", that would be crazy, I'm just
demonstrating that expecting a segfault for this kind of error is not a safe or
valid assumption.

> which doesn't have the
> same feature (see https://sourceware.org/bugzilla/show_bug.cgi?id=5618 for
> the background).
> 
> I think most users prefer invalid uses of pointers to fail loudly so they
> can be caught early.  Few users expect output functions to fail, and even
> fewer bother to check for failures when writing to standard streams.

Agreed, but this code has been present since r53839 in 2002, and there's an
explicit test for that behaviour:
testsuite/27_io/basic_ostream/inserters_character/char/8.cc

This is not an accident, it's a supported feature (and arguably documented, by
the code and the test).

> Besides the inserter without the test resulting in more efficient code and
> GCC emitting a warning when a null pointer is passed to it (it doesn't now
> even though it should because of the strlen call), leaving it to the
> compiler to deal with can also lead to better code downstream thanks to
> -fdelete-null-pointer-checks.

I think there would need to be a period of deprecation and an optional
assertion enabled by _GLIBCXX_ASSERTIONS before we can do that (assuming we
want to).

If we were starting from scratch I wouldn't add the check (e.g. see the patch
at https://gcc.gnu.org/ml/gcc-patches/2018-06/msg00768.html that I proposed
today) but it's far too late to just turn this into a segfault because somebody
on stackoverflow found it surprising and doesn't know how to check iostreams
for errors.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread p.sanders at alpinesoft dot co.uk
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #12 from Paul Sanders  ---
Sorry, I posted that in a bit of a rush.  I took a proper look and the null
pointers that set badbit actually make excellent sense.

So I'll suggest a way out of the backwards compatibility conundrum when
`ostream::operator<<` is passed a null pointer (of any sort).  The suggestion I
have is to test for a null pointer _only_ if the stream is set up to throw
badbit exceptions  Otherwise, just blindly dereference the pointer and crash.

The rationale for this is that people who have set up an exception handler for
badbit _might_ just have done so to catch null pointers.  I think it's unlikely
but it's possible.  Those who haven't are probably getting away with something
they shouldn't.

Any interest?

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread p.sanders at alpinesoft dot co.uk
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #11 from Paul Sanders  ---
> I think most users prefer invalid uses of pointers to fail loudly so they can 
> be caught early.  Few users expect output functions to fail, and even fewer 
> bother to check for failures when writing to standard streams.

Yes, that's very much my own personal preference, but as I dig into this more I
can see there are problems with doing what I'm asking for.

> Removing this longstanding behaviour now could break an unknown number of 
> programs which are (implicitly or explicitly) relying on this behaviour of 
> libstdc++.

I think they'd have to be relying on it explicitly (i.e. they turn on badbit
exceptions or check explicitly for badbit).  Otherwise, they have a bug. 
Question is, how many are there out there?  I doubt many are testing badbit. 
Not for this reason, anyway.

There is however, some info at
[cppreference](http://en.cppreference.com/w/cpp/io/ios_base/iostate) which
supports your view.  Passing in quite a few types as nullptr sets badbit (too
many, IMO), but `char *` is not mentioned explicitly.

So I suppose it's reasonable for people to expect that _all_ null pointers
should be treated in the same way.  It's a shame anyone on the standards
committee ever thought that testing for null pointers in this way was useful
but I suppose we're stuck with it now.  Personally, I think it was a big
mistake.

> Failure to open a file sets failbit, not badbit.

Noted, thank you, I'm not very familiar with iostreams.  Forget that bit then.

> The program did work fine though, and anybody can verify that it does so 
> intentionally, not by accident.

Nice link, thank you, but that doesn't change the basic argument that many
programs will not be working quite right because they accidentally passed in a
null pointer and didn't expect the resulting behaviour of `ostream`.  Still, I
might in the end have to say here, sadly: c'est la vie.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread msebor at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #10 from Martin Sebor  ---
As a data point, calling printf ("%s", p) does lead to a segfault in Glibc for
a null p because GCC turns the call into puts(p) which doesn't have the same
feature (see https://sourceware.org/bugzilla/show_bug.cgi?id=5618 for the
background).

I think most users prefer invalid uses of pointers to fail loudly so they can
be caught early.  Few users expect output functions to fail, and even fewer
bother to check for failures when writing to standard streams.

Besides the inserter without the test resulting in more efficient code and GCC
emitting a warning when a null pointer is passed to it (it doesn't now even
though it should because of the strlen call), leaving it to the compiler to
deal with can also lead to better code downstream thanks to
-fdelete-null-pointer-checks.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #9 from Jonathan Wakely  ---
(In reply to Paul Sanders from comment #8)
> Thanks for your comments.  I can see there are two sides to this.
> 
> I was in the middle of composing the tract below.  I'll include that anyway
> because it took me ages to type.  There's a bit at the end about people who
> might get bitten if you do make this change, but a plausible alternative to
> crashing the program might be to (a) print "(null)", and (b) leave the
> badbit alone.  I think I'd settle for that, I actually quite like the way
> the printf family behaves.

That would be another valid outcome of undefined behaviour, but would make it
much harder to detect the problem. Setting badbit makes it possible to detect
and take action (including calling std::abort() or throwing an exception, if
you want to).

> My view of the badbit, BTW, is that it's not there to catch programming
> errors - it's there to report adverse events that come up at runtime (e.g.
> cannot open file).  I expand on that below.

Failure to open a file sets failbit, not badbit.

> After all, ostream would likely crash if the pointer was invalid, so what's
> so special about NULL?

It's detectable.

> [Additional note: I can see if you change the current behaviour you might
> get a bit of flak from people whose programs "used to work fine".  Well,
> they didn't, and they should be grateful to you for alerting them to that
> fact.]

The program did work fine though, and anybody can verify that it does so
intentionally, not by accident:

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/ostream.tcc;h=e02ba55a6262b96ead90595e1aad56119a965844;hb=HEAD#l323

As the standard says:

  Permissible undefined behavior ranges from ignoring the situation completely
  with unpredictable results, to behaving during translation or program
  execution in a documented manner characteristic of the environment (with or
  without the issuance of a diagnostic message), to terminating a translation
  or execution (with the issuance of a diagnostic message).

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread p.sanders at alpinesoft dot co.uk
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #8 from Paul Sanders  ---
Thanks for your comments.  I can see there are two sides to this.

I was in the middle of composing the tract below.  I'll include that anyway
because it took me ages to type.  There's a bit at the end about people who
might get bitten if you do make this change, but a plausible alternative to
crashing the program might be to (a) print "(null)", and (b) leave the badbit
alone.  I think I'd settle for that, I actually quite like the way the printf
family behaves.

My view of the badbit, BTW, is that it's not there to catch programming errors
- it's there to report adverse events that come up at runtime (e.g. cannot open
file).  I expand on that below.

-

The code I posted was by way of example.  I didn't realise at the time what was
causing the behaviour I was observing - this whole thing came up on
StackOverflow and you can see the thread here (please excuse me calling gcc
'naughty', I didn't have the full picture then):

https://stackoverflow.com/questions/50696746/c-template-class-instance-issue

I guess my point is that the way things are implemented currently is likely to
lead to subtle, undetected bugs.  Not everyone checks for errors properly, and
not everyone (and that included me until just now) knows that if you want an
iostream to throw exceptions you have to tell it so.  Result?  People
innocently write code like that shown on in that thread and think they have
done all they need to.  But they haven't.

So I see the current behaviour as being the worst of all worlds.  After all,
ostream would likely crash if the pointer was invalid, so what's so special
about NULL?  They are both programming errors, and not the sort of situation
you want to have to write extra code to handle explicitly. Throwing an
exception should be reserved for some external influence on the stream (such as
a remote file becoming inaccessible due to a network error, say), which you
*do* need to handle in your code.

To put it another way, what are you going to do in your exception handler if
you get a nullptr exception (whatever that is - is there even one?).  Display
"nullptr encountered, operation aborted" to the user and bail out?  That's not
going to impress your customer base.  The reason I like bringing the program
down is that it provides an opportunity to generate a stack trace and post it
home somehow.  But hopefully, the coding error is discovered before the code
even gets out into the field, if you do this.

[Additional note: I can see if you change the current behaviour you might get a
bit of flak from people whose programs "used to work fine".  Well, they didn't,
and they should be grateful to you for alerting them to that fact.]

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #7 from Jonathan Wakely  ---
I don't know the original reason for handling null pointers here, but it's
consistent with glibc's printf which prints "(null)" when a null pointer is
provided for a %s specifier.

Removing this longstanding behaviour now could break an unnknown number of
programs which are (impicitly or explicitly) relying on this behaviour of
libstdc++.

It's not at all clear to me that crashing is preferable.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #6 from Jonathan Wakely  ---
And they don't test for iostream operations failing?

The program has undefined behaviour, i.e. a bug. Whether it's better to
identify that and treat it as a corrupt stream state (setting badbit, and
optionally throwing an exception) or crash the program depends on the program.
There's no single choice that's correct for all programs.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #5 from Jonathan Wakely  ---
*** Bug 86129 has been marked as a duplicate of this bug. ***

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread p.sanders at alpinesoft dot co.uk
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #4 from Paul Sanders  ---
@Johnathon Crashing the program is the right thing to do, because it means that
the developer (or the test department) will get to find out about the problem
before the customer does.

Does that help you see why I am pushing for it?

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread redi at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #3 from Jonathan Wakely  ---
Arguably crashing the program is more unfriendly.

We could add the nonnull attribute to the relevant ostream members, or add
assertions that can be optionally enabled, but I'm not convinced that crashing
is necessarily an improvement.

[Bug libstdc++/86130] Expect SIGSEGV but program just silently exits

2018-06-13 Thread p.sanders at alpinesoft dot co.uk
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86130

--- Comment #2 from Paul Sanders  ---
Hi Martin,

Thanks very much for your prompt reply, and I completely agree with your
viewpoint.

I therefore hereby request that libstc++ stops behaving like that and just lets
the SIGSEGV happen.  The current behaviour is very unfriendly.