[Bug libstdc++/67996] New: std::ios_base::seekdir raises -Wswitch with Clang

[Bug libstdc++/67996] New: std::ios_base::seekdir raises -Wswitch with Clang

[kim.grasman at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67996

            Bug ID: 67996
           Summary: std::ios_base::seekdir raises -Wswitch with Clang
           Product: gcc
           Version: 4.6.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kim.grasman at gmail dot com
  Target Milestone: ---

When using libstdc++ with Clang, we can't seem to form a fully-covered switch
over std::ios_base::seekdir:

  void f(std::ios_base::seekdir way)
    switch(way) {
      case std::ios_base::beg:
        // ...
        break;

      case std::ios_base::cur:
        // ...
        break;

      case std::ios_base::end:
        // ...
        break;
    }
  }

...t.cc:87:12: warning: enumeration value
'_S_ios_seekdir_end' not handled in switch [-Wswitch]
                        switch (way)

This looks like it was discussed long ago in #17922, so I don't know if this
has regressed or if it's something about Clang's implementation of this
diagnostic that is different from GCC.

A discussion on the Clang list is available here:
http://article.gmane.org/gmane.comp.compilers.clang.devel/45198/match=overeager

What's the motivation for _S_ios_seekdir_end? Any chance it could be removed?

Thanks!

[Bug libstdc++/67996] std::ios_base::seekdir raises -Wswitch with Clang

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67996

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Kim Gräsman from comment #0)
> What's the motivation for _S_ios_seekdir_end? Any chance it could be removed?

Presumably to make sure the enum type is more than 16-bits, which could be done
with a fixed underlying type in C++11, but not in C++03.

I don't think we should change this just because of a warning. The std::lib is
allowed to define whatever enumerators it wants to (as long as they use
reserved names).
>From gcc-bugs-return-499801-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Sat Oct 17 01:45:35 2015
Return-Path: <gcc-bugs-return-499801-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 4753 invoked by alias); 17 Oct 2015 01:45:34 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 4722 invoked by uid 48); 17 Oct 2015 01:45:30 -0000
From: "ch3root at openwall dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/67999] New: Wrong optimization of pointer comparisons
Date: Sat, 17 Oct 2015 01:45:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: new
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: 5.2.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: ch3root at openwall dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter target_milestone
Message-ID: <bug-67999-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-10/txt/msg01356.txt.bz2
Content-length: 1241

https://gcc.gnu.org/bugzilla/show_bug.cgi?idg999

            Bug ID: 67999
           Summary: Wrong optimization of pointer comparisons
           Product: gcc
           Version: 5.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ch3root at openwall dot com
  Target Milestone: ---

The following program:

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
  size_t len = 0xC0000000; /* 3GB */
  char *buf = malloc(len);
  if (!buf)
    return 1;

  printf("buf       = %p\n", (void *)buf);
  printf("len       = 0x%08zx\n", len);
  printf("buf + len = %p\n", (void *)(buf + len));

  printf("buf + len < buf: ");
  if (buf + len < buf)
    printf("true\n");
  else
    printf("false\n");

  return 0;
}

generates output like this:

$ gcc -m32 -O2 test.c && ./a.out
buf       = 0x3751d008
len       = 0xc0000000
buf + len = 0xf751d008
buf + len < buf: true

The result of the comparison "buf + len < buf" is wrong.

AFAICT the comparison "buf + len < buf" is simplified into "len < 0" where
"len" is treated as a signed value which is wrong when "len" is of an unsigned
type and has a large value.

[Bug libstdc++/67996] std::ios_base::seekdir raises -Wswitch with Clang

[kim.grasman at gmail dot com]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 4981 bytes --]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67996

--- Comment #2 from Kim Gräsman <kim.grasman at gmail dot com> ---
Thanks, I figured that was why.

The warning is not a problem if I'm using <ios> directly, then I can just
disable it locally.

