Bug 22200

Summary: numeric_limits<signed>::is_modulo is inconsistent with gcc
Product: gcc Reporter: Michael Veksler <veksler>
Component: libstdc++Assignee: Not yet assigned to anyone <unassigned>
Status: NEW ---    
Severity: normal CC: bagnara, gcc-bugs, gdr, gdr, glisse, marc.glisse, rguenth
Priority: P2    
Version: 4.0.0   
Target Milestone: ---   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2010-02-19 10:30:40
Bug Depends on: 30484    
Bug Blocks:    

Description Michael Veksler 2005-06-27 15:19:11 UTC
GCC does not have a proper modulo overflow semantics for signed
integral types.
1. The loop optimizer seems to assume that signed overflow does
   not happen (or is undefined). 
2. On x86 INT_MAX/-1 seems to trap (instead of implementing
   modulo semantics).

In that case numeric_limits<signed>:is_modulo should be synchronized
with gcc and be set to false instead of true.

Another option, is to change gcc to fully support the modulo
arithmetics of signed integer.
Comment 1 Andrew Pinski 2005-06-27 15:21:21 UTC
INT_MAX/-1 is undefined.
and signed overflow is undefined.

Why file this bug when the comments on the list say this is not a bug?
Comment 2 Michael Veksler 2005-06-27 15:33:39 UTC
This is a bug because std::numeric_limits<signed>::is_modulo
should be true only if singed overflow is defined.
This is not the case with gcc, because gcc does not have the
extension "signed oveflow == module" then is_modulo should
not lie.
The undefinedness referse to MAX_INT+1 not to is_modulo.
Comment 3 Andrew Pinski 2005-06-27 15:40:33 UTC
I think we need to read:
"ISO/IEC 10967-1 Language Independent Arithmetic, part 1" since that is what the standard references 
for is_modulo.
Comment 4 Gabriel Dos Reis 2005-06-27 16:06:32 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| INT_MAX/-1 is undefined.
| and signed overflow is undefined.
| 
| Why file this bug when the comments on the list say this is not a bug?

Maybe the real question is why you feel you must send your message.

Please do consider the issue as raised on the main list and the
discussion there. 

The point is we must say something about numerci_limits<>, and this
consistently with the behaviour of GCC.  This probably is not an issue
for C, but it is for C++.

For all useful purposes, do remember that GCC used to define is_modulo
as false for signed integer types.  It was consciously changed to true
by RTH.  See the reference I gave.

Now, if you do not understand the issue, please refrain from sending
unhelpful messages.

-- Gaby
Comment 5 Andrew Pinski 2005-06-27 16:07:01 UTC
Actually it is modulo for all operations.  
and INT_MAX/-1 does not raise a trap.
Comment 6 Gabriel Dos Reis 2005-06-27 16:09:09 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| I think we need to read:
| "ISO/IEC 10967-1 Language Independent Arithmetic, part 1" since that
| is what the standard references for is_modulo.

yes, as pointed out in the message I sent.

It was pointed out that the loop optimizers was making some
assumtpions.  If the only is only for the loop optimizers, then we
can also consider the option of explicitly allowing specific behaviour
for induction variables.  At any rate, a proper resolution for this
PR is hardly "the standard says it is undefined behaviour".

