Bug 241588

A little background[1] on the topic.
 
"Executing a program on a Pentium III processor enables the FTZ flag, but not DAZ."[2]
 
DAZ is not supported by Pentium 3[3] nor VIA Nehemiah[3]:
(This is VIA Nehemiah[4].)
 
DAZ is also not supported by AMD Athlon XP. (I have one and I can see.)
 
"Some processor steppings support SSE2 but do not support the DAZ mode."[5]
From page #60[5] there is an example how to detect DAZ support.
 
"Initial steppings of Pentium® 4 processors did not support DAZ."[6]
 
An incorrect way to detect DAZ[7]. As we can see above[6], there is no guarantee that if SSE2 is available then DAZ is supported as well.
 
"But is this for all i386 CPUs or only for older models? How come this
never crashed before?"[8]
Because browser-fingerprinter scripts which are utilizing Web Audio API were not that widespread as they are now. Also, not many people are using real 32 bit CPUs.
 
The proper solution would be to detect DAZ support. We can keep using the 0x8000 mask if there is SSE as it is now. But if there is DAZ as well, use 0x8040. This way we can get the most optimal performance on most CPUs.
 
If SSE2 detection happens all around, this should be detected as well. The number of CPU steppings having DAZ support is even less than those having SSE2 support. As the usage of Web Audio API for browser-fingerprinting[9] gets even more ubiquitous, this segfault will happen more and more often.
 
[1]: https://en.wikipedia.org/wiki/Subnormal_number
[2]: http://physics.ujep.cz/~zmoravec/prga/main_for/mergedProjects/optaps_for/common/optaps_dsp_rtme.htm
[3]: https://www.carlh.net/plugins/denormals.php
[4]: https://en.wikipedia.org/wiki/List_of_VIA_Eden_microprocessors#%22Nehemiah%22_(130_nm)
[5]: https://bochs.sourceforge.io/techspec/24161821.pdf
[6]: http://web.archive.org/web/20111101165633/http://software.intel.com/en-us/articles/x87-and-sse-floating-point-assists-in-ia-32-flush-to-zero-ftz-and-denormals-are-zero-daz/
[7]: https://bugs.webkit.org/show_bug.cgi?id=134060
[8]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1012548#47
[9]: https://fingerprint.com/blog/audio-fingerprinting/

> The proper solution would be to detect DAZ support. We can keep using the
> 0x8000 mask if there is SSE as it is now. But if there is DAZ as well, use
> 0x8040. This way we can get the most optimal performance on most CPUs.

Detecting DAZ can be done in run time:

#include <cstring>
#include <cinttypes>
// Precondition: SSE support, but we take that granted for i386
bool isDazSupported()
{
#if defined(__i386__)
#if defined(__x86_64__)
  return true;
#else
  struct fxsave_area_struct {
    uint8_t		before[28];
    uint32_t	mxcsr_mask;
    uint8_t		after[480];
  } __attribute__ ((aligned (16)));

  fxsave_area_struct regdata;
  memset(&regdata, 0, sizeof(fxsave_area_struct));
  asm volatile ("fxsave %0" : "=m" (regdata));
  return regdata.mxcsr_mask & 0x0040;
#endif
#else
  return false;
#endif
}

I don't know where should I put isDazSupported(). This function would be needed for WebRTC as well, because in its code the false assumption was made there as well that all x86 CPU supports DAZ.
Also, isDazSupported() should be called only once. In the past there was a run time check for SSE2 support, I can't find it now. I also don't know how the 32 bit WebKit build without SSE2 is being built currently on 64 bit systems. The DAZ support might be handled similarly.

We could have a simpler solution: Treat all 64 bit builds DAZ compatible and all 32 bit builds DAZ incompatible. In this case the drawback would be that 32 bit builds would have a performance degradation on all 64 bit CPUs and on some newer 32 bit Pentium 4 CPUs. 

In this case the fix in DenormalDisabler.h would be pretty straightforward.
#if CPU(X86_64)
        setCSR(m_savedCSR | 0x8040);
#else
        setCSR(m_savedCSR | 0x8000);
#endif

Then a similar fix could be done in denormal_disabler.cc.

// Control register bit mask to disable denormals on the hardware.
#if defined(WEBRTC_DENORMAL_DISABLER_X86_SUPPORTED)
#if defined(WEBRTC_ARCH_X86_64)
// On x86_64 two bits are used: flush-to-zero (FTZ) and denormals-are-zero (DAZ).
constexpr int kDenormalBitMask = 0x8040;
#else
// On x86 one bit is used: flush-to-zero (FTZ).
constexpr int kDenormalBitMask = 0x8000;
#endif
#elif defined(WEBRTC_ARCH_ARM_FAMILY)
// On ARM one bit is used: flush-to-zero (FTZ).
constexpr int kDenormalBitMask = 1 << 24;
#endif

Created attachment 460286 [details]
git diff

I have coded the fix. I built it on a 64 bit Debian install, seems to work fine. But I'd like to test it on my physical 32 bit CPU before I open a PR on GitHub.
I want to build it on the same Debian install where I built the 64 bit version.

The problem is that I couldn't figure out yet how to build a 32 bit binary.

I tried
build-webkit --gtk --32-bit --makeargs="-j3"
but libwebkit2gtk-4.1.so.0.2.0 is still a 64 bit binary. "ELF 64-bit LSB shared object, x86-64", that's what `file` says.
Should I clear the output folders somehow and rebuild all? Or something completely different is needed?

Comment on attachment 460286 [details]
git diff

View in context: https://bugs.webkit.org/attachment.cgi?id=460286&action=review

> Source/WebCore/platform/audio/DenormalDisabler.h:40
> +#if defined(__GNUC__) && defined(__SSE__)

Use

#if COMPILER(GCC_COMPATIBLE) && defined(__SSE__)

> Source/WebCore/platform/audio/DenormalDisabler.h:87
> +#if defined(__GNUC__) && defined(__SSE__)

Use

#if COMPILER(GCC_COMPATIBLE) && defined(__SSE__)

> Source/WebCore/platform/audio/DenormalDisabler.h:90
> +#if defined(__x86_64__)

Use

#if CPU(X86_64)

Created attachment 460311 [details]
patch

Thanks for reviewing.
Changes applied requested by review.

(In reply to karogyoker2+webkit from comment #5)
> Created attachment 460311 [details]
> patch
> 
> Thanks for reviewing.
> Changes applied requested by review.

Oops, you need to use webkit-patch command to upload a patch.
It lacks COMMIT_MESSAGE change so we cannot land it.

Created attachment 460313 [details]
Patch

Committed r295652 (251657@main): <https://commits.webkit.org/251657@main>

Reviewed commits have been landed. Closing PR #1597 and removing active labels.