search

JvanKatwijk / dab-cmdline

DAB decoding library with example of its use
GNU General Public License v2.0
57 stars 29 forks source link

[Win32] crash in FULL_SPIRAL_sse() #76

Closed gvanem closed 3 years ago

gvanem commented 3 years ago

I'm having a hard time building and using this package for MSVC (32-bit). Some alignment issue in the SSE instructions is causing this crash in e.g. dab-scanner -j -C10A -Q (the SDRPlay version):

sdrdevice found = 1803049E94, hw Version = 255
{
checking data in channel 10A
dab processor will stop
checking data in channel 10B
dab processor will stop
checking data in channel 10C
dab processor will stop
checking data in channel 10D
dab processor will stop
checking data in channel 11A
dab processor will stop
checking data in channel 11B
dab processor will stop
checking data in channel 11C
dab processor will stop
checking data in channel 11D
0

Some station on 11D, then a boom:

dab!FULL_SPIRAL_sse+0x62:
613b3e52 c5f96f09        vmovdqa xmm1,xmmword ptr [ecx] ds:002b:011b1bb8=000000ff000000ff0000000000000000
ecx=011b1bb8

ECX is not a multiple of 16 for the offending code a824 = ((__m128i *) Branchtab); probably. Call-stack:

dab!FULL_SPIRAL_sse(int amount = 0x183, int * Y = 0x011b1a98, int * X = 0x011b1998, int * syms = 0x03e00100, unsigned char * dec = 0x03e06200 "", int * Branchtab = 0x011b1bb8)+0x62
dab!viterbiSpiral::deconvolve(short * input = <Value unavailable error>, unsigned char * output = <Value unavailable error>)+0x11d
dab!ficHandler::process_ficInput(short ficno = 0)+0x99
dab!ficHandler::process_ficBlock(class std::vector<short,std::allocator<short> > data = { size=0xc00 }, short blkno = 1)+0xcc
dab!dabProcessor::run(void)+0x694
dab!std::thread::_Invoke<std::tuple<void (void * _RawVals = <Value unavailable error>)+0x42
...

I've patched viterbi-spiral.h like so:

#include <xmmintrin.h>
class   viterbiSpiral {
...
 _MM_ALIGN16   COMPUTETYPE Branchtab    [NUMSTATES / 2 * RATE];
...

without any help.

JvanKatwijk commented 3 years ago

My windows knowledge and understanding is essentially zero (actually even less, and I do not intend to study on it) What I see in your post is that you changed

  COMPUTETYPE Branchtab   [NUMSTATES / 2 * RATE] ALIGN_16;

to

_MM_ALIGN16 COMPUTETYPE Branchtab [NUMSTATES / 2 * RATE];

Since it deviates from the way the GCC compilers add the attribute, I'll add an "ifdef" to the offending code

Op zo 25 apr. 2021 om 14:33 schreef Gisle Vanem @.***>:

I'm having a hard time building and using this package for MSVC (32-bit). Some alignment issue in the SSE instructions is causing this crash in e.g. dab-scanner -j -C10A -Q (the SDRPlay version):

sdrdevice found = 1803049E94, hw Version = 255 { checking data in channel 10A dab processor will stop checking data in channel 10B dab processor will stop checking data in channel 10C dab processor will stop checking data in channel 10D dab processor will stop checking data in channel 11A dab processor will stop checking data in channel 11B dab processor will stop checking data in channel 11C dab processor will stop checking data in channel 11D 0

Some station on 11D, then a boom:

dab!FULL_SPIRAL_sse+0x62: 613b3e52 c5f96f09 vmovdqa xmm1,xmmword ptr [ecx] ds:002b:011b1bb8=000000ff000000ff0000000000000000 ecx=011b1bb8

ECX is not a multiple of 16 for the offending code a824 = ((__m128i *) Branchtab); probably. Call-stack:

dab!FULL_SPIRAL_sse(int amount = 0x183, int Y = 0x011b1a98, int X = 0x011b1998, int syms = 0x03e00100, unsigned char dec = 0x03e06200 "", int Branchtab = 0x011b1bb8)+0x62 dab!viterbiSpiral::deconvolve(short input = , unsigned char output = )+0x11d dab!ficHandler::process_ficInput(short ficno = 0)+0x99 dab!ficHandler::process_ficBlock(class std::vector<short,std::allocator > data = { size=0xc00 }, short blkno = 1)+0xcc dab!dabProcessor::run(void)+0x694 dab!std::thread::_Invoke<std::tuple<void (void _RawVals = )+0x42 ...

I've patched viterbi-spiral.h like so:

include

class viterbiSpiral { ... _MM_ALIGN16 COMPUTETYPE Branchtab [NUMSTATES / 2 * RATE]; ...

without any help.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/issues/76, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQALY6QTNPO673YTM4DTKQDX3ANCNFSM43RHSVNQ .

-- Jan van Katwijk

gvanem commented 3 years ago

Thanks for a quick response. W/o a SSE_AVAILABLE it kinda works. No crash, but no results from dab-scanner -Q -L1. (except for a 0 at ch 11D, 12B and 12D ?!).

I have built Welle-IO using MSVC which do work fine. So there are multiple issues with Windows support and especially MSVC:

All these items I have fixed quite easily in some example programs, but not this SSE alignment issue.

JvanKatwijk commented 3 years ago

Thanks

A couple of years ago I wrote the library with only Linux in mind, the same for most of my other programs As said, I do not have any affection to Windows. For Qt-DAB - for which I provide a Windows installer - I use the mingw-gcc toolset and cross compile. In a different project I am - since early this year - doing some things for Windows using MSVC, but it only strengthens my aversion to Windows and windows development

Op zo 25 apr. 2021 om 15:40 schreef Gisle Vanem @.***>:

Thanks for a quick response. W/o a SSE_AVAILABLE it kinda works. No crash, but no results from dab-scanner -Q -L1. (except for a 0 at ch 11D, 12B and 12D ?!).

I have built Welle-IO using MSVC which do work fine. So there are multiple issues with Windows support and especially MSVC:

  • sigaction stuff.
  • VLA.
  • SSE/MMX alignment.
  • getopt()

  • C++ conformance; with -std:c++17 I get no important warnings such as:

    library/src/backend/audio-backend.cpp(66): warning C4316: 'uep_protection': object allocated on the heap may not
    be aligned 16 in -std:c++14. No idea why.

All these items I have fixed quite easily in some example programs, but not this SSE alignment issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/issues/76#issuecomment-826326507, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQE6QKEVOYH5BSJOYZ3TKQLWFANCNFSM43RHSVNQ .

-- Jan van Katwijk

JvanKatwijk commented 3 years ago

Btw, if you want to scan the band while playing Windows, the Qt-DAB program does a better job than the scan module for dab-cmdline

Op zo 25 apr. 2021 om 15:54 schreef jan van katwijk @.***

:

Thanks

A couple of years ago I wrote the library with only Linux in mind, the same for most of my other programs As said, I do not have any affection to Windows. For Qt-DAB - for which I provide a Windows installer - I use the mingw-gcc toolset and cross compile. In a different project I am - since early this year - doing some things for Windows using MSVC, but it only strengthens my aversion to Windows and windows development

Op zo 25 apr. 2021 om 15:40 schreef Gisle Vanem @.***

:

Thanks for a quick response. W/o a SSE_AVAILABLE it kinda works. No crash, but no results from dab-scanner -Q -L1. (except for a 0 at ch 11D, 12B and 12D ?!).

I have built Welle-IO using MSVC which do work fine. So there are multiple issues with Windows support and especially MSVC:

  • sigaction stuff.
  • VLA.
  • SSE/MMX alignment.
  • getopt()

  • C++ conformance; with -std:c++17 I get no important warnings such as:

    library/src/backend/audio-backend.cpp(66): warning C4316: 'uep_protection': object allocated on the heap may not
    be aligned 16 in -std:c++14. No idea why.

All these items I have fixed quite easily in some example programs, but not this SSE alignment issue.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/issues/76#issuecomment-826326507, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQE6QKEVOYH5BSJOYZ3TKQLWFANCNFSM43RHSVNQ .

-- Jan van Katwijk

-- Jan van Katwijk

gvanem commented 3 years ago

, the Qt-DAB program does a better job than the scan module for dab-cmdline

Have tried, didn't like it. My motive with DAB-cmdline is simply to use dab.dll in the new SDRangel demodulator plugin. But I'm confused regarding the "Win32/MSVC state" of the various forks of this package. Seems there are too many?

I have built Welle-IO using MSVC which do work fine.

And I can add, DAB-reception works fine in the DAB-plugin of SDRuno. But despite your aversion against Windows, I'll try to add some PR to fix this tragic situation...

srcejon commented 3 years ago

My windows knowledge and understanding is essentially zero (actually even less, and I do not intend to study on it) What I see in your post is that you changed COMPUTETYPE Branchtab [NUMSTATES / 2 RATE] ALIGN_16; to _MM_ALIGN16 COMPUTETYPE Branchtab [NUMSTATES / 2 RATE]; Since it deviates from the way the GCC compilers add the attribute, I'll add an "ifdef" to the offending code Op zo 25 apr. 2021 om 14:33 schreef Gisle Vanem @.***>:

I think GCC allows attributes to be in front of the declaration, so you may be able to define ALIGN_16 in such a way that it works for both.

srcejon commented 3 years ago

Have tried, didn't like it. My motive with DAB-cmdline is simply to use dab.dll in the new SDRangel demodulator plugin. But I'm confused regarding the "Win32/MSVC state" of the various forks of this package. Seems there are too many?

Today's release of SDRangel should now have DAB working on 64-bit Windows using this library - and Jvan has merged all the changes I made to get it to run.

I haven't yet updated SDRangel to use the updated DAB API though - so probably best to just use the fork SDRangel checks out automatically (you don't need to build the examples, just the library) See https://github.com/f4exb/sdrangel/wiki/Compile-in-Windows

gvanem commented 3 years ago

I noted this B I G N O T E at: https://github.com/AlbrechtL/welle.io/blob/next/src/backend/viterbi.cpp#L137

which has caused the FULL_SPIRAL_sse() junk to simply be removed from Welle-IO. When I do not know, but there was an issue on it 4 years ago. Seems related to my problem here.

You should perhaps remove it in your version too.

gvanem commented 3 years ago

.. so probably best to just use the fork SDRangel checks out automatically

You mean this: https://github.com/f4exb/DAB-cmdline/tree/msvc ?

JvanKatwijk commented 3 years ago

The issue was that welle had taken the wrong buffer size

Op ma 26 apr. 2021 om 18:20 schreef Gisle Vanem @.***>:

I noted this B I G N O T E at:

https://github.com/AlbrechtL/welle.io/blob/next/src/backend/viterbi.cpp#L137

which has caused the FULL_SPIRAL_sse() junk to simply be removed from Welle-IO. When I do not know, but there was an issue https://github.com/AlbrechtL/welle.io/issues/123 on it 4 years ago. Seems related to my problem here.

You should perhaps remove it in your version too.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JvanKatwijk/dab-cmdline/issues/76#issuecomment-826972490, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCPHQBWHYWWGFQT25RP5TDTKWHDVANCNFSM43RHSVNQ .

-- Jan van Katwijk

gvanem commented 3 years ago

I found the issue with SSE & alignment problem yesterday. So I was about to close it, but you beat me to it. Thanks!