Palemoon 29.0.1 format-overflow error with gcc 9.3

Talk about code development, features, specific bugs, enhancements, patches, and similar things.
Forum rules
Please keep everything here strictly on-topic.
This board is meant for Pale Moon source code development related subjects only like code snippets, patches, specific bugs, git, the repositories, etc.

This is not for tech support! Please do not post tech support questions in the "Development" board!
Please make sure not to use this board for support questions. Please post issues with specific websites, extensions, etc. in the relevant boards for those topics.

Please keep things on-topic as this forum will be used for reference for Pale Moon development. Expect topics that aren't relevant as such to be moved or deleted.
User avatar
Bababo26
Moongazer
Moongazer
Posts: 9
Joined: 2021-02-12, 11:49

Palemoon 29.0.1 format-overflow error with gcc 9.3

Unread post by Bababo26 » 2021-02-13, 11:31

Palemoon 29.0.1 fails to compile with gcc 9.3 (in my case gcc version 9.3.0 (Gentoo 9.3.0-r2 p4)). I'm using the gentoo ebuild from https://github.com/deu/palemoon-overlay (overriding the compiler version check). There was no problem with the version 28.17.0 compilation using the same compiler.

29.0.1 fails with the following errors when compiling o/js/src/jit/Unified_cpp_js_src_jit14.cpp

Code: Select all

In file included from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x64/BaseAssembler-x64.h:9,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/Assembler-x86-shared.h:16,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x64/Assembler-x64.h:247,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/MacroAssembler-x86-shared.h:14,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x64/MacroAssembler-x64.h:11,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/MacroAssembler.h:17,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/BaselineJIT.h:17,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/BaselineIC.h:19,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/MIR.h:20,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/LIR.h:19,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/shared/Lowering-shared.h:12,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/Lowering-x86-shared.h:9,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/Lowering-x86-shared.cpp:6,
                 from /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/o/js/src/jit/Unified_cpp_js_src_jit14.cpp:2:
In member function ‘void js::jit::X86Encoding::BaseAssembler::twoByteOpSimd(const char*, js::jit::X86Encoding::VexOperandType, js::jit::X86Encoding::TwoByteOpcodeID, int32_t, js::jit::X86Encoding::RegisterID, js::jit::X86Encoding::XMMRegisterID, js::jit::X86Encoding::XMMRegisterID)’,
    inlined from ‘void js::jit::X86Encoding::BaseAssembler::vmovsd_mr(int32_t, js::jit::X86Encoding::RegisterID, js::jit::X86Encoding::XMMRegisterID)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/BaseAssembler-x86-shared.h:3155:22,
    inlined from ‘void js::jit::AssemblerX86Shared::vmovsd(const js::jit::Address&, js::jit::FloatRegister)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/Assembler-x86-shared.h:614:23,
    inlined from ‘void js::jit::Assembler::pop(js::jit::FloatRegister)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x64/Assembler-x64.h:350:15,
    inlined from ‘void js::jit::MacroAssembler::Pop(js::jit::FloatRegister)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp:581:8:
/var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/BaseAssembler-x86-shared.h:3967:21: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
 3967 |                 spew("%-11s" MEM_ob ", %s", legacySSEOpName(name),
      |                 ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3968 |                      ADDR_ob(offset, base), XMMRegName(dst));
      |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In member function ‘void js::jit::X86Encoding::BaseAssembler::twoByteOpSimd(const char*, js::jit::X86Encoding::VexOperandType, js::jit::X86Encoding::TwoByteOpcodeID, int32_t, js::jit::X86Encoding::RegisterID, js::jit::X86Encoding::XMMRegisterID, js::jit::X86Encoding::XMMRegisterID)’,
    inlined from ‘void js::jit::X86Encoding::BaseAssembler::vmovsd_rm(js::jit::X86Encoding::XMMRegisterID, int32_t, js::jit::X86Encoding::RegisterID)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/BaseAssembler-x86-shared.h:3110:22,
    inlined from ‘void js::jit::AssemblerX86Shared::vmovsd(js::jit::FloatRegister, const js::jit::Address&)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/Assembler-x86-shared.h:620:23,
    inlined from ‘void js::jit::Assembler::push(js::jit::FloatRegister)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x64/Assembler-x64.h:341:15,
    inlined from ‘void js::jit::MacroAssembler::Push(js::jit::FloatRegister)’ at /var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp:560:9:
/var/tmp/portage/www-client/palemoon-29.0.1/work/palemoon-29.0.1/platform/js/src/jit/x86-shared/BaseAssembler-x86-shared.h:3964:21: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
 3964 |                 spew("%-11s%s, " MEM_ob, legacySSEOpName(name),
      |                 ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3965 |                      XMMRegName(dst), ADDR_ob(offset, base));
      |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors
To get around this problem I added -Wformat-overflow to my compile flags which converted the above errors to warnings. After the successful build I had a look to see if this was a false positive from gcc but I think it isn't.

The code causing the problem inside the twoByteOpSimd method is

Code: Select all

    if (useLegacySSEEncoding(src0, dst)) {
        if (IsXMMReversedOperands(opcode)) {
            spew("%-11s%s, " MEM_ob, legacySSEOpName(name),
                 XMMRegName(dst), ADDR_ob(offset, base));
        } else {
            spew("%-11s" MEM_ob ", %s", legacySSEOpName(name),
                 ADDR_ob(offset, base), XMMRegName(dst));
        }
        m_formatter.legacySSEPrefix(ty);
        m_formatter.twoByteOp(opcode, offset, base, dst);
        return;
    }
