SPU2-X Windows XP crash fix

[mirh]

Long story short, I tried to look for that " invalid access to memory location". And well, I found out we ain't alone. But workarounds weren't working with SPU2.

So I tried to force the thing on the whole damn pcsx2 solution, toghether with /MT and some other switches because reasons. After a lot of tinkering with property manager (inheriting sucks in VS2015), pinpointing and swearing at msdn with no damn info it seems...

The problem is in wxBase30.

More precisely you have to tell the compiler to build the project with /Zc:threadSafeInit- parameter

If other clues were needed, I dunno, people like @ramapcsx2 could try to decipher this :s

[turtleli]

http://trac.wxwidgets.org/ticket/13116

Also in wx/setup.h

// Enable the use of compiler-specific thread local storage keyword, if any.
// This is used for wxTLS_XXX() macros implementation and normally should use
// the compiler-provided support as it's simpler and more efficient, but is
// disabled under Windows in wx/msw/chkconf.h as it can't be used if wxWidgets
// is used in a dynamically loaded Win32 DLL (i.e. using LoadLibrary()) under
// XP as this triggers a bug in compiler TLS support that results in crashes
// when any TLS variables are used.
//
// If you're absolutely sure that your build of wxWidgets is never going to be
// used in such situation, either because it's not going to be linked from any
// kind of plugin or because you only target Vista or later systems, you can
// set this to 2 to force the use of compiler TLS even under MSW.
//
// Default is 1 meaning that compiler TLS is used only if it's 100% safe.
//
// Recommended setting: 2 if you want to have maximal performance and don't
// care about the scenario described above.
#define wxUSE_COMPILER_TLS 1

https://connect.microsoft.com/VisualStudio/feedback/details/1789709/visual-c-2015-runtime-broken-on-windows-server-2003-c-11-magic-statics

Windows Server 2003 and Windows XP have problems with dynamically loading a DLL (via LoadLibrary) that uses thread-local storage, which is what thread-safe statics use internally to provide efficient execution when the static local has already been initialized. As these systems are out of support, it is extremely unlikely for a patch to be created for those systems to add this support as is present in Vista and newer OSes, and we are reluctant to penalize the performance on in-support OSes to provide this functionality to the old out-of-support ones.

To work around the issue you can use /Zc:threadSafeInit- to disable the thread-safe initialization code and this will avoid the thread-local variable. Of course by doing so the initialization code reverts back to the VS2013 mode and is not thread-safe, so this option is only viable if you don't rely on the thread-safety of local statics.

Making changes to either might hurt performance and/or introduce bugs - I don't think either should be changed.

[ramapcsx2]

Yea, I tend to agree. Keep #define wxUSE_COMPILER_TLS 1, as this will also kind of future proof the issue.

[mirh]

Performance has been benchmarked here. And regardless, we are already using the slow path in Wx.

As for introducing bugs.. isn't that like just the behavior vs2013 had?

[turtleli]

Well, I checked, we are indeed on the slow path. As for VS2013 behaviour, I don't know, but I'm still very much against changing the compiler flag and potentially introducing bugs that we could quite simply avoid by not doing so.

[mirh]

It should just be about this change. You'll know better than me what that can imply or not.

And I almost wonder if this issue shouldn't be worth to be reported upstream, considering how much Wx-team seems into compatibility.

[gregory38]

If people want to use a nearly 20 years old OS, they must use old PSX2 build. Hack to support xp is a no go.

[mirh]

Shouldn't the point be whether this entail any downside/problem or not?

[gregory38]

A flag changes that can creatle subtle issue, or in future code is bad.

[mirh]

It's exactly in evaluating the "probability" of these issues that I think the point resides (and what I was asking).
Anyway, I sent a ticket on Wx tracker. Perhaps they will have some fresher clue.

[willkuer]

The vs2013 build bot is a valid and usable workaround, or? I guess we have like three xp users. They can either stick to older versions or use the appveyor 2013 build bot.

[gregory38]

nobody want to fiddle with thread safety. We already get enough issue with them. We can't support xp for a century. Besides, xp users can still install 1.4 (vs2013). They loose nothing.

[turtleli]

Potential upsides of dropping XP are:

