CacheFileUtils.cpp - call of overloaded 'sqrt(uint64_t&)' is ambiguous

RESOLVED FIXED in Firefox 56

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: petr.sumbera, Assigned: michal.novotny)

Tracking

Trunk
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [necko-would-take])

Attachments

(1 attachment)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170608105825

Steps to reproduce:

While building Firefox on Solaris with fix for Bug 1325336:

gmake[3]: Entering directory '/scratch/firefox/obj-x86_64-pc-solaris2.12/netwerk/cache2'
/usr/bin/g++ -std=gnu++11 -o Unified_cpp_netwerk_cache20.o -c -I/scratch/firefox/obj-x86_64-pc-solaris2.12/dist/stl_wrappers -I/scratch/firefox/obj-x86_64-pc-solaris2.12/dist/system_wrappers -include /scratch/firefox/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/scratch/firefox/netwerk/cache2 -I/scratch/firefox/obj-x86_64-pc-solaris2.12/netwerk/cache2 -I/scratch/firefox/netwerk/base -I/scratch/firefox/netwerk/cache -I/scratch/firefox/obj-x86_64-pc-solaris2.12/dist/include  -I/scratch/firefox/obj-x86_64-pc-solaris2.12/dist/include/nspr -I/scratch/firefox/obj-x86_64-pc-solaris2.12/dist/include/nss       -fPIC  -DMOZILLA_CLIENT -include /scratch/firefox/obj-x86_64-pc-solaris2.12/mozilla-config.h -MD -MP -MF .deps/Unified_cpp_netwerk_cache20.o.pp  -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wformat -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe  -g -O -fno-omit-frame-pointer   -Wno-error=shadow  /scratch/firefox/obj-x86_64-pc-solaris2.12/netwerk/cache2/Unified_cpp_netwerk_cache20.cpp
In file included from /scratch/firefox/obj-x86_64-pc-solaris2.12/netwerk/cache2/Unified_cpp_netwerk_cache20.cpp:74:0:
/scratch/firefox/netwerk/cache2/CacheFileUtils.cpp: In member function 'uint32_t mozilla::net::CacheFileUtils::CachePerfStats::MMA::GetStdDev()':
/scratch/firefox/netwerk/cache2/CacheFileUtils.cpp:593:23: error: call of overloaded 'sqrt(uint64_t&)' is ambiguous
   return sqrt(variance);
                       ^
In file included from /usr/include/math.h:17:0,
                 from /scratch/firefox/obj-x86_64-pc-solaris2.12/dist/system_wrappers/math.h:3,
                 from /usr/gcc/5/include/c++/5.4.0/cmath:44,
                 from /scratch/firefox/obj-x86_64-pc-solaris2.12/dist/system_wrappers/cmath:3,
                 from /scratch/firefox/obj-x86_64-pc-solaris2.12/dist/stl_wrappers/cmath:44,
                 from /scratch/firefox/obj-x86_64-pc-solaris2.12/dist/include/mozilla/MathAlgorithms.h:15,
                 from /scratch/firefox/obj-x86_64-pc-solaris2.12/dist/include/nsTArray.h:17,
                 from /scratch/firefox/netwerk/cache2/CacheIOThread.h:11,
                 from /scratch/firefox/netwerk/cache2/CacheFileIOManager.h:8,
                 from /scratch/firefox/netwerk/cache2/CacheFileChunk.h:8,
                 from /scratch/firefox/netwerk/cache2/CacheFile.h:8,
                 from /scratch/firefox/netwerk/cache2/CacheEntry.h:9,
                 from /scratch/firefox/netwerk/cache2/CacheEntry.cpp:6,
                 from /scratch/firefox/obj-x86_64-pc-solaris2.12/netwerk/cache2/Unified_cpp_netwerk_cache20.cpp:2:
/usr/include/iso/math_iso.h:203:21: note: candidate: long double std::sqrt(long double)
  inline long double sqrt(long double __X) { return __sqrtl(__X); }
                     ^
/usr/include/iso/math_iso.h:171:15: note: candidate: float std::sqrt(float)
  inline float sqrt(float __X) { return __sqrtf(__X); }
               ^
/usr/include/iso/math_iso.h:65:15: note: candidate: double std::sqrt(double)
 extern double sqrt __P((double));
               ^
Component: Untriaged → Networking: Cache
Product: Firefox → Core
(Reporter)

Comment 1

a year ago
Something like this helps:

diff -r 6b9895321335 netwerk/cache2/CacheFileUtils.cpp
--- a/netwerk/cache2/CacheFileUtils.cpp Mon Jun 26 08:40:01 2017 -0700
+++ b/netwerk/cache2/CacheFileUtils.cpp Tue Jun 27 08:24:04 2017 +0000
@@ -590,7 +590,7 @@
   }

   variance -= avgSq;
-  return sqrt(variance);
+  return sqrt((long double)variance);
 }

 CachePerfStats::PerfData::PerfData()

Not sure why it's not problem on other systems. I used gcc 5.4. Any comment on this?
Component: Networking: Cache → Untriaged
Flags: needinfo?(michal.novotny)
Product: Core → Firefox
Component: Untriaged → Networking: Cache
Product: Firefox → Core
Whiteboard: [necko-would-take]
(In reply to Petr Sumbera from comment #1)
> Something like this helps:
> 
> diff -r 6b9895321335 netwerk/cache2/CacheFileUtils.cpp
> --- a/netwerk/cache2/CacheFileUtils.cpp Mon Jun 26 08:40:01 2017 -0700
> +++ b/netwerk/cache2/CacheFileUtils.cpp Tue Jun 27 08:24:04 2017 +0000
> @@ -590,7 +590,7 @@
>    }
> 
>    variance -= avgSq;
> -  return sqrt(variance);
> +  return sqrt((long double)variance);
>  }
> 
>  CachePerfStats::PerfData::PerfData()
> 
> Not sure why it's not problem on other systems. I used gcc 5.4. Any comment
> on this?

In http://www.cplusplus.com/reference/cmath/sqrt/ I can see an overloading function

|double sqrt(T x)| is defined for integral types, but in the error message I don't.
(Assignee)

Comment 3

a year ago
Created attachment 8882321 [details] [diff] [review]
fix
Assignee: nobody → michal.novotny
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(michal.novotny)
Attachment #8882321 - Flags: review?(valentin.gosu)
Comment on attachment 8882321 [details] [diff] [review]
fix

Review of attachment 8882321 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/cache2/CacheFileUtils.cpp
@@ +596,1 @@
>  }

should we also moz_assert that variance >= 0? Right now this is completely correct, but future changes could introduce undefined behaviour.
Attachment #8882321 - Flags: review?(valentin.gosu) → review+
> should we also moz_assert that variance >= 0? Right now this is completely
> correct, but future changes could introduce undefined behaviour.

Nevermind. I just noticed variance is unsigned.
(Assignee)

Comment 6

a year ago
(In reply to Valentin Gosu [:valentin] from comment #5)
> > should we also moz_assert that variance >= 0? Right now this is completely
> > correct, but future changes could introduce undefined behaviour.
> 
> Nevermind. I just noticed variance is unsigned.

This is handled at http://searchfox.org/mozilla-central/source/netwerk/cache2/CacheFileUtils.cpp#584
Keywords: checkin-needed

Comment 7

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/50b6fb73c960
CacheFileUtils.cpp - call of overloaded 'sqrt(uint64_t&)' is ambiguous. r=valentin
Keywords: checkin-needed

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/50b6fb73c960
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.