Bug 245075 - www/varnish4: Fails to build with clang 10; "error: implicit conversion from 'long' to 'double' changes value from 9223372036854775807 to 9223372036854775808"
Summary: www/varnish4: Fails to build with clang 10; "error: implicit conversion from ...
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: Mark Felder
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-26 15:24 UTC by Laurence 'GreenReaper' Parry
Modified: 2020-05-18 17:32 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (feld)


Attachments
Failing portion of compilation log (2.53 KB, text/plain)
2020-03-26 15:24 UTC, Laurence 'GreenReaper' Parry
no flags Details
Patch for lib/libvmod_std/vmod_std_conversions.c (1.14 KB, patch)
2020-03-26 15:25 UTC, Laurence 'GreenReaper' Parry
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laurence 'GreenReaper' Parry 2020-03-26 15:24:08 UTC
Created attachment 212724 [details]
Failing portion of compilation log

After installing devel/llvm10 from a port, I tried to recompile our server's software with it, especially varnish as it recently fell over when we didn't (it relies on the version of llvm it is compiled with being there on startup, and we'd removed it).

Unfortunately there is a new error (from a new warning) - basically LONG_MAX overflows the value that can be precisely represented within a double by about 11 bits of precision.

The master version has a comment regarding this:
https://raw.githubusercontent.com/varnishcache/varnish-cache/master/lib/libvmod_std/vmod_std_conversions.c
"
 * technically, as our VCL_INT is int64_t, its limits are INT64_MIN/INT64_MAX.
 *
 * Yet, for conversions, we use VNUMpfx with a double intermediate, so above
 * 2^53 we see rounding errors. In order to catch a potential floor rounding
 * error, we make our limit 2^53-1
"

This appears to have been applied in Commit 0f7f757c "sensible limits for VCL_INT and VCL_BYTES"
https://code.uplex.de/varnishcache/varnish-cache/commit/0f7f757c478351e7a30c9820f7542b868743b7b8
but it was later moved purely into vmod_std_conversions.c

varnish 4.1 seems to use LONG_MAX/MIN rather than INT64_MAX/MIN and the only functions concerned are vmod_real2integer and vmod_time2integer because vmod_integer concerns an int, not a long, so it doesn't have that check. (This change was made in https://varnish-cache.org/lists/pipermail/varnish-commit/2012-October/009180.html - "Change VCL::INT from C::int to C::long to gain more range on 64bit architectures.")

I created a patch. Adding this to files/ adjusting the Makefile accordingly:
EXTRA_PATCHES+= ${FILESDIR}/fix-long-to-double-rounding.patch
causes compilation to succeed again with clang10.

Don't know if you want to apply it, but varnish 4.1 is still supported, I'm using it, and in fact there's a new version, 4.1.11, released February 11:
https://github.com/varnishcache/varnish-cache/blob/4.1/doc/changes.rst
Comment 1 Laurence 'GreenReaper' Parry 2020-03-26 15:25:26 UTC
Created attachment 212725 [details]
Patch for lib/libvmod_std/vmod_std_conversions.c
Comment 2 Mark Felder freebsd_committer freebsd_triage 2020-05-18 17:32:46 UTC
Thanks for the backported patch and update!
Comment 3 commit-hook freebsd_committer freebsd_triage 2020-05-18 17:32:47 UTC
A commit references this bug:

Author: feld
Date: Mon May 18 17:32:24 UTC 2020
New revision: 535759
URL: https://svnweb.freebsd.org/changeset/ports/535759

Log:
  www/varnish4: Update to 4.1.11

  Changelog: https://varnish-cache.org/releases/rel4.1.11.html

  Also backport change required for compiling with LLVM 10.

  PR:		245075

Changes:
  head/www/varnish4/Makefile
  head/www/varnish4/distinfo
  head/www/varnish4/files/patch-lib_libvmod__std_vmod__std__conversions.c