• No need to install DirectX redists on Windows 8, 8.1 and Windows 10 to run PCSX2. There are plenty of people asking for help on the forums where the answer is "Install the DirectX redists". We can avoid a lot of that.
• We can use XAudio 2.8/2.9 for Windows 8, 8.1 and 10 and XAudio 2.7 for Windows 7/Vista, since 2.7 can be buggy (and not because of us) and I don't really know whether my workaround works or not.
• Reduce usage of DirectX SDK headers to only xaudio2.h (EDIT: and whatever that pulls in) for SPU2-X. That'd let people compile everything except SPU2-X without the DirectX SDK installed, some people do seem to have trouble installing the SDK on later Windows OSes.
• We can move to the newer non-XP compatible toolkits. We can use some of the newer API, and the Visual Studio code analysis tools will work.

This is a hint on what I'm working on. So the ability to run future PCSX2 dev builds on XP will disappear soon. It's already been stated multiple times that 1.4.0 is the last release to support XP and it's also in the README.md, so it should come as no surprise.

[mirh]

I guess we have like three xp users.

I definitively know the "pros" are next to nonexistent. My point was more on whether the possibility of problems for enabling this switch couldn't be even more negligible than the little number you took as example.

nobody want to fiddle with thread safety

? Who should? We always lived without it up to this point.
I didn't want this to become a XP yes, or not discussion, I already know what is crappy doom is. And this is even why I didn't ask the team to debug this and I lost the time myself.

Said this, if the benefits proposed by turteli are real I can see the huge jump ahead. But I wouldn't know if this is the best place to discuss them since as I said it was just about this change and if any I see the problem in estimating the odds of "side effects".

EDIT: just while I was finalizing this comment, it seems Wx would gladly accept upstream a patch to include this change.

[mirh]

Fixed upstream :* ^^

[ramapcsx2]

Meaning?

[mirh]

Ehrm.. meaning they were looking forward to including the fix in 3.1.0 (and patch is there), but they still have to merge it.

[RebelliousX]

The funny thing is, I had this issue 2 or 3 months ago, for me I needed /Zc:threadSafeInit- for both plugin and wxWidgets to be built that way with #define wxUSE_COMPILER_TLS set to 2. And it only works in XP for Release build, and not Debug using 140_xp compiler. I don't really care about XP but some users requested it.

[mirh]

Really? You could have documented it tbh.
For me (for release at least indeed) it was just about adding

<ClCompile>
 <AdditionalOptions Condition="'$(VisualStudioVersion)' == '14.0'">/Zc:threadSafeInit- %(AdditionalOptions)</AdditionalOptions>
</ClCompile>