-- Gaby
Comment 7 Gabriel Dos Reis 2005-06-27 16:21:17 UTC
(In reply to comment #1)
> INT_MAX/-1 is undefined.
> and signed overflow is undefined.
> 
> Why file this bug when the comments on the list say this is not a bug?

(In reply to comment #5)
> Actually it is modulo for all operations.  
> and INT_MAX/-1 does not raise a trap.

Andrew --

You do not seem to understand this PR.  Please DO NOT close it.
Your eagerness to close PRs is doing harms -- that was already debated
last couple o weeks and I do not want to repeat that again.
Comment 8 Gabriel Dos Reis 2005-06-27 16:25:02 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Actually it is modulo for all operations.  

But then do read the comment as far as the loop optimizer is
concerned. It does not seem like it understands that it is modulo
arithmetic. 

| and INT_MAX/-1 does not raise a trap.

It that is the case, then it is one issue less.  But that wasn't the
purpose of this PR.  The issue is far from resolved just because
you assert that INT_MAX/-1 does not trap.

-- Gaby
Comment 9 Haren Visavadia 2005-06-27 16:32:17 UTC
(In reply to comment #1)
> Why file this bug when the comments on the list say this is not a bug?
It's for the potentially long debate.
Comment 10 Michael Veksler 2005-06-27 16:35:01 UTC
In Comment #5 Andrew Pinski writes:
> Actually it is modulo for all operations.  
> and INT_MAX/-1 does not raise a trap.

That was a typo on my part.

It was supposed to be INT_MIN/-1

INT_MAX/-1 does not require modulo because in 2's complement
INT_MIN==-INT_MAX-1
Comment 11 Andrew Pinski 2005-06-27 16:53:38 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc


On Jun 27, 2005, at 12:25 PM, gdr at integrable-solutions dot net wrote:

> | Actually it is modulo for all operations.
>
> But then do read the comment as far as the loop optimizer is
> concerned. It does not seem like it understands that it is modulo
> arithmetic.

But that is because overflow is undefined, read what other people
have written.  It is modulo for all normal operations but it is still
undefined at what the output will be.

-- Pinski

Comment 12 Andrew Pinski 2005-06-27 17:29:54 UTC
(In reply to comment #7)
> Andrew --
> 
> You do not seem to understand this PR.  Please DO NOT close it.
> Your eagerness to close PRs is doing harms -- that was already debated
> last couple o weeks and I do not want to repeat that again.

yes it will be debated until the end of time.  INT_MIN/-1 is undefined as declared by the C/C++ 
standards so this is invalid.  we get modulo results most of the time, but again since this is undefined, 
we may as well say it is modulo because it is.  I think the standard say that if two postive values are 
added to and you __may__ get a value that is less than both of them, then is_modulo should be set to 
true.  (I don't have the copy right in front of me right now but IIRC that is what it says)


In fact from the comment in std_limits.h says "if possible" meaning "may":
    /** True if the type is @e modulo, that is, if it is possible to add two
        positive numbers and have a result that wraps around to a third number
        that is less.  Typically false for floating types, true for unsigned
        integers, and true for signed integers.  */
Comment 13 Andrew Pinski 2005-06-27 17:34:31 UTC
Invalid as the C++ standard says:
" True if the type is modulo.203) A type is modulo if it is possible to add two positive numbers and 
have a result that wraps around to a third number that is less. Generally, this is false for floating types, 
true for unsigned integers, and true for signed integers on"

it says "if possible" which means not always but some of the time.
Comment 14 Paul Schlie 2005-06-27 18:00:32 UTC
(In reply to comment #13)
> Invalid as the C++ standard says:
> " True if the type is modulo.203) A type is modulo if it is possible to add two positive numbers and 
> have a result that wraps around to a third number that is less. Generally, this is false for floating 
types, 
> true for unsigned integers, and true for signed integers on"
> 
> it says "if possible" which means not always but some of the time.

It's only "possible" because the sum of two positive numbers may not exceed that
type's MAX value; not because if a type is modulo it may not produce a modulo result.

Comment 15 Gabriel Dos Reis 2005-06-27 18:19:04 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc

"pinskia at physics dot uc dot edu" <gcc-bugzilla@gcc.gnu.org> writes:

| Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc
| 
| 
| On Jun 27, 2005, at 12:25 PM, gdr at integrable-solutions dot net wrote:
| 
| > | Actually it is modulo for all operations.
| >
| > But then do read the comment as far as the loop optimizer is
| > concerned. It does not seem like it understands that it is modulo
| > arithmetic.
| 
| But that is because overflow is undefined, read what other people
| have written.

I read what other people say.  But you do not seem to understand what
is_modulo means, even when you point to the LIA-1 part.  Please, do 
consider the semantics described there and the quote I provided.
Basically, you can decide that it overflows and send a notification of
overlfow but "wrap" with defined meaning. 

-- Gaby
Comment 16 Gabriel Dos Reis 2005-06-27 18:23:22 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| ------- Additional Comments From pinskia at gcc dot gnu dot org  2005-06-27 17:29 -------
| (In reply to comment #7)
| > Andrew --
| > 
| > You do not seem to understand this PR.  Please DO NOT close it.
| > Your eagerness to close PRs is doing harms -- that was already debated
| > last couple o weeks and I do not want to repeat that again.
| 
| yes it will be debated until the end of time.  INT_MIN/-1 is undefined as declared by the C/C++ 
| standards so this is invalid.  we get modulo results most of the time, but again since this is undefined, 

The issue is whether that modulo result is taken to be the definition that
GCC gives to the operation.  At this point, saying "the standard 
says it is undefined behaviour" is pointless and unhelpful.

| we may as well say it is modulo because it is.

Yes, that is part of the issue.

|  I think the standard say that if two postive values are  
| added to and you __may__ get a value that is less than both of them,

yes, but at this point we're more interesting in what GCC decides that
behaviour should be.  It is irrelevant to say "the standard says it is
undefined behaviour".

| then is_modulo should be set to  true.  (I don't have the copy right in front of me right now but IIRC that is what it says)
| 
| 
| In fact from the comment in std_limits.h says "if possible" meaning "may":
|     /** True if the type is @e modulo, that is, if it is possible to add two
|         positive numbers and have a result that wraps around to a third number
|         that is less.  Typically false for floating types, true for unsigned
|         integers, and true for signed integers.  */

Thanks, I'm the author of <limits>.  For all useful purposes, please
Andrew go back and read the link I gave to RTH's message.

-- Gaby
Comment 17 Gabriel Dos Reis 2005-06-27 18:25:58 UTC
Andrew is being silly.
Comment 18 Gabriel Dos Reis 2005-06-27 18:27:06 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| ------- Additional Comments From pinskia at gcc dot gnu dot org  2005-06-27 17:34 -------
| Invalid as the C++ standard says:

Andrew --
Stop this unhelpful behaviour.  

If you don't understand the issue, refrain from closing it.   There is
a consistency problem between numeric_limimts<>::is_modulo and GCC's
optimizations assumptions. 

-- Gaby
Comment 19 Andrew Pinski 2005-06-27 18:34:37 UTC
(In reply to comment #16)
> Thanks, I'm the author of <limits>.  For all useful purposes, please
> Andrew go back and read the link I gave to RTH's message.

Yes and RTH's comment about trapping is wrong, because PPC does not trap at all.  See PR 22203 for 
that bug.
Comment 20 Gabriel Dos Reis 2005-06-27 18:46:47 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is inconsistend with gcc

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| ------- Additional Comments From pinskia at gcc dot gnu dot org  2005-06-27 18:34 -------
| (In reply to comment #16)
| > Thanks, I'm the author of <limits>.  For all useful purposes, please
| > Andrew go back and read the link I gave to RTH's message.
| 
| Yes and RTH's comment about trapping is wrong, because PPC does not trap at all.  See PR 22203 for 
| that bug.

You seem to confuse all issues.  Please be careful in trying issues.
The fact that you assert that integers don't trap does not make RTH's
comment wrong.  Furtheremore, are you claiming that

    1 / 0 

does not trap on PPC? (I do not have a PPC to check).  If that is
true, you're welcoome to submit a patch to rectify the value for PPC.

-- Gaby
Comment 21 Michael Veksler 2005-06-27 20:28:07 UTC
(In reply to comment #13)
> Invalid as the C++ standard says:
> " True if the type is modulo.203) A type is modulo if it is possible to add
> two positive numbers and 
> have a result that wraps around to a third number that is less. Generally,
> this is false for floating types, 
> true for unsigned integers, and true for signed integers on"
> 
> it says "if possible" which means not always but some of the time.

It seems to me you are being subtly inconsistent.
Correct me if I misread your comments:
1. signed overflow is undefined
   [letting the compiler do whatever it likes.
    In that case the compiler may decide, among other things, to trap,
    saturate or modulo].
2. It is OK for a compiler to set is_modulo because as it is undefined,
   modulo result may be recieved from the random junk that "undefined"
   produces. And to prove your point you refer to "...it is possible to
   add two positive numbers and have a result that wraps around.."

The logic of 2. works both ways. Any logical conclusion will be
valid in the presense of "undefined". I can say that since <limits>
does not know exactly what gcc does or does not do, it wll *may*
be possible that the undefined behavior always leads to MAX_INT.

So, if we want to have a reasonable meaning for the sentance 
"...it is possible to add ...", it should be read as 
"...it is possible to choose such values that when added...".
Reading any other way does not make sense, especially if you put
the nondeterministic "undefined" into the mix. 

This requires a DR to get a formal clarification, for people with 
Andrew's point of view, and avoid future confusion.

Andrew: Don't you think that all this noise indicates that this is
not a clear cut case? When there is a dispute, aren't we choose
the most pragmatic and useful point of view (until a formal clarification
is given)? Don't you agree that:
1. Sometimes using "undefined" simplifies the compiler and improves
   generated code.
2. (OTOH) Undefind situations are unhelpful the the users, they complicate
   debugging, and make programming harder. Reducing rules that imply
   undefined behavior semantics is good for the users.
3. Each language strikes a different balance between 1 and 2.
4. Claiming that numeric_limits<signed>::is_modulo based on "undefined"
   does not help the user, because as a result you lose all the benefits of
   is_modulo to begin with. Consider the example:
   template <class T> T foo(T a, T b)
   {
    // For example simplicity, this "if" is not generic.
    if (numeric_limits<T>::is_modulo)
      return trivial_code (a+b); // undefined for signed
    else
      return contorted_slow_code(a, b);
   }

   is_modulo = true is unhelpful, it will trigger the undefined case,
   a case that the code tries to avoid. It makes it more difficult to 
   write generic code that avoids the trouble.
   It is much more helpful to go for is_modulo = false.
5. There is no performance penatly for is_modulo = false, so item 1 does
   not come into play.
6. According to 3, the language should strike a balance between 1 and 2.
   Since 1 is not playing any role here, then 2 wins and the language
   should go for the semantics that does not depend on "undefined".
7. The text of the standard may be interpreded both ways (a fact: several
   people draw contradictory conclusions from the document). In that
   case we should choose what makes more sense if we would write the
   document.
8. When interpreting an ambiguous document (7), we should take the motives
   of the writers and the whole document objectives into account.
   According to 6, is_modulo = false strikes the best balance, as a result
   it would have been preferred by any reasonable committy.

  Michael   
Comment 22 Andrew Pinski 2005-06-27 21:02:42 UTC
(In reply to comment #21)
> 1. Sometimes using "undefined" simplifies the compiler and improves
>    generated code.
> 2. (OTOH) Undefind situations are unhelpful the the users, they complicate
>    debugging, and make programming harder. Reducing rules that imply
>    undefined behavior semantics is good for the users.

No I disagree with that, most undefined behaviors are very easy to debug,
the most obvious one and most invoked undefined behavior in terms of bug
reports is violating C/C++ aliasing rules (see PR  21920 which has 69 duplicates).
In fact if you look at GCC, it is the only compiler were people run into violating C/C++
aliasing rules beause we use them to check if two addresses can alias.  This is unlike almost all other 
compilers which represent their aliasing info as base+address and they only invoke the C/C++ aliasing 
rules when they cannot figure out if they point to the same address.
Comment 23 Michael Veksler 2005-06-27 22:00:31 UTC
(In reply to comment #22)
> (In reply to comment #21)
> > 2. (OTOH) Undefind situations are unhelpful the the users, they complicate
> >    debugging, and make programming harder. Reducing rules that imply
>  >   undefined behavior semantics is good for the users.

> No I disagree with that, most undefined behaviors are very easy to debug,
> the most obvious one and most invoked undefined behavior in terms of bug
> reports is violating C/C++ aliasing rules (see PR  21920 which has 69
> duplicates).

What you disagree with? You're claiming that undefined behavior does
not complicate debugging? Isn't it true that with undefined behavior
you're most likely to bump into behavioral differences between -O2 and -O0?
Isn't it true that debugging an -O2 code is more complicated than -O0 ?
Q.E.D.

Also, how did you come to the conclusion that "most undefined behaviors
are very easy to debug"? You don't know how much time it took the user
to reduce 100 KLOC to simple test cases of these 69 dups of PR  21920.
Once the test case is reduced, it is pretty trival to understand it.

> In fact if you look at GCC, it is the only compiler were people run into
> violating C/C++
> aliasing rules beause we use them to check if two addresses can alias.
> This is unlike almost all other 
> compilers which represent their aliasing info as base+address and they only
> invoke the C/C++ aliasing 
> rules when they cannot figure out if they point to the same address.

The first time I got into trouble with aliasing rules was with xlC, not
with gcc. It was in template inline code (STL allocators).

Trivial base+address would have detected that 
free_list_element* and T* alias (they were copies of one another!).
Because I used a temporary copy of u.next_free_element, instead of
accessing u.next_free_element directly, xlC would think that the
pointers do not alias.

Now since most normal people don't write STL allocators at their leisure,
they have less chance to bump into such a complex scenario as I did.

The aliasing rules were violated only due to inlining, a very difficult
thing for a user to spot. I had to debug PPC assembly code to figure out
what went wrong (remember that debugging with -O2 has a very limited
utility). And yes, I was aware of the aliasing rules, just make a subtle
mistake. Would I not have been aware of the aliasing rules, then it would
take me 3 days instead of 1 to analyze it. I could I reduce my 100 KLOC
(no valgrind for AIX to find this memory corruption) to 15 trival lines,
which any competent C/C++ programmer would find to be invalid.

Please, pretty please, take into account that the distance between the C++
text, and assembly (and probably GIMPLE) is significantly greater
than that of C, due to generic programming. It is much harder to analyze
the subtalties of undefined behavior in C++.
Comment 24 Paolo Carlini 2010-02-18 22:03:05 UTC
Richard, can you comment on this issue? Do you think it's currently correct to have numeric_limits<>:is_modulo == true for all our signed integral types? We are not making any progress on this issue :(
Comment 25 Richard Biener 2010-02-19 10:22:06 UTC
(In reply to comment #24)
> Richard, can you comment on this issue? Do you think it's currently correct to
> have numeric_limits<>:is_modulo == true for all our signed integral types? We
> are not making any progress on this issue :(

Well, it's certainly not 100% correct to claim signed ints have modulo semantics.
I don't know if it is helpful to say numeric_limits<>:is_modulo == false to
our users.

It would probably be useful to add a preprocessor macro when -fwrapv is
in effect.
Comment 26 Paolo Carlini 2010-02-19 10:30:40 UTC
Ah, thanks, that pre-processor macro idea looks very nice to me. Let's see what people think, and in case, I'll take  care of that as soon as 4.5 branches.
Comment 27 joseph@codesourcery.com 2010-02-19 11:13:34 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is
 inconsistent with gcc

The issue with -fwrapv not making INT_MIN / -1 and INT_MIN % -1 have 
modulo semantics is discussed in bug 30484.

Comment 28 Paolo Carlini 2010-02-19 11:18:57 UTC
Thanks Joseph. Indeed, I remember well when my friend Roberto opened that issue, but didn't expect it was still open ;)
Comment 29 Paolo Carlini 2010-02-19 11:26:10 UTC
So, if I understand correctly the dependencies, I think we should mark this as blocked by 30484: when -fwrapv makes INT_MIN / -1 and INT_MIN % -1 have modulo semantics we can conditionalize numeric_limits<signed>::is_modulo on the new pre-processor macro.
Comment 30 Michael Veksler 2010-02-21 07:52:47 UTC
(In reply to comment #25)
> It would probably be useful to add a preprocessor macro when -fwrapv is
> in effect.
> 
Wouldn't it be a violation of the one definition rule (ODR), 
when one translation unit is compiled with -fwrapv and another without?
In that case this would be a regression.
Comment 31 Paolo Carlini 2010-02-21 09:51:29 UTC
(In reply to comment #30)
> Wouldn't it be a violation of the one definition rule (ODR), 
> when one translation unit is compiled with -fwrapv and another without?
> In that case this would be a regression.

I don't know if this comes as a surprise to you, but in the library we have *tons* of potential violations of this type, due to command line switches, not just the big cases like debug-mode, but also rtti, exceptions, etc. That said, if people prefer a false value, always, just tell me and let's close this PR because I don't think the compiler is going to change any time soon to warrant a true value irrespective of any command line option ...

Comment 32 Michael Veksler 2010-02-21 12:44:14 UTC
(In reply to comment #31)
> (In reply to comment #30)
> > Wouldn't it be a violation of the one definition rule (ODR), 
> > when one translation unit is compiled with -fwrapv and another without?
> > In that case this would be a regression.
> 
> I don't know if this comes as a surprise to you, but in the library we have
> *tons* of potential violations of this type, due to command line switches, not
> just the big cases like debug-mode, but also rtti, exceptions, etc. That said,
> if people prefer a false value, always, just tell me and let's close this PR
> because I don't think the compiler is going to change any time soon to warrant
> a true value irrespective of any command line option ...

If this hazard is so prevalent shouldn't it deserve a separate PR?
If a method or function depend on a flag or macro then it can be handled
by overloading and specialization without ODR violation.

When this hazard is, like in this case, appears in a constant then things
are more difficult. An unexpected behavior may be observed when is_modulo
is passed by reference, and I don't see what can be done in this case,
not in 100% of the scenarios. Even if GCC annotates the two different 
variants of is_modulo differently, such that there will be two different
physical allocations of is_modulo, it will still be possible to get to
some misbehavior in weird cases. Oh well...
Comment 33 Paolo Carlini 2010-02-21 12:53:48 UTC
(In reply to comment #32)
> If this hazard is so prevalent shouldn't it deserve a separate PR?
> If a method or function depend on a flag or macro then it can be handled
> by overloading and specialization without ODR violation.

It would be closed immediately as WONTFIX, to be clear. There is nothing we can do in the foreseeable future, it's **everywhere** and totally unfixable without breaking ABI and much more than that.

> When this hazard is, like in this case, appears in a constant then things
> are more difficult. An unexpected behavior may be observed when is_modulo
> is passed by reference, and I don't see what can be done in this case,
> not in 100% of the scenarios. Even if GCC annotates the two different 
> variants of is_modulo differently, such that there will be two different
> physical allocations of is_modulo, it will still be possible to get to
> some misbehavior in weird cases. Oh well...

I see that I'm not going to work on it. Actually, I'll wait one month or so, and then if I will not get at least one concrete proposal, I will close this one as WONTFIX.
Comment 34 Richard Biener 2010-02-21 13:04:46 UTC
(In reply to comment #33)
> (In reply to comment #32)
> > If this hazard is so prevalent shouldn't it deserve a separate PR?
> > If a method or function depend on a flag or macro then it can be handled
> > by overloading and specialization without ODR violation.
> 
> It would be closed immediately as WONTFIX, to be clear. There is nothing we can
> do in the foreseeable future, it's **everywhere** and totally unfixable without
> breaking ABI and much more than that.
> 
> > When this hazard is, like in this case, appears in a constant then things
> > are more difficult. An unexpected behavior may be observed when is_modulo
> > is passed by reference, and I don't see what can be done in this case,
> > not in 100% of the scenarios. Even if GCC annotates the two different 
> > variants of is_modulo differently, such that there will be two different
> > physical allocations of is_modulo, it will still be possible to get to
> > some misbehavior in weird cases. Oh well...
> 
> I see that I'm not going to work on it. Actually, I'll wait one month or so,
> and then if I will not get at least one concrete proposal, I will close this
> one as WONTFIX.

Or suspend it.  I think this warrants a defect report anyway since I think
is_modulo was intended to describe CPU behavior as otherwise defining
signed integer overflow as undefined in one piece of the standard and
then requiring a sane answer for is_modulo in another part sounds
like a conflict of interest.
Comment 35 Michael Veksler 2010-02-21 13:33:38 UTC
(In reply to comment #34)
> (In reply to comment #33)
> > (In reply to comment #32)
> > > If this hazard is so prevalent shouldn't it deserve a separate PR?
> > > If a method or function depend on a flag or macro then it can be handled
> > > by overloading and specialization without ODR violation.
> > 
> > It would be closed immediately as WONTFIX, to be clear. There is nothing we can
> > do in the foreseeable future, it's **everywhere** and totally unfixable without
> > breaking ABI and much more than that.

Yes, it is a big task which is probably not worth it, but is it unfixable?
Is it possible that these ODR violations will ever be translated to security
issues? For example, the data structure is updated in one translation 
unit in a way that causes memory corruption when accessed by a different
definition in a different translation unit? This is all theoretic but
as history has shown, theoretic security threats become real - eventually.

> > I see that I'm not going to work on it. Actually, I'll wait one month or so,
> > and then if I will not get at least one concrete proposal, I will close this
> > one as WONTFIX.
> 
> Or suspend it.  I think this warrants a defect report anyway since I think
> is_modulo was intended to describe CPU behavior as otherwise defining
> signed integer overflow as undefined in one piece of the standard and
> then requiring a sane answer for is_modulo in another part sounds
> like a conflict of interest.
> 
Maybe it is better to make things clear, but I am not so sure of the 
outcome. I think that is_modulo is meaningful only if it is true, otherwise,
when false user's code can't rely on any specific behavior. 
Maybe is_modulo=false is the right answer even for -fwrapv after all.
Comment 36 rguenther@suse.de 2010-02-21 13:37:11 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is
 inconsistent with gcc

On Sun, 21 Feb 2010, veksler at il dot ibm dot com wrote:

> > Or suspend it.  I think this warrants a defect report anyway since I think
> > is_modulo was intended to describe CPU behavior as otherwise defining
> > signed integer overflow as undefined in one piece of the standard and
> > then requiring a sane answer for is_modulo in another part sounds
> > like a conflict of interest.
> > 
> Maybe it is better to make things clear, but I am not so sure of the 
> outcome. I think that is_modulo is meaningful only if it is true, otherwise,
> when false user's code can't rely on any specific behavior. 
> Maybe is_modulo=false is the right answer even for -fwrapv after all.

Or maybe is_modulo=true has only practical meaning for code that
doesn't invoke undefined behavior anyway (like adding two
signed integers when the operation will overflow).  That also
avoids the /-1 and %-1 issues.

It really depends on how is_modulo was supposed to be used.

Richard.
Comment 37 Gabriel Dos Reis 2010-02-21 18:04:18 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is 
	inconsistent with gcc

On Sun, Feb 21, 2010 at 7:04 AM, rguenth at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:

> Or suspend it.  I think this warrants a defect report anyway since I think
> is_modulo was intended to describe CPU behavior as otherwise defining
> signed integer overflow as undefined in one piece of the standard and
> then requiring a sane answer for is_modulo in another part sounds
> like a conflict of interest.

is_modulo is intended to describe the implementation's choice, not necessarily
the CPU behaviour (which the implementation may choose to mask or not.)

The issue here is that GCC does not always deliver the CPU behaviour, so it
is more a problem with GCC than with the standard.

Users who want to make incompatible assumptions about types in the same program
deserve what they get.
Comment 38 Michael Veksler 2010-02-21 18:20:17 UTC
(In reply to comment #37)
 
> is_modulo is intended to describe the implementation's choice, not necessarily
> the CPU behaviour (which the implementation may choose to mask or not.)
> 
> The issue here is that GCC does not always deliver the CPU behaviour, so it
> is more a problem with GCC than with the standard.
> 
> Users who want to make incompatible assumptions about types in the same program
> deserve what they get.
> 

This is not that simple. libstdc++.so is out of user's control,
libkde.so (or whatever its name is) is out of user's control.

So if -fwrapv affects the definition of is_modulo then:

If libstdc++.so is compiled without -fwrapv, then the user can't 
use -fwrapv without a potential ODR violation. What if libkde.so,
which is supplied by a third party, was compiled with -fwrapv, while
libsdc++.so was compiled by some sysadmin, accessible through NFS.
They must be compiled with exactly the same flags, both out of user's control,
otherwise they'll cause an ODR violation.

Having is_modulo lie looks not as bad as it initially seemed. At least I have
the option to ignore is_modulo altogether without going to the land of
undefined behavior.
Comment 39 Gabriel Dos Reis 2010-02-21 18:50:21 UTC
Subject: Re:  numeric_limits<signed>::is_modulo is 
	inconsistent with gcc

On Sun, Feb 21, 2010 at 12:20 PM, veksler at il dot ibm dot com
<gcc-bugzilla@gcc.gnu.org> wrote:
> ------- Comment #38 from veksler at il dot ibm dot com  2010-02-21 18:20 -------
> (In reply to comment #37)
>
>> is_modulo is intended to describe the implementation's choice, not necessarily
>> the CPU behaviour (which the implementation may choose to mask or not.)
>>
>> The issue here is that GCC does not always deliver the CPU behaviour, so it
>> is more a problem with GCC than with the standard.
>>
>> Users who want to make incompatible assumptions about types in the same program
>> deserve what they get.
>>
>
> This is not that simple. libstdc++.so is out of user's control,
> libkde.so (or whatever its name is) is out of user's control.

Agreed.

>
> So if -fwrapv affects the definition of is_modulo then:
>

so the issue then is that you get two people who make incompatible
assumptions about the same type for the same program.  If that is
case, I'm not sure what libstdc++ would have to do.  After all, the resulting
program is out of the control of libstdc++.

> If libstdc++.so is compiled without -fwrapv, then the user can't
> use -fwrapv without a potential ODR violation.

agreed.

> What if libkde.so,
> which is supplied by a third party, was compiled with -fwrapv, while
> libsdc++.so was compiled by some sysadmin, accessible through NFS.

agreed.

> They must be compiled with exactly the same flags, both out of user's control,
> otherwise they'll cause an ODR violation.
>
> Having is_modulo lie looks not as bad as it initially seemed. At least I have
> the option to ignore is_modulo altogether without going to the land of
> undefined behavior.

my conclusion would be that is_modulo is not the right interface to
test whether a component in the program was compiled with -fwrapv.
Comment 40 Marc Glisse 2012-02-29 12:32:10 UTC
I haven't seen it mentioned in the discussion here, but in C++11, the definition of is_modulo was clarified as:

"True if the type is modulo. A type is modulo if, for any operation involving +, -, or * on values of that type whose result would fall outside the range [min(),max()], the value returned differs from the true value by an integer multiple of max() - min() + 1."

Do people have objections to switching numeric_limits<signed>::is_modulo to false (setting it to true when -fwrapv is used can still be discussed afterwards)?
Comment 41 Gabriel Dos Reis 2012-02-29 13:36:59 UTC
(In reply to comment #40)
> I haven't seen it mentioned in the discussion here, but in C++11, the
> definition of is_modulo was clarified as:
> 
> "True if the type is modulo. A type is modulo if, for any operation involving
> +, -, or * on values of that type whose result would fall outside the range
> [min(),max()], the value returned differs from the true value by an integer
> multiple of max() - min() + 1."
> 
> Do people have objections to switching numeric_limits<signed>::is_modulo to
> false (setting it to true when -fwrapv is used can still be discussed
> afterwards)?


I agree with this.  Thanks, Marc.
Comment 42 paolo@gcc.gnu.org 2012-04-29 09:25:24 UTC
Author: paolo
Date: Sun Apr 29 09:25:17 2012
New Revision: 186944

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186944
Log:
2012-04-29  Marc Glisse  <marc.glisse@inria.fr>

	PR libstdc++/22200
    	* include/std/limits (numeric_limits<>::is_modulo): False for
    	signed types.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/limits
Comment 43 Marc Glisse 2012-05-05 01:11:47 UTC
(In reply to comment #25)
> It would probably be useful to add a preprocessor macro when -fwrapv is
> in effect.

What would be the preferred form, macros __GCC_WRAPV, __GCC_TRAPV, __GCC_STRICT_OVERFLOW, etc, defined only when the matching flag is passed? Or maybe a macro __GCC_INTEGER_OVERFLOW that is 0 for undefined, 1 for wrapping, 2 for trapping, etc?

Maybe I should file this as a different PR? adding the macros doesn't mean we have to use them in is_modulo. By the way, for is_modulo, we could probably arrange so that the values don't have to be in libstdc++.so, if that helps.