Bug 224947

Summary: audio/libsidplay: fails to build with clang 6.0 (blocks 8 ports)
Product: Ports & Packages Reporter: Jan Beich <jbeich>
Component: Individual Port(s)Assignee: Jan Beich <jbeich>
Status: Closed FIXED    
Severity: Affects Only Me CC: dim, ehaupt, tijl
Priority: --- Keywords: needs-patch
Version: LatestFlags: tijl: maintainer-feedback+
jbeich: maintainer-feedback? (dim)
jbeich: maintainer-feedback? (ehaupt)
Hardware: Any   
OS: Any   
See Also: https://bugzilla.mozilla.org/show_bug.cgi?id=1329520
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=225307
Bug Depends on:    
Bug Blocks: 224669    

Description Jan Beich freebsd_committer 2018-01-06 06:44:50 UTC
FreeBSD -CURRENT plans to update base Clang to 6.0.0 soon. While testing this port failed to build. The underlying issue appears to be Clang's idiosyncrasy with C++11 attributes. GCC 6+ is unaffected.

$ cat a.cc
extern "C" void exit(int);

#include <stdlib.h>

int main() { return 0; }

$ c++ a.cc
In file included from a.cc:3:
In file included from /usr/include/c++/v1/stdlib.h:94:
/usr/include/stdlib.h:97:1: error: function declared '[[noreturn]]' after its first declaration
_Noreturn void   exit(int);
^
/usr/include/sys/cdefs.h:278:22: note: expanded from macro '_Noreturn'
#define _Noreturn               [[noreturn]]
                                  ^
a.cc:1:17: note: declaration missing '[[noreturn]]' attribute is here
extern "C" void exit(int);
                ^
1 error generated.

===========================================================================
=======================<phase: configure      >============================
===>  Configuring for libsidplay-1.36.59_1
[...]
checking whether exception-handling is supported... no
[...]
checking size of char... configure: error: cannot compute sizeof (char), 77
See `config.log' for more details.
===>  Script "configure" failed unexpectedly.

To reproduce:
$ poudriere jail -cj clang6-amd64 -a amd64 -v projects/clang600-import -m svn+https
$ poudriere bulk -Ctj clang6-amd64 audio/libsidplay

http://package18.nyi.freebsd.org/data/headamd64PR224669-default/2018-01-02_08h32m49s/logs/errors/libsidplay-1.36.59_1.log
Comment 1 Jan Beich freebsd_committer 2018-01-06 07:20:57 UTC
Most configure_error bustage in bug 224669 is actually due to C++11 attributes poisoning.
Comment 2 Dimitry Andric freebsd_committer 2018-01-06 16:59:03 UTC
So this is caused by a bunch of GNU configure declaring their own version of exit(), without any attributes, then our stdlib.h declaring it with attributes, right?

In that case using USE_CXXSTD=gnu++98 might also be an acceptable workaround, since that was the original standard with which those configure scripts were executed.

In C++11 or later, I would indeed say that declaring functions inconsistently should be a cause for errors, or at least warnings.  It is a pity you cannot turn these warnings off, though.
Comment 3 Jan Beich freebsd_committer 2018-01-08 15:32:05 UTC
(In reply to Dimitry Andric from comment #2)
> So this is caused by a bunch of GNU configure declaring their own
> version of exit(), without any attributes, then our stdlib.h
> declaring it with attributes, right?

Tijl, do you know why autoconf doesn't pick up the system declaration? The following snippet suggests the loop should terminate on the first iteration

  # Some people use a C++ compiler to compile C.  Since we use `exit',
  # in C++ we need to declare it.  In case someone uses the same compiler
  # for both compiling C and C++ we need to have the C++ compiler decide
  # the declaration of exit, since it's the most demanding environment.
  [...]
    for ac_declaration in \
     '' \
     'extern "C" void std::exit (int) throw (); using std::exit;' \
     'extern "C" void std::exit (int); using std::exit;' \
     'extern "C" void exit (int) throw ();' \
     'extern "C" void exit (int);' \
     'void exit (int);'
  do
    cat >conftest.$ac_ext <<_ACEOF
  /* confdefs.h.  */
  _ACEOF
  cat confdefs.h >>conftest.$ac_ext
  cat >>conftest.$ac_ext <<_ACEOF
  /* end confdefs.h.  */
  $ac_declaration
  #include <stdlib.h>
  int
  main ()
  {
  exit (42);
    ;
    return 0;
  }
  _ACEOF

> In that case using USE_CXXSTD=gnu++98 might also be an acceptable
> workaround, since that was the original standard with which those
> configure scripts were executed.

Requires checking consumers for regressions.

https://gcc.gnu.org/wiki/Cxx11AbiCompatibility