to 3rdparty\wxwidgets3.0\build\msw\Common.props.
But now that I think, "'$(PlatformToolset)' == 'v140_xp'" would be even neater (I didn't test it yet).

I wonder why they all fled my bug report though.

[RebelliousX]

@mirh I did mention that in the forum and my github repo//commits. Also mentioned that SPU-X has same problem.

Anyways, if you look at the TLS option in setup.h and read the comment above it about wx/msw/chkconf.h. If you look at that file how it handles TLS, it only checks:

/*
 * Unfortunately we can't use compiler TLS support if the library can be used
 * inside a dynamically loaded DLL under Windows XP, as this can result in hard
 * to diagnose crashes due to the bugs in Windows TLS support, see #13116.
 *
 * So we disable it unless we can be certain that the code will never run under
 * XP, as is the case if we're using a compiler which doesn't support XP such
 * as MSVC 11+, unless it's used with the special "_xp" toolset, in which case
 * _USING_V110_SDK71_ is defined.
 *
 * However defining wxUSE_COMPILER_TLS as 2 overrides this safety check, see
 * the comments in wx/setup.h.
 */
#if wxUSE_COMPILER_TLS == 1
    #if !wxCHECK_VISUALC_VERSION(11) || defined(_USING_V110_SDK71_)
        #undef wxUSE_COMPILER_TLS
        #define wxUSE_COMPILER_TLS 0
    #endif
#endif

So it effectively uses #define wxUSE_COMPILER_TLS 0 for anything other than VS version 11 (VS2012/VS2013) or when using windows SDK71 not SDK81 (which they are related to the use of v110_xp, v120_xp or v140_xp too). So using VS2015 then compiler TLS is set to 0 at the end (disabled). That is why it was working in VS2013 but not 2015.

Both VS and wxWidgets trying to solve TLS issues at the same time seems to get bad results. I chose to let VS handle thread local storage safety and disable checks in wxWidgets for maximum performance when set as 2 for compiler TLS in setup.h.

And an answer to your question why they "fled" the bug report, in my openion vads wxWidget's dev, is waiting for XP to die out, he is clueless as everybody else how to handle this issue, and afraid to cause regressions, since Microsoft gave up on XP, and that issue doesn't exist from Vista onwards. The same way PCSX2 devs think.

[mirh]

Mhh no, really, they seem to care a lot about it, and they all had been very sympathetic up to that point.

So it effectively uses #define wxUSE_COMPILER_TLS 0 for anything other than VS version 11 (VS2012/VS2013) or when using windows SDK71 not SDK81 (which they are related to the use of v110_xp, v120_xp or v140_xp too). So using VS2015 then compiler TLS is set to 0 at the end (disabled).

Man, until 9ed9b2d we were indeed using that.

That is why it was working in VS2013 but not 2015.

And mhh no? This problem was about the change introduced here, which nullified wx workaround. You need both to be aware of the situation for that stuff to work.

Funnily, while I was googling for that I found this. What about.. W8 SDK supporting XP too? ^^

[RebelliousX]

Well, I was speaking for my experience with wxWidgets and my own plugin, not SPU2-X specifically.
And I know about /Zc:threadSafeInit- change in VS2014 CTP. I am using that as I said.

To answer your last question, Yes and no: You can use Windows SDK8.1 with targeting XP but you have to be careful, since XP doesn't support DirectX 10 nor of course 11. (see the links at the bottom)

I did however, to get rid of DirectX SDK dependency and to rely on Windows SDK use Windows SDK7.1 (all DirectX headers that I need) and 8.1 (for the missing dxguid.lib) and it works for me.

Here are two useful links:
(How to substitute DirectX SDK with Windows SDK)
https://blogs.msdn.microsoft.com/chuckw/2015/03/23/the-zombie-directx-sdk/
(How to target XP with Windows SDK)
https://blogs.msdn.microsoft.com/chuckw/2012/11/26/visual-studio-2012-update-1/

And for SPU2-X, I bet you can use the needed XAudio2.h from Windows SDK instead of DirectX SDK without issues.

[mirh]

And for SPU2-X, I bet you can use the needed XAudio2.h from Windows SDK instead of DirectX SDK without issues.

Yes, #1253 should perfect that.
The problem in this ticket though was about the crashing due to wxwidgets.

[turtleli]

I'm going to close this - The compiler flag isn't going to be changed, and I don't see a point in making the change if we're not supporting XP on the dev builds anyway.

[mirh]

Never say never :p
Anyway, that compiler flag is now included in wx 3.0 branch.

[gregory38]

Honestly. Only a single web browser is supported on XP, Firefox. They will drop the support mid 2017.

[mirh]

I'm not pretending you to waste your time for support, but anyway I don't see the correlation between browser and pcsx2.

[gregory38]

What I mean is that the OS can't be used safely in any way. It is time to migrate to a new OS. And soon Win7 support will be dropped too. So let's not encourage people to keep a deprecated OS

[mirh]

Oh, you meant that.
Well, I don't see anything wrong with the offline use of XP.
Also, just for the records:

• firefox 52 ESR is still being supported till first months of 2018
• you may be confusing Vista and 7 (which is being dropped in 2020, not exactly soon)

And by then, tbh, I'd expect the year of linux on desktop 😄

[gregory38]

https://blog.mozilla.org/futurereleases/2016/12/23/firefox-support-for-xp-and-vista/

It is September 2017 not 2018.

Well 3 years is soon. People need time to upgrade.

[mirh]

In mid-2017, user numbers on Windows XP and Vista will be reassessed and a final support end date will be announced.

In my experience, people need time to realize they have to upgrade
Besides, tbh I much more likely see 2010 computers still offering a satisfactory experience in 2020, than 2005 computers offering a satisfactory experience in 2011, if you get me.

[gregory38]

Read all the blog.

Yes I know. My gf's computer (Win7/Linux) is good enough for her work. If I replace the HDD by a SSD I can keep it for an additional 10 years. I'm afraid Win7 will still be here in 2025...

[mirh]

I did read all the blog, but I also read bugzilla boards and indeed they were targeting somewhere in 2018 to drop 52 ESR branch.