Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 731280 - net-misc/asterisk fails to compile: clang/LLVM
Summary: net-misc/asterisk fails to compile: clang/LLVM
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Jaco Kroon
URL:
Whiteboard:
Keywords: PullRequest
Depends on:
Blocks: systemwide-clang
  Show dependency tree
 
Reported: 2020-07-07 15:16 UTC by Agostino Sarubbo
Modified: 2021-01-22 03:51 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
build.log (build.log,13.94 KB, text/plain)
2020-07-07 15:16 UTC, Agostino Sarubbo
Details
other.tar.bz2 (other.tar.bz2,8.61 KB, application/x-bzip-compressed-tar)
2020-07-07 15:16 UTC, Agostino Sarubbo
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Agostino Sarubbo gentoo-dev 2020-07-07 15:16:39 UTC
https://blogs.gentoo.org/ago/2020/07/04/gentoo-tinderbox/

Issue: net-misc/asterisk fails to compile.
Discovered on: amd64

NOTE:
This machine uses a clang/LLVM toolchain.
If you think that this issue is strictly related to clang/LLVM please block bug 408963. If you think that this issue isstrictly related to the LLD linker, please block bug 731004.
Comment 1 Agostino Sarubbo gentoo-dev 2020-07-07 15:16:45 UTC
Created attachment 648236 [details]
build.log

build log and emerge --info
Comment 2 Agostino Sarubbo gentoo-dev 2020-07-07 15:16:48 UTC
Created attachment 648238 [details]
other.tar.bz2

other logs
Comment 3 Jaco Kroon 2020-07-07 18:55:59 UTC
checking for RAII support... checking for clang -fblocks... configure: error: BlocksRuntime is required for clang, please install libblocksruntime

Happy to make this work if possible, but I'm not sure what's required here.

https://github.com/mackyle/blocksruntime

Seems to explain the issue.  Not sure if either of these USE flags on clang might take care of the issue, and then, how can I enforce that:

 - - default-compiler-rt            : Use compiler-rt instead of libgcc as the default rtlib for
                                      clang
 - - default-libcxx                 : Use libc++ instead of libstdc++ as the default stdlib for
                                      clang
Comment 4 Jaco Kroon 2020-08-21 12:29:03 UTC
Hi Agostino,

Could you please look at and respond on the below?

(In reply to Jaco Kroon from comment #3)
> checking for RAII support... checking for clang -fblocks... configure:
> error: BlocksRuntime is required for clang, please install libblocksruntime
> 
> Happy to make this work if possible, but I'm not sure what's required here.
> 
> https://github.com/mackyle/blocksruntime
> 
> Seems to explain the issue.  Not sure if either of these USE flags on clang
> might take care of the issue, and then, how can I enforce that:
> 
>  - - default-compiler-rt            : Use compiler-rt instead of libgcc as
> the default rtlib for
>                                       clang
>  - - default-libcxx                 : Use libc++ instead of libstdc++ as the
> default stdlib for
>                                       clang
Comment 5 Agostino Sarubbo gentoo-dev 2020-08-27 18:53:49 UTC
Hello Jaco,

I'm the reporter of something that runs automatically on a machine. If you are unsure about the solution I'd suggest to ask on the gentoo-dev mailing list.
Comment 6 Fabian Groffen gentoo-dev 2020-08-28 06:32:00 UTC
blocks is a "vector feature" from Apple, Clang supports this because Apple systems use Clang as system compiler (and the system headers use blocks features)

In this case, I cannot imagine Asterisk to /need/ blocks support, it probably just wants to exploit it whenever it is available.  That said, the configure logic probably dies at this point because it assumes if the compiler accepts -fblocks, the final linking also succeeds.  Perhaps you can
1) either disable the entire blocks check (blocksruntime isn't in Gentoo's shipping, and I don't think there's much interest for it -- fair)
2) extend the check to disable blocks support when linking failed after compiler support was detected.
Comment 7 Jaco Kroon 2020-08-28 06:45:53 UTC
(In reply to Fabian Groffen from comment #6)
> blocks is a "vector feature" from Apple, Clang supports this because Apple
> systems use Clang as system compiler (and the system headers use blocks
> features)
> 
> In this case, I cannot imagine Asterisk to /need/ blocks support, it
> probably just wants to exploit it whenever it is available.  That said, the
> configure logic probably dies at this point because it assumes if the
> compiler accepts -fblocks, the final linking also succeeds.  Perhaps you can
> 1) either disable the entire blocks check (blocksruntime isn't in Gentoo's
> shipping, and I don't think there's much interest for it -- fair)
> 2) extend the check to disable blocks support when linking failed after
> compiler support was detected.