I split up one of the problem spew calls and discovered that the XMMRegName(dst) was the problem value. After some digging in the code I think the problem is caused by the following
  1. useLegacySSEEncoding returns true if src0 == dst (useVEX_ defaults to true so other logic is not used)
  2. src0 = invalid_xmm when twoByteOpSimd is called by vmovsd_mr therefore gcc can assume for the code path dst = invalid_xmm
  3. XMMRegName does not provide a value for invalid_xmm therefore no string to spew.
So I think the real problem is an array index problem - the names array in XMMRegName does not have a invalid_xmm member. Making the following change allowed me to compile Unified_cpp_js_src_jit14.cpp with no format-overflow warnings/errors.

Code: Select all

diff --git a/js/src/jit/x86-shared/Constants-x86-shared.h b/js/src/jit/x86-shared/Constants-x86-shared.h
index e5f1d7cd8..bbae28f2a 100644
--- a/js/src/jit/x86-shared/Constants-x86-shared.h
+++ b/js/src/jit/x86-shared/Constants-x86-shared.h
@@ -43,10 +43,11 @@ enum XMMRegisterID {
 inline const char* XMMRegName(XMMRegisterID reg)
 {
     static const char* const names[] = {
-        "%xmm0", "%xmm1", "%xmm2", "%xmm3", "%xmm4", "%xmm5", "%xmm6", "%xmm7"
+        "%xmm0", "%xmm1", "%xmm2", "%xmm3", "%xmm4", "%xmm5", "%xmm6", "%xmm7",
 #ifdef JS_CODEGEN_X64
-       ,"%xmm8", "%xmm9", "%xmm10", "%xmm11", "%xmm12", "%xmm13", "%xmm14", "%xmm15"
+        "%xmm8", "%xmm9", "%xmm10", "%xmm11", "%xmm12", "%xmm13", "%xmm14", "%xmm15",
 #endif
+        "invalid"
     };
     MOZ_ASSERT(size_t(reg) < mozilla::ArrayLength(names));
     return names[reg];
BTW - does the MOZ_ASSERT make sense?

A different way to get rid of the format-overflow warnings/errors is to prevent the problem spews if src0 is invalid_xmm.

Code: Select all

diff --git a/js/src/jit/x86-shared/BaseAssembler-x86-shared.h b/js/src/jit/x86-shared/BaseAssembler-x86-shared.h
index 54b862a56..cea5aff53 100644
--- a/js/src/jit/x86-shared/BaseAssembler-x86-shared.h
+++ b/js/src/jit/x86-shared/BaseAssembler-x86-shared.h
@@ -3959,7 +3959,7 @@ threeByteOpImmSimd("vblendps", VEX_PD, OP3_BLENDPS_VpsWpsIb, ESCAPE_3A, imm, off
     void twoByteOpSimd(const char* name, VexOperandType ty, TwoByteOpcodeID opcode,
                        int32_t offset, RegisterID base, XMMRegisterID src0, XMMRegisterID dst)
     {
-        if (useLegacySSEEncoding(src0, dst)) {
+        if (src0 != invalid_xmm && useLegacySSEEncoding(src0, dst)) {
             if (IsXMMReversedOperands(opcode)) {
                 spew("%-11s%s, " MEM_ob, legacySSEOpName(name),
                      XMMRegName(dst), ADDR_ob(offset, base));
I'm not sure if either approach looks like a correct fix but I was just trying to see if my theory for format-overflow warnings/errors was correct which I think both demonstrate.

vannilla
Moon Magic practitioner
Moon Magic practitioner
Posts: 2183
Joined: 2018-05-05, 13:29

Re: Palemoon 29.0.1 format-overflow error with gcc 9.3

Unread post by vannilla » 2021-02-13, 17:24

I had the same problem, but unlike you I couldn't find the root cause, so I resorted to downgrade gcc to version 8 for the sake of having an updated browser.
Thank you for looking up the issue.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35479
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Palemoon 29.0.1 format-overflow error with gcc 9.3

Unread post by Moonchild » 2021-02-13, 23:37

I'm surprised that this is hit to begin with. jitspewing is an advanced debugging option for internal JIT development and should never be hit in release builds... unless Mozilla changed that somewhere along the way. It's a little weird that gcc 9 tries to evaluate all this a compile-time. it shouldn't. That does sound like a little too aggressive of an optimization.

That being said, adding a name entry for invalid_xmm to the register names sounds like the proper solution from casual inspection of the code, if invalid_mmx is actively used (currently the register definition above it is one entry larger). Since it's only for spewing it doesn't affect compilation.
Yes the MOZ_ASSERT makes sense and WOULD actually be hit in a debug build here; I don't really have time to dig completely into the call chain to know why this gets hit in 29 and not 28, but it certainly seems to be a discrepancy that should be fixed to prevent null strings for an fprint. (and "<" is correct because 0-based array vs. 1-based length)
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
Bababo26
Moongazer
Moongazer
Posts: 9
Joined: 2021-02-12, 11:49

Re: Palemoon 29.0.1 format-overflow error with gcc 9.3

Unread post by Bababo26 » 2021-02-14, 10:08

Moonchild wrote:
2021-02-13, 23:37
I don't really have time to dig completely into the call chain to know why this gets hit in 29 and not 28, but it certainly seems to be a discrepancy that should be fixed to prevent null strings for an fprint. (and "<" is correct because 0-based array vs. 1-based length)
I assume it wasn't a problem with 28 because "inlining of code optimization in our JIT compiler for JavaScript (IonMonkey)" was disabled and without the inlining of the code the problem can't be spotted.

New Tobin Paradigm

Re: Palemoon 29.0.1 format-overflow error with gcc 9.3

Unread post by New Tobin Paradigm » 2021-02-14, 12:03

The option is a runtime option and unrelated.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35479
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Palemoon 29.0.1 format-overflow error with gcc 9.3

Unread post by Moonchild » 2021-02-14, 14:56

Fixed with this commit.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

Locked