But we ran into this with a third-party library having the code above in one of
their headers. This makes it nigh-impossible for us to enable -Wswitch for our
own code, as the third-party header could be indirectly included in any
translation unit.

  ios:
     enum seekdir { beg, cur, end, _S_ios_seekdir_end };

  third-party.h:
    switch(seekdir) {
    case beg: break;
    case cur: break;
    case end: break;
    }

  us.cpp:
    #include "third-party.h"  // WARN

I wasn't sure if the extra enumerator was allowed by the standard, so thanks
for confirming that.

We've now worked around it by patching the third-party to use an if/else chain
instead, but I thought I'd raise it anyway because it creates a messy situation
when the warning "leaks" like this.
>From gcc-bugs-return-499805-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Sat Oct 17 06:47:47 2015
Return-Path: <gcc-bugs-return-499805-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 90335 invoked by alias); 17 Oct 2015 06:47:46 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 90304 invoked by uid 48); 17 Oct 2015 06:47:43 -0000
From: "thaines.astro at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c/68000] New: Suboptimal ternary operator codegen
Date: Sat, 17 Oct 2015 06:47:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: new
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c
X-Bugzilla-Version: 5.1.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: thaines.astro at gmail dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Resolution:
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_id short_desc product version bug_status bug_severity priority component assigned_to reporter target_milestone attachments.created
Message-ID: <bug-68000-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2015-10/txt/msg01360.txt.bz2
Content-length: 2067

https://gcc.gnu.org/bugzilla/show_bug.cgi?idh000

            Bug ID: 68000
           Summary: Suboptimal ternary operator codegen
           Product: gcc
           Version: 5.1.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: thaines.astro at gmail dot com
  Target Milestone: ---

Created attachment 36534
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id6534&actioníit
Preprocessed C code for "Suboptimal ternary operator codegen"

I think this problem is likely related to bug 23286, bug 56096, bug 64524, and
possibly bug 67879.

struct pair8 { uint8_t x,y; };
uint8_t foo_manual_hoist(struct pair8 *p) {
        const uint8_t a = p->x + 1;
        return a == p->y ? 0 : a;
}
uint8_t foo(struct pair8 *p) {
        return p->x + 1 == p->y ? 0 : p->x + 1;
}
uint8_t foo_if(struct pair8 *p) {
        uint8_t a = p->x + 1;
        if (a == p->y)
                a = 0;
        return a;
}
int main() {}

Under gcc-5.1, gcc -m64 -march=native -O3 -g -S -masm=intel -o test.asm test.c
produces

foo_manual_hoist:
    movzx eax, BYTE PTR [rdi+1]
    mov   edx, 0
    add   eax, 1
    cmp   al, BYTE PTR [rdi+2]
    cmove eax, edx
    ret
foo:
    movzx edx, BYTE PTR [rdi+1]
    movzx ecx, BYTE PTR [rdi+2]
    mov   eax, edx   // weird!
    add   edx, 1
    add   eax, 1
    cmp   edx, ecx
    mov   edx, 0
    cmove eax, edx
    ret
foo_if:
    movzx eax, BYTE PTR [rdi+1]
    mov   edx, 0
    add   eax, 1
    cmp   al, BYTE PTR [rdi+2]
    cmove eax, edx
    ret

As expect, the ternary version with the manual hoist of the common
subexpression produces identical codegen as the if version with the same hoist.
What is unexpected, is the duplication of the common subexpression for the
"bare" ternary version (line 3 of the disassembly of foo). It is clear that the
compiler sees the expressions to be the same, but treats them separately
anyway. As an aside, clang-3.6 produces identical code for all three versions.

[Bug libstdc++/67996] std::ios_base::seekdir raises -Wswitch with Clang

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67996

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Then maybe the warning needs to be smarter, e.g. not warn for enumerators
defined in system headers using the implementation's reserved names.

In any case, it's not a libstdc++ problem, our code is correct, and GCC doesn't
issue any warning.