I tend to agree with you on this, from the RAII m4 file (./autoconf/ast_check_raii.m4):

Lines 10 to 15, check for clang vs everything else.

If NOT CLANG, lines 19 to 30, no possible failure, but it tries to use -fnested-functions if possible.

Lines 35 to 49 ... does a test compile with -lfblocks, failing that -fblocks -lBlocksRuntime, and failing that fails, if I'm understanding you correctly, I reckon just nuke the AC_MSG_ERROR on line 45 and explicitly set set AST_CLANG_BLOCKS_LIBS and AST_CLANG_BLOCKS ="", and we test that?

If that works, push it upstream.
Comment 8 Fabian Groffen gentoo-dev 2020-08-28 06:55:35 UTC
I think turning the AC_MSG_ERROR into AC_MSG_WARN and indeed zapping the vars should be acceptable for upstream, since it wouldn't not disable the feature on macOS, yet allow the build to continue on other systems using Clang.

I hope this helps/works.
Comment 9 Jaco Kroon 2020-08-28 08:20:54 UTC
(In reply to Fabian Groffen from comment #8)
> I think turning the AC_MSG_ERROR into AC_MSG_WARN and indeed zapping the
> vars should be acceptable for upstream, since it wouldn't not disable the
> feature on macOS, yet allow the build to continue on other systems using
> Clang.
> 
> I hope this helps/works.

This indeed does.  Assuming that the sys-libs/blocksruntime build from Sergei makes it into the tree, is there a way to set a dependency based on the toolchain in use?

Something like:

{B,R}DEPEND="if_using_clang? ( sys-libs/blocksruntime )"

Based on a quick grep, it looks like clang? ( sys-libs/blocksruntime ) may do the trick?  But I'll probably double that into:

clang? ( fblocks? ( sys-libs/blocksruntime )) depending on whether or not I manage to modify configure.

I already have clang installed ... will figure out how to get asterisk compiled with clang whilst still using gcc for everything else.
Comment 10 Fabian Groffen gentoo-dev 2020-08-28 08:27:59 UTC
I disagree a bit with what Sergei did.

What should be done IMO is the same as we do for OpenMP support.

1. global USE-flag blocks
2. clang[blocks] should pull in the blocksruntime
3. asterix[blocks] should depend on clang[blocks]
4. the blocks check should be made a enable/disable feature, but if that's too much, or gets resistance from upstream, USE=!blocks should ensure the configure script doesn't even try to enable -fblocks (e.g. no unrecorded dependency on blocksruntime would be possible).

(little caveat, since GCC doesn't support blocks syntax [yet], the flag should probably just pull in Clang, unlike with USE=openmp)
Comment 11 Fabian Groffen gentoo-dev 2020-08-28 08:31:58 UTC
Note that I think we should not start requiring Clang for packages just because they /can/ use blocks support.  It's kind of a huge hassle to compile an llvm toolchain first just to compile a package that would also work fine with GCC.

If there's additional functionality exposed due to blocks usage, then it must be a USE-flag choice, which would be USE=blocks as I mentioned before.
Comment 12 Jaco Kroon 2020-08-28 09:17:43 UTC
(In reply to Fabian Groffen from comment #10)
> I disagree a bit with what Sergei did.
> 
> What should be done IMO is the same as we do for OpenMP support.
> 
> 1. global USE-flag blocks
> 2. clang[blocks] should pull in the blocksruntime

> 3. asterix[blocks] should depend on clang[blocks]

asterisk works just fine with gcc.  This means there really is three choices at this stage, and I suspect other packages have similar issues:

1.  gcc
2.  clang[!blocks]
3.  clang[blocks]

Frankly, I'm inclined to just let ./configure figure out, but the dep on blocksruntime is an interesting one.

> 4. the blocks check should be made a enable/disable feature, but if that's
> too much, or gets resistance from upstream, USE=!blocks should ensure the
> configure script doesn't even try to enable -fblocks (e.g. no unrecorded
> dependency on blocksruntime would be possible).

And I'm on the "if it's available, use it" page, but how does one deal with the automagic dependency?  And your suggestion of putting that as a USE flag on clang actually makes sense, then asterisk could have a depend on clang that simply states:

IUSE+="clang blocks"

clang? ( sys-devel/clang[?blocks] )

That, however, feels like a BDEPEND, and the RDEPEND for clang && blocks is *probably* only on the blocksruntime library (but I'm not sure how the libc et all works here as I suspect it still refers back to glibc).

> (little caveat, since GCC doesn't support blocks syntax [yet], the flag
> should probably just pull in Clang, unlike with USE=openmp)

(In reply to Fabian Groffen from comment #11)
> Note that I think we should not start requiring Clang for packages just
> because they /can/ use blocks support.  It's kind of a huge hassle to
> compile an llvm toolchain first just to compile a package that would also
> work fine with GCC.

The double negation had me there for a bit.  I fully agree.

> If there's additional functionality exposed due to blocks usage, then it
> must be a USE-flag choice, which would be USE=blocks as I mentioned before.

There is not.  It's more of an internal optimization from what I can tell.  Basically if it's using CLANG, it WANTS to pass -fblocks [+-lBlocksRuntime] to everything, which simply implies it's using it as an optional extra.

What bugs me, let's say I hack the ./configure simply to change AC_MSG_ERROR to AC_MSG_WARN (which is fair), that'll make it pass, however, now if the blocksruntime is available this will introduce an automagic dependency at BUILD time on blocksruntime, and this is for me more the problematic issue currently.  So I could make this into ./configure --{enable,disable,with,without}-fblocks option that just happens to depend on clang && blocks USE flags.  But really, is clang USE flag the right thing?  Surely, CC= determines the compiler to use, not the USE=clang?
Comment 13 Jaco Kroon 2020-08-28 09:20:22 UTC
Consider this sequence:

1.  USE=blocks emerge clang
2.  USE=-blocks emerge clang
3.  emerge asterisk

Due to the first emerge pulling in BlocksRuntime ... the RUNTIME will be available for asterisk to use.

My implied assumption of course is that -fblocks will still work, but linking will fail due to the lack of runtime in the case where the first emerge was never run.
Comment 14 Jaco Kroon 2020-08-28 09:40:31 UTC
I'm getting really rude now ... triple post, sorry.  Figured I best test my assumption:

jkroon@plastiekpoot ~/src/test $ cat blocks.c 
int main()
{
	^{return 42;}();
}
jkroon@plastiekpoot ~/src/test $ clang -fblocks -c blocks.c 
jkroon@plastiekpoot ~/src/test $ clang -fblocks -o blocks blocks.c 
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: /tmp/blocks-5aa57a.o:(.rodata+0x20): undefined reference to `_NSConcreteGlobalBlock'
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)
[1] jkroon@plastiekpoot ~/src/test $ clang -o blocks blocks.c 
blocks.c:3:2: error: blocks support disabled - compile with -fblocks or pick a deployment target that supports them
        ^{return 42;}();
        ^
1 error generated.
[1] jkroon@plastiekpoot ~/src/test $ 


So my assumption does seem to be correct.  The last compile does hint that a simple configure change may not be sufficient.  So looking at the code in include/asterisk/utils.h:

1097 #if defined(__clang__)
1098 typedef void (^_raii_cleanup_block_t)(void);
1099 static inline void _raii_cleanup_block(_raii_cleanup_block_t *b) { (*b)(); }
1100 
1101 #define RAII_VAR(vartype, varname, initval, dtor)                                                                   \
1102     __block vartype varname = initval;                                                                              \
1103     _raii_cleanup_block_t _raii_cleanup_ ## varname __attribute__((cleanup(_raii_cleanup_block)     ,unused)) =     \
1104         ^{ {(void)dtor(varname);} };
1105 
1106 #elif defined(__GNUC__)
1107 
1108 #define RAII_VAR(vartype, varname, initval, dtor)                              \
1109     auto void _dtor_ ## varname (vartype * v);                                 \
1110     void _dtor_ ## varname (vartype * v) { dtor(*v); }                         \
1111     vartype varname __attribute__((cleanup(_dtor_ ## varname))) = (initval)
1112 
1113 #else
1114     #error "Cannot compile Asterisk: unknown and unsupported compiler."
1115 #endif /* #if __GNUC__ */

Which is the ONLY place I can locate '^{' in the *asterisk* code (there are a few in the third-party/pjproject in .m files ... so since we build pjproject independently I don't think we care here, we might on the pjproject side), we may just need to change that into a kind of "if flocks is available use the current __clang__ variant, else, use the current __GNUC__ variant (I'm not seeing anything gcc specific in that variant, but I only ever really use gcc so I can't say with 100% certainty).

So whilst I believe I understand the problem, and (I believe) I can fix (and submit fix upstream), I would not know how to best deal with ebuild side.  For asterisk I believe the following changes required:

Add an explicit --{enable,disable}-fblocks, logic follows:

if explicit disabled; then
  : # do nothing
else
  test -fblocks options availability, including RunTime requirements.
  if explicitly enable && not available; then
     fail
  fi
  if available; then
    set options to use
  fi
fi

And obviously update the utils.h to only use that syntax if available, probably by way of a HAVE_FBLOCKS option set by the above.

This would then make the ebuild easier in that we can ALWAYS pass --{enable,disable}-fblocks based on USE flag, however, we only really want to allow setting USE=blocks IFF CC=clang (we can pull in the runtime dep based on USE flag.

So the masking of USE=blocks if CC!=clang is the part that I'll need help with.
Comment 15 Fabian Groffen gentoo-dev 2020-08-28 10:39:28 UTC
automagic dependencies are a no-no per Gentoo policies: https://devmanual.gentoo.org/general-concepts/use-flags/index.html

In the case of Asterisk, I think you really don't have to solve this, and just unconditionally disable blocks support.  The gain is probably minimal, and you can always wait for a user to really want its support.  Then is a good time to see if the blocks thing was resolved on a higher level (= clang) already or not.

The question is whether Clang supports nested functions here the way GCC does.

Can you:

  sed -i -e 's:choke:/*let clang pass as gcc*/:' autoconf/ast_check_raii.m4
  eautoreconf

and see if with CC=clang asterisk actually compiles (assuming configure goes to check as if it were using the GCC case now).

If not, then likely if tc-is-clang, you need blocks for raii.
Comment 16 Jaco Kroon 2021-01-11 08:40:03 UTC
Final resolution:  I've pulled in blocksruntime as a dependency for asterisk if CC=clang (by way of USE flag and some pkg_pretend checks).

I will however have to use/stable mask the USE blocks USE flag unless blocksruntime can get stabled for x86 and amd64 soon.  Ideally keyworded for arm, arm64, ppc and ppc64 so that I can drop the package.use.mask values.

Added Sergei since he maintains blocksruntime.  Sergei, would you be able to provide us with your plans and timelines regarding blocksruntime w.r.t. the above?

Long Explanation:

The problem with asterisk is that it needs scoped destructors in "C", as such, it uses __attribute__((cleanup)), but it uses it with wrapper functions to avoid some extra pointer indirection in the destructor (by way of nested functions in gcc, or -fblocks in the case of clang).  I've started inquiring about whether a patch to fix this and rather have the extra indirection (which works on both gcc and clang cleanly without nested functions / -fblocks and is thus a generic solution) would be something that can be considered, but so far it hasn't garnered much interest.  I'll escalate this to the asterisk-dev ML probably.

For us this means we have two options for immediate clang support:

1.  Cook the patch (which affects a HUGE amount of code).  I've counted this at over 125 destructor functions, and over 2000 users of these destructors (which I don't think there should be change involved with at since this is all wrapped up in a very nasty RAII_VAR macro).  As such, a patch like this would be a big one, and be heavy effort to maintain due to large probability of code changes around the changes, or additional users that may need patching too.  Minimum such a patch would affect probably about 500-1000 lines of code (not counting context).  (read:  it must go upstream or not at all.)

2.  Pull in blocksruntime for clang support.  The associated PR does this in the cleanest way that I could come up with currently.  This however means that if we want clang on the asterisk keyworded arches for arm, arm64, ppc and ppc64 we need to see if we can get blocksruntime keyworded for those arches too, and stabled for x86 and amd64.

Option 1 is long term in my opinion a cleaner/better solution, actually dropping potential extra code (slightly smaller binaries, possibly fewer indirect trampoline style calls) and enlarging compiler support for asterisk from not only gcc and clang, but to other compilers too that does support __attribute__((cleanup(...))).  For now there is no real choice but to go with opion 2.
Comment 17 Sergei Trofimovich gentoo-dev 2021-01-11 17:41:04 UTC
(In reply to Jaco Kroon from comment #16)
> Final resolution:  I've pulled in blocksruntime as a dependency for asterisk
> if CC=clang (by way of USE flag and some pkg_pretend checks).
> 
> I will however have to use/stable mask the USE blocks USE flag unless
> blocksruntime can get stabled for x86 and amd64 soon.  Ideally keyworded for
> arm, arm64, ppc and ppc64 so that I can drop the package.use.mask values.
> 
> Added Sergei since he maintains blocksruntime.  Sergei, would you be able to
> provide us with your plans and timelines regarding blocksruntime w.r.t. the
> above?

I don't plan to call stabilization myself as I have no stable tools that rely on sys-libs/blocksruntime. But I don't mind it getting more unstable or stable keywords as a dependency.

I suggest you to file KEYWORDREQ/STABLEREQ bugs either against sys-libs/blocksruntime explicitly or as part of net-misc/asterisk. Word of caution: your bigger blockers might be llvm packages. For example we don't have `ppc` keywords for llvm.
Comment 18 Larry the Git Cow gentoo-dev 2021-01-22 03:51:17 UTC
The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=6202f7bf618faf8a3fb24e6c7b586dd7aad5d144

commit 6202f7bf618faf8a3fb24e6c7b586dd7aad5d144
Author:     Jaco Kroon <jaco@uls.co.za>
AuthorDate: 2021-01-08 18:20:54 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2021-01-22 03:50:57 +0000

    net-misc/asterisk: 16.15.1-r2 version bump.
    
    * clang/LLVM support
    * subslots
    * autoconf 2.70
    * codec2 support
    * drop /var/spool/asterisk from tmpfiles
    * Move check_extra_config (kernel checks) to pkg_pretend.
    * (Hopefully) fix use of $ED vs $D.
    * Other minor non-functional tweaks.
    
    Closes:  https://bugs.gentoo.org/show_bug.cgi?id=731280
    Closes:  https://bugs.gentoo.org/show_bug.cgi?id=750581
    Closes:  https://bugs.gentoo.org/show_bug.cgi?id=763918
    Signed-off-by: Jaco Kroon <jaco@uls.co.za>
    Closes: https://github.com/gentoo/gentoo/pull/18994
    Signed-off-by: Sam James <sam@gentoo.org>

 net-misc/asterisk/asterisk-16.15.1-r2.ebuild       | 322 +++++++++++++++++++++
 .../files/asterisk-16.15.1-r2-autoconf-2.70.patch  |  14 +
 2 files changed, 336 insertions(+)

Additionally, it has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=880e7d3d5531e20e13b35683fa352671bd838726

commit 880e7d3d5531e20e13b35683fa352671bd838726
Author:     Jaco Kroon <jaco@uls.co.za>
AuthorDate: 2021-01-08 18:18:20 +0000
Commit:     Sam James <sam@gentoo.org>
CommitDate: 2021-01-22 03:50:56 +0000

    net-misc/asterisk: 13.18.1-r2 version bump.
    
    * clang/LLVM support
    * subslots
    * autoconf 2.70
    * drop /var/spool/asterisk from tmpfiles
    * Move check_extra_config (kernel checks) to pkg_pretend.
    * (Hopefully) fix use of $ED vs $D.
    * Other minor non-functional tweaks.
    
    Bug:  https://bugs.gentoo.org/show_bug.cgi?id=731280
    Bug:  https://bugs.gentoo.org/show_bug.cgi?id=750581
    Signed-off-by: Jaco Kroon <jaco@uls.co.za>
    Signed-off-by: Sam James <sam@gentoo.org>

 net-misc/asterisk/asterisk-13.38.1-r2.ebuild       | 313 +++++++++++++++++++++
 .../files/asterisk-13.18.1-r2-autoconf-2.70.patch  |  10 +
 net-misc/asterisk/files/asterisk.tmpfiles2.conf    |   1 +
 net-misc/asterisk/metadata.xml                     |   2 +
 profiles/arch/amd64/package.use.mask               |   5 +
 profiles/arch/base/package.use.mask                |   4 +
 profiles/arch/x86/package.use.mask                 |   5 +
 7 files changed, 340 insertions(+)