Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gcc 9.0.1 segfault while compiling GSdx #2948

Closed
lucag73 opened this issue May 1, 2019 · 28 comments
Closed

gcc 9.0.1 segfault while compiling GSdx #2948

lucag73 opened this issue May 1, 2019 · 28 comments

Comments

@lucag73
Copy link

lucag73 commented May 1, 2019

PCSX2 version:
git checkout: 56a976e

  • Notes:
    • It's preferred to only report bugs which are present on the latest development builds.
    • The PSX backward compatibility mode is still in its early stage of development. Issue reports are too early at this point.

gcc version 9.0.1 20190312 (Red Hat 9.0.1-0.10) (GCC)
[fedora 30]
fails to compile the GSdx plugin with a segmentation fault.

Description of the issue:
(What's the problem? - Screenshots showing the issue if applicable)

gcc 9.0.1 segfaults while compiling GSdx

With the following patch, the compilation ends and the hw rasterizer works; however the software rasterizer does not work (as it should have been expected).

--- a/plugins/GSdx/Renderers/SW/GSRasterizer.h
+++ b/plugins/GSdx/Renderers/SW/GSRasterizer.h
@@ -72,7 +72,7 @@ class IDrawScanline : public GSAlignedClass<32>
 {
 public:
        typedef void (*SetupPrimPtr)(const GSVertexSW* vertex, const uint32* index, const GSVertexSW& dscan);
-       typedef void (__fastcall *DrawScanlinePtr)(int pixels, int left, int top, const GSVertexSW& scan);
+       typedef void (*DrawScanlinePtr)(int pixels, int left, int top, const GSVertexSW& scan);
        typedef void (IDrawScanline::*DrawRectPtr)(const GSVector4i& r, const GSVertexSW& v); // TODO: jit
@lucag73
Copy link
Author

lucag73 commented May 1, 2019

I also include a copy of the preprocessed sources which gave the problem.

cchSAE22.log

@gregory38
Copy link
Contributor

If gcc segfault. It is a gcc bug. Even with invalid code, compiler shouldn't crash.

@lucag73
Copy link
Author

lucag73 commented May 5, 2019

Indeed; actually it appears that pcsx2 would be a good test for the functionality of gcc (every time I upgrade gcc, I find some issue with the compiler).
In any case gcc-9.1.1 (currently in fedora-updates-testing) seems to have fixed the problem (at least the compilation does not break).

@gregory38
Copy link
Contributor

gregory38 commented May 5, 2019

Indeed, PCSX2 hit too much gcc bug. I already opened 2 or 3 gcc bugs... And we even have some cmake code to blacklist some gcc version

@orbea

This comment has been minimized.

@orbea

This comment has been minimized.

@lightningterror
Copy link
Contributor

Maybe @arcum42 can lend a hand here too. Cleanup the patch, maybe silence the warnings ..etc.

@arcum42
Copy link
Contributor

arcum42 commented May 6, 2019

That's a gcc patch, not a pcsx2 patch. I'm not familiar with gcc's code, nor do I have commit rights there. ^_^

If I knew what the warnings were, I could look at those, though.

@orbea

This comment has been minimized.

@arcum42
Copy link
Contributor

arcum42 commented May 6, 2019

@orbea Thanks. Sounds like a bunch of explicitly telling it to use the default on constructors and destructors in places. I wish they had an option to automatically generate patches to fix warnings like this...

@gregory38
Copy link
Contributor

There is a clang-based tool.that can improve code. Maybe they add this case. Otherwise you can silence the warning and we will fix it later

@arcum42
Copy link
Contributor

arcum42 commented May 6, 2019

Unless I'm mistaken, a lot of it will be taking a warning like this:
/tmp/SBo/pcsx2/common/include/Utilities/pxEvents.h: In member function ‘virtual wxEvent* pxSimpleEvent::Clone() const’:
/tmp/SBo/pcsx2/common/include/Utilities/pxEvents.h:84:68: warning: implicitly-declared ‘pxSimpleEvent::pxSimpleEvent(const pxSimpleEvent&)’ is deprecated [-Wdeprecated-copy]
84 | virtual wxEvent *Clone() const { return new pxSimpleEvent(*this); }

And taking the bit in quotes and adding something like:
pxSimpleEvent::pxSimpleEvent(const pxSimpleEvent&) = default;

Haven't tested that yet, though...

@gregory38
Copy link
Contributor

Orbea search my name/email (note I mean in older gcc version).

Arcum. Yes. My understanding is that either all constructors are user defined or all are default. So we need to add declaration of implicit constructor with = default.

@orbea

This comment has been minimized.

@gregory38
Copy link
Contributor

Btw does it fully work (no crash?)

@orbea

This comment has been minimized.

@arcum42
Copy link
Contributor

arcum42 commented May 11, 2019

I went and installed gcc 9 in my 32-bit chroot. I had to do it from a ppm because apparently the chroot can't be upgraded because Ubuntu is thinking of getting rid of 32-bit builds, which is worrisome.

I was able to replicate the segfault. Got rid of it, but commenting out compiling GSdx isn't exactly a solution we can use, though it's nice to know gcc 9 specifically doesn't like that plugin.

I believe I've taken care of most or all of the new depreciated warnings, but unfortunately, the segfault doesn't appear related. Wish I knew specifically what that patch fixed so I could see if I could find a workaround. I'll have to look around.

Here's some currently untested (but compiled) code that gets rid of the warnings:
`diff --git a/common/include/Utilities/pxEvents.h b/common/include/Utilities/pxEvents.h
index 409b72dbd..cd5e2679f 100644
--- a/common/include/Utilities/pxEvents.h
+++ b/common/include/Utilities/pxEvents.h
@@ -81,6 +81,8 @@ public:
{
}

  • pxSimpleEvent(const pxSimpleEvent&) = default;
  • virtual wxEvent *Clone() const { return new pxSimpleEvent(*this); }
    };

diff --git a/common/include/x86emitter/x86types.h b/common/include/x86emitter/x86types.h
index 9b2d1f426..71f9f9d02 100644
--- a/common/include/x86emitter/x86types.h
+++ b/common/include/x86emitter/x86types.h
@@ -504,6 +504,9 @@ public:
xAddressVoid operator-(const void right) const;
xAddressVoid operator
(int factor) const;
xAddressVoid operator<<(u32 shift) const;
+

  • xAddressReg& operator=(const xAddressReg&) = default;

};

// --------------------------------------------------------------------------------------
diff --git a/common/src/x86emitter/x86emitter.cpp b/common/src/x86emitter/x86emitter.cpp
index 078cdda27..2c2b109b2 100644
--- a/common/src/x86emitter/x86emitter.cpp
+++ b/common/src/x86emitter/x86emitter.cpp
@@ -542,7 +542,6 @@ xAddressVoid xAddressReg::operator<<(u32 shift) const
return xAddressVoid(xEmptyReg, *this, 1 << shift);
}

// --------------------------------------------------------------------------------------
// xAddressVoid (method implementations)
// --------------------------------------------------------------------------------------
diff --git a/pcsx2/gui/AppCorePlugins.cpp b/pcsx2/gui/AppCorePlugins.cpp
index 5a74bb696..51907c66c 100644
--- a/pcsx2/gui/AppCorePlugins.cpp
+++ b/pcsx2/gui/AppCorePlugins.cpp
@@ -138,6 +138,7 @@ protected:
PluginsEnum_t m_pid;

public:

  • LoadSinglePluginEvent(const LoadSinglePluginEvent&) = default;
    virtual ~LoadSinglePluginEvent() = default;
    virtual LoadSinglePluginEvent *Clone() const { return new LoadSinglePluginEvent(*this); }

@@ -167,6 +168,7 @@ protected:
FnPtr_AppPluginPid m_method;

public:

  • SinglePluginMethodEvent(const SinglePluginMethodEvent&) = default;
    virtual ~SinglePluginMethodEvent() = default;
    virtual SinglePluginMethodEvent *Clone() const { return new SinglePluginMethodEvent(*this); }

diff --git a/plugins/GSdx/GSVector4.h b/plugins/GSdx/GSVector4.h
index 955fbf935..06be7cb73 100644
--- a/plugins/GSdx/GSVector4.h
+++ b/plugins/GSdx/GSVector4.h
@@ -57,6 +57,8 @@ public:
{
}

  • constexpr GSVector4(const GSVector4&) = default;
  • __forceinline GSVector4(float x, float y, float z, float w)
    {
    m = _mm_set_ps(w, z, y, x);
    `

@orbea

This comment has been minimized.

@arcum42
Copy link
Contributor

arcum42 commented May 12, 2019

Yeah, I'm just mostly seeing if there's any sort of workaround I can put in place. I doubt this is the last time we'll have people compiling on gcc 9.x. Though it may end up just being a matter of blocking gcc 9.0.1-9.10.

Though I should've read the thread a little more carefully, as it specifies that it is the fastcall attribute on DrawScanlinePtr that's crashing the compile, and that removing it causes the software renderer to stop working.

@orbea

This comment has been minimized.

@arcum42
Copy link
Contributor

arcum42 commented May 12, 2019

Glad to hear that. The main issue will be distributions that haven't applied the patch. I went ahead and pushed the compiler warning fixes, though I've switched back to gcc 8.3 until the ppm I'm using updates.

@Tatsh
Copy link

Tatsh commented May 29, 2019

@robzombie91
Copy link

I also downgraded temporarily to 8.3.0 and the package was able to build with no issues.

@rffontenelle
Copy link
Contributor

rffontenelle commented Jul 26, 2019

FYI: I don't know if it is related, but GCC's bug tracker has an open report entitled: pcsx2-git: "At global scope: cc1plus: internal compiler error: Segmentation fault"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90341

@mirh
Copy link

mirh commented Aug 24, 2019

@Pixelnarium
Copy link

I stumbled across this today. I could work around this by removing USE_LTO flag. The compiler is GCC 9.2.0 (flatpak 19.08 SDK) so I did not get the warning from the SearchForStuff.cmake file. I think the warning should also be shown on GCC 9.2.x

@lightningterror
Copy link
Contributor

lightningterror commented Nov 8, 2021

What's the status on this with latest master?

@lightningterror
Copy link
Contributor

No response, assume resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests