Bug 233506 - devel/llvm70: (and prev versions) LLVM assertions are enabled unintentionally
Summary: devel/llvm70: (and prev versions) LLVM assertions are enabled unintentionally
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Brooks Davis
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-25 15:43 UTC by Val Packett
Modified: 2019-01-17 23:33 UTC (History)
5 users (show)

See Also:
bugzilla: maintainer-feedback? (brooks)
jbeich: maintainer-feedback? (kde)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Val Packett 2018-11-25 15:43:36 UTC
LLVM's CMake build only enables assertions in Debug builds of LLVM.

The FreeBSD Ports do not set the LLVM_ENABLE_ASSERTIONS CMake variable.

LLVM relies on CMake to automatically add -DNDEBUG in non-Debug builds.

However, something — looks like our *FLAGS environment variables — overrides CMake's flags and -DNDEBUG is lost.

Compare: (in CMakeCache)

CMAKE_CXX_FLAGS_RELEASE:STRING=-O2 -pipe -fstack-protector -isystem /usr/local/include -fno-strict-aliasing  -isystem /usr/local/include

CMAKE_CXX_FLAGS_RELWITHDEBINFO:STRING=-O2 -g -DNDEBUG

So we're shipping debug assertions in production, and looks like not intentionally!!

This has exposed a few easily fixable bugs in some tools (e.g. Mull), but it looks like there are some clang assertions (e.g. https://github.com/Andersbakken/rtags/issues/586 — I'm seeing the same one in devel/ccls) that no one wants to fix. (And they are deep in clang, not in the user of libclang.)


// As a workaround, we can add something like:

CFLAGS+=-DNDEBUG
CXXFLAGS+=-DNDEBUG
Comment 1 Dimitry Andric freebsd_committer freebsd_triage 2018-12-08 11:35:30 UTC
As far as I remember, the earlier llvm/clang ports just had an end-user configurable option to enable or disable asssertions.  This seems to have been removed with the restructuring towards the current state of the ports.

I am unsure as to why the option was removed, but I would suggest that it be re-added, with the default set to OFF.
Comment 3 commit-hook freebsd_committer freebsd_triage 2019-01-15 16:19:58 UTC
A commit references this bug:

Author: brooks
Date: Tue Jan 15 16:19:34 UTC 2019
New revision: 490385
URL: https://svnweb.freebsd.org/changeset/ports/490385

Log:
  Update to a new snapshot.

  Add -DNDEBUG to CFLAGS/CXXFLAGS to prevent USES=cmake from accidentally
  reenabling assertions. [0]

  This should fix i386 builds.

  PR:		233506 [0]

Changes:
  head/devel/llvm-devel/Makefile
  head/devel/llvm-devel/Makefile.snapshot
  head/devel/llvm-devel/distinfo
  head/devel/llvm-devel/pkg-plist
Comment 4 commit-hook freebsd_committer freebsd_triage 2019-01-17 23:29:41 UTC
A commit references this bug:

Author: brooks
Date: Thu Jan 17 23:29:32 UTC 2019
New revision: 490610
URL: https://svnweb.freebsd.org/changeset/ports/490610

Log:
  More throughly disable assertions.  This works around USES=cmake
  overriding the CFLAGS used to build. [0]

  devel/llvm[45]0: Fix build on GCC systems. [1]

  devel/llvm[456]: Avoid realpath calls where possible in the wrapper
  script. [2]

  PR:		233506 [0], 234647 [1], 234937 [1]
  Submitted by:	greg@unrelenting.technology [0], pkubaj@anongoth.pl [1],
  		bdrewery [2]
  Differential Revision:	https://reviews.freebsd.org/D17990

Changes:
  head/devel/llvm40/Makefile
  head/devel/llvm40/files/llvm-wrapper.sh.in
  head/devel/llvm50/Makefile
  head/devel/llvm50/files/llvm-wrapper.sh.in
  head/devel/llvm60/Makefile
  head/devel/llvm60/files/llvm-wrapper.sh.in
  head/devel/llvm70/Makefile
Comment 5 Brooks Davis freebsd_committer freebsd_triage 2019-01-17 23:33:41 UTC
I've applied the proposed fix.  Please reopen this or file a new report if it's not sufficient.