> In C++11 or later, I would indeed say that declaring functions
> inconsistently should be a cause for errors, or at least warnings.
> It is a pity you cannot turn these warnings off, though.

Why Clang accepts

  #include <stdlib.h>
  extern "C" void exit(int);

but rejects

  extern "C" void exit(int);
  #include <stdlib.h>

Is such an inconsistent behavior really standardized?
Comment 4 Tijl Coosemans freebsd_committer 2018-01-08 16:02:21 UTC
It looks like you can just add USES=autoreconf to the port Makefile to regenerate configure.  The included configure has been generated with an old version of autoconf.
Comment 5 Jan Beich freebsd_committer 2018-01-08 17:21:34 UTC
Indeed, USES=autoreconf works around Clang quirk but requires adjusting any patches against configure or Makefile.in. audio/libsidplay still fails. Not sure about the fix: s/sbyte/ubyte/ or static_cast or -Wno-error=narrowing (e.g., ports r458130).

samples.cpp:80:2: error: constant expression evaluates to 128 which cannot be narrowed to type 'sbyte' (aka 'signed char') [-Wc++11-narrowing]
        0x80,0x91,0xa2,0xb3,0xc4,0xd5,0xe6,0xf7,
        ^~~~
samples.cpp:80:2: note: insert an explicit cast to silence this issue
        0x80,0x91,0xa2,0xb3,0xc4,0xd5,0xe6,0xf7,
        ^~~~
        static_cast<sbyte>( )
[...]

build log: https://ptpb.pw/NPiS
Comment 6 Dimitry Andric freebsd_committer 2018-01-08 20:36:46 UTC
(In reply to Jan Beich from comment #5)
> Indeed, USES=autoreconf works around Clang quirk but requires adjusting any
> patches against configure or Makefile.in. audio/libsidplay still fails. Not
> sure about the fix: s/sbyte/ubyte/ or static_cast or -Wno-error=narrowing
> (e.g., ports r458130).
> 
> samples.cpp:80:2: error: constant expression evaluates to 128 which cannot
> be narrowed to type 'sbyte' (aka 'signed char') [-Wc++11-narrowing]
>         0x80,0x91,0xa2,0xb3,0xc4,0xd5,0xe6,0xf7,
>         ^~~~
> samples.cpp:80:2: note: insert an explicit cast to silence this issue
>         0x80,0x91,0xa2,0xb3,0xc4,0xd5,0xe6,0xf7,
>         ^~~~
>         static_cast<sbyte>( )

Yes, this is pretty annoying, inserting all those static casts would be horrendous. If the warning can be turned off, so much the better.
Comment 7 commit-hook freebsd_committer 2018-01-26 14:40:01 UTC
A commit references this bug:

Author: jbeich
Date: Fri Jan 26 14:39:16 UTC 2018
New revision: 459997
URL: https://svnweb.freebsd.org/changeset/ports/459997

Log:
  audio/libsidplay: unbreak build with Clang 6 (C++14 by default)

  configure:4036: c++ -o conftest -O2 -pipe -O3 -fno-strict-aliasing -fno-omit-frame-pointer -march=native -fstack-protector    -fstack-protector conftest.cc  >&5
  In file included from conftest.cc:32:
  In file included from /usr/include/c++/v1/stdlib.h:94:
  /usr/include/stdlib.h:97:1: error: function declared '[[noreturn]]' after its first declaration
  _Noreturn void   exit(int);
  ^
  /usr/include/sys/cdefs.h:280:22: note: expanded from macro '_Noreturn'
   #define _Noreturn               [[noreturn]]
                                     ^
  conftest.cc:11:6: note: declaration missing '[[noreturn]]' attribute is here
  void exit (int);
       ^
  [...]
  configure:4053: error: cannot compute sizeof (char), 77

  samples.cpp:80:2: error: constant expression evaluates to 128 which cannot be narrowed to type 'sbyte' (aka 'signed char') [-Wc++11-narrowing]
          0x80,0x91,0xa2,0xb3,0xc4,0xd5,0xe6,0xf7,
          ^~~~
  samples.cpp:80:2: note: insert an explicit cast to silence this issue
          0x80,0x91,0xa2,0xb3,0xc4,0xd5,0xe6,0xf7,
          ^~~~
          static_cast<sbyte>( )

  PR:		224947
  Reported by:	pkg-fallout (blocks 8 ports)
  Submitted by:	tijl (autoreconf)
  Approved by:	portmgr blanket

Changes:
  head/audio/libsidplay/Makefile
  head/audio/libsidplay/files/
  head/audio/libsidplay/files/patch-src_samples.cpp
Comment 8 Jan Beich freebsd_committer 2018-01-26 14:44:36 UTC
(In reply to commit-hook from comment #7)
Oops, -O3 .. -fno-omit-frame-pointer -march=native are my local CFLAGS I forgot to strip from commit message.