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

openhantek crashing #5

Closed
Vascom opened this issue Apr 30, 2019 · 19 comments
Closed

openhantek crashing #5

Vascom opened this issue Apr 30, 2019 · 19 comments
Labels
BUG Something isn't working Volunteers wanted NO SUPPORT unless someone wants to help!

Comments

@Vascom
Copy link
Contributor

Vascom commented Apr 30, 2019

This happen then I set Normal mode and move trigger threshold.

Also as I understand "normal" mode in oscilloscopes it should stop refresh data on the screen if trigger not crossed. It is bug too

/usr/include/c++/9/bits/stl_vector.h:1042: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) [with _Tp = std::pair<QOpenGLVertexArrayObject*, int>; _Alloc = std::allocator<std::pair<QOpenGLVertexArrayObject*, int> >; std::vector<_Tp, _Alloc>::reference = std::pair<QOpenGLVertexArrayObject*, int>&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.
KCrash: Application 'OpenHantek' crashing...
KCrash: Attempting to start /usr/libexec/drkonqi from kdeinit
sock_file=/run/user/1000/kdeinit5__0
Getting sample data failed:  "Other error"
Qt has caught an exception thrown from an event handler. Throwing
exceptions from an event handler is not supported in Qt.
You must not let any exception whatsoever propagate through Qt code.
If that is not possible, in Qt 5 you must at least reimplement
QCoreApplication::notify() and catch all exceptions there.

terminate called after throwing an instance of 'std::length_error'
  what():  vector::_M_default_append
Unable to start Dr. Konqi

And backtrace from gdb https://paste.fedoraproject.org/paste/L6k6zoKs9lqEUIZnhwhcdw

@Ho-Ro Ho-Ro added the More input please! More info necessary to handle this issue label Apr 30, 2019
@Ho-Ro
Copy link
Member

Ho-Ro commented Apr 30, 2019

This happen then I set Normal mode and move trigger threshold.

Tried to reproduce without "success". Please describe the exact way to get the crash.

Also as I understand "normal" mode in oscilloscopes it should stop refresh data on the screen if trigger not crossed. It is bug too

"Normal" works as follows:

  • Screen stops in "Normal" mode as soon as trigger condition becomes false and a triggered trace is already on the screen.
  • As soon as trigger condition becomes true the screen is updated.
  • Switching from "Auto" to "Normal" with false trigger condition doesn't stop the screen (because no triggered trace is available).

@Vascom
Copy link
Contributor Author

Vascom commented Apr 30, 2019

I will show you video after few days.

@Vascom
Copy link
Contributor Author

Vascom commented May 5, 2019

It is here

oh_v_0-2019-05-05_07.41.00.zip
It is latest version from git.

@Vascom
Copy link
Contributor Author

Vascom commented May 5, 2019

And here you can see that Normal mode not working as expected:
oh_v_1-2019-05-05_09.34.28.zip

@Ho-Ro
Copy link
Member

Ho-Ro commented May 5, 2019

I reproduced all your steps with a 1st time run (no config file) as well as with a saved config. I was not able to see this crash, also not with all recent versions. Looks like the crash comes out of some OpenGL code. As your and my video HW as well as Qt version are different you're unfortunately on your own - sorry.
I will show the rough data flow later in this thread to give you some hints where to collect some more debug info.

@Vascom
Copy link
Contributor Author

Vascom commented May 5, 2019

May be I should change some build options?

@Ho-Ro
Copy link
Member

Ho-Ro commented May 5, 2019

build option? - I don't know

Here's the data flow:
hantekdso/hantekdsocontrol sets sampling parameter, starts sampling and converts the raw 8bit samples into a vector of real physical double values (in volt).
void HantekDsoControl::convertRawDataToSamples(const std::vector<unsigned char> &rawData) -> result.data[channel][pos]
These values are modified in the post processing chain (post/softwaretrigger, post/spectrumgenerator,...) and finally post/graphgenerator creates the picture by calling the OpenGL functions.
void GraphGenerator::generateGraphsTYvoltage(PPresult *result)

    // not triggered but triggered values available and normal mode
    if ( postTrigSamples <= preTrigSamples && trigAvailable && scope->trigger.mode == Dso::TriggerMode::NORMAL ) { 
        result->softwareTriggerTriggered = false; // show red trigger status on top left screen
        return; // stop processing, reuse old values
    }

Another shot in the dark:
During my checks to #2 I saw that my screen looks more like your 1st picture without antialiasing than your smoothed pic. Maybe theOpenGL crash is related to this difference.
Please try to disable antialiasing in glscope.cpp:43
// format.setSamples(4); // Antialiasing, Multisampling

@Vascom
Copy link
Contributor Author

Vascom commented May 6, 2019

Disabling antialiasing not helped.

Look at more good backtrace:

#0  0x00007ffff7e0beb5 in raise () from /lib64/libc.so.6
No symbol table info available.
#1  0x00007ffff7df6895 in abort () from /lib64/libc.so.6
No symbol table info available.
#2  0x00005555555cdaa8 in std::__replacement_assert (__file=<optimized out>, __line=<optimized out>, __function=<optimized out>, 
    __condition=<optimized out>) at /usr/include/c++/9/x86_64-redhat-linux/bits/c++config.h:2510
No locals.
#3  0x00005555555b29ad in std::vector<std::pair<QOpenGLVertexArrayObject*, int>, std::allocator<std::pair<QOpenGLVertexArrayObject*, int> > >::operator[] (__n=0, this=0x555555ee6bc0) at /usr/include/qt5/QtGui/qmatrix4x4.h:718
        __PRETTY_FUNCTION__ = <optimized out>
#4  GlScope::drawVoltageChannelGraph (historyIndex=<optimized out>, graph=..., channel=0, this=0x5555559f7940)
    at /usr/src/debug/openhantek-2.02-1.fc30.x86_64/openhantek/src/glscope.cpp:614
        v = <optimized out>
        b = <optimized out>
        dMode = <optimized out>
        v = <optimized out>
        b = <optimized out>
        dMode = <optimized out>
#5  GlScope::paintGL (this=0x5555559f7940) at /usr/src/debug/openhantek-2.02-1.fc30.x86_64/openhantek/src/glscope.cpp:423
        channel = 0
        graph = <optimized out>
        __for_range = std::__cxx11::list = {[0] = {allocatedMem = 245760, buffer = {d_ptr = 0x7fffe000e0b0}, 
            vaoVoltage = std::vector of length 0, capacity 3, vaoSpectrum = std::vector of length 3, capacity 3 = {{first = 0x555555a3b5f0, 
                second = 0}, {first = 0x555555d60140, second = 0}, {first = 0x555555d5e060, second = 0}}}}
        __for_begin = <optimized out>
        __for_end = <optimized out>
        gl = <optimized out>
        historyIndex = <optimized out>
        gl = <optimized out>
        historyIndex = <optimized out>
        m = <optimized out>
        graph = <optimized out>
        __for_range = <optimized out>
        __for_begin = <optimized out>
--Type <RET> for more, q to quit, c to continue without paging--
        __for_end = <optimized out>
        channel = <optimized out>
#6  GlScope::paintGL (this=0x5555559f7940) at /usr/src/debug/openhantek-2.02-1.fc30.x86_64/openhantek/src/glscope.cpp:395
        gl = <optimized out>
        historyIndex = <optimized out>
        m = <optimized out>
        graph = <optimized out>
        __for_range = <optimized out>
        __for_begin = <optimized out>
        __for_end = <optimized out>
        channel = <optimized out>

@Vascom
Copy link
Contributor Author

Vascom commented May 6, 2019

I found that it is one of Fedora build flags. I will continue to find which one.

@Vascom
Copy link
Contributor Author

Vascom commented May 6, 2019

It is -Wp,-D_GLIBCXX_ASSERTIONS build flag.

Enable C++ standard library hardening with -D_GLIBCXX_ASSERTIONS. This turns on cheap range checks for C++ arrays, vectors, and strings.

And now I see really good Normal mode with freezing.

But this assertion still can report about vector length problem in code.

@Vascom
Copy link
Contributor Author

Vascom commented May 6, 2019

If apply this patch

diff -r -U3 OpenHantek6022.orig/openhantek/src/glscope.cpp OpenHantek6022/openhantek/src/glscope.cpp
--- OpenHantek6022.orig/openhantek/src/glscope.cpp	2019-05-06 13:11:41.474816511 +0300
+++ OpenHantek6022/openhantek/src/glscope.cpp	2019-05-06 13:11:18.862736954 +0300
@@ -611,6 +611,8 @@
     if (!scope->voltage[channel].used) return;
 
     m_program->setUniformValue(colorLocation, view->screen.voltage[channel].darker(100 + 10 * historyIndex));
+
+    if (channel >= graph.vaoVoltage.size()) return;
     Graph::VaoCount &v = graph.vaoVoltage[channel];
 
     QOpenGLVertexArrayObject::Binder b(v.first);

crashing are stopped. But trace disappears instead of freeze when no trigger event..

So please rewrite this code as you more powerful in that.

Ho-Ro added a commit that referenced this issue May 6, 2019
Signed-off-by: Martin <Ho-Ro@users.noreply.github.com>
@Ho-Ro
Copy link
Member

Ho-Ro commented May 6, 2019

Added global -D_GLIBCXX_ASSERTIONS (as already used by Fedora) and #undef it only for 1 file.
@Vascom please double check if it works

@Vascom
Copy link
Contributor Author

Vascom commented May 7, 2019

It is very ugly fix. You should correct code instead it.
I am already disable this assertion for openhantek build in Fedora rpmfusion repo. So I prefer wait normal fix.

@Ho-Ro Ho-Ro added Enhancement New feature or request Volunteers wanted NO SUPPORT unless someone wants to help! and removed More input please! More info necessary to handle this issue labels May 7, 2019
@Ho-Ro
Copy link
Member

Ho-Ro commented May 7, 2019

I know it's ugly, but it works reliably according to the data flow:

  • Raw 8-bit ADC values are collected in HantekDsoControl and converted to real-world double samples (scaled with voltage and sample rate).
  • These samples are emitted to PostProcessing::input() via signal/slot.
  • PP calls mathchannelGenerator, spectrumGenerator and finally graphGenerator where 3 voltage traces (vaoVoltage) and 3 spectrum traces (vaoSpectrum) are generated and filled if channel is used (CH1, CH2, MATH).
  • graphGenerator calls SoftwareTrigger::compute()
  • if trigger condition is false && we have already a triggered trace && trigger mode is Normal then gG returns without creating a new trace (vector of length 0), but as we do not modify the content of vaoVoltage the old trace is still in memory (capacity 3):
    vaoVoltage = std::vector of length 0, capacity 3
  • It will be accessed later in GlScope::drawVoltage() with ChannelGraph(Graph::VaoCount &v = graph.vaoVoltage[channel]; where the assert hits:
__for_range = std::__cxx11::list = {[0] = {allocatedMem = 245760, buffer = {d_ptr = 0x7fffe000e0b0}, 
            vaoVoltage = std::vector of length 0, capacity 3, vaoSpectrum = std::vector of length 3, capacity 3 = {{first = 0x555555a3b5f0, 
                second = 0}, {first = 0x555555d60140, second = 0}, {first = 0x555555d5e060, second = 0}}}}

I will update and reuse the detailed data flow above also in a separate readme document. This was missing in the past and the maintainers had to deep-dive into the code structure that grew over years.
For a future refactoring my plan would be to keep the latest triggered samples stored as a backup and reuse them if the trigger fails in normal mode - but not yet... Volunteers welcome:exclamation:
For the debian (and macOS) build the newly added global #define _LIBCXX_ASSERTIONS adds some more safety and revokes this only for glscope.cpp - I can live with that.

@Vascom
Copy link
Contributor Author

Vascom commented May 7, 2019

Will it cause a memory leak or memory corruption?

@Vascom
Copy link
Contributor Author

Vascom commented May 7, 2019

After applying this patch (or disable _GLIBCXX_ASSERTIONS at all) I have new crash in same file:

#0  GlScope::drawVoltageChannelGraph (historyIndex=<optimized out>, graph=..., channel=0, this=0x5555559b90e0)
    at /usr/src/debug/openhantek-2.03-1.fc30.x86_64/openhantek/src/glscope.cpp:620
        v = {first = <error reading variable v (Cannot access memory at address 0x0)>

Steps to reproduce:

  1. Enable Normal mode.
  2. Enable digital phosphor.
  3. Move trigger away from signal.
  4. Go to settings and increase Digital phosphor depth to 10. Press Apply button.

@Ho-Ro Ho-Ro added BUG Something isn't working and removed Enhancement New feature or request labels May 7, 2019
@Ho-Ro
Copy link
Member

Ho-Ro commented May 7, 2019

Will it cause a memory leak or memory corruption?

No, because the vector has a capacity of 3 and is not destroyed, only the element count is set to 0. The content is still there and will be displayed.

The "digital phosphor" issue is a bug. It has low probability to meet the conditions above but it's not correct. I think I found a real clean solution for all issues above, but it will take some days to implement.

Ho-Ro added a commit that referenced this issue May 8, 2019
Signed-off-by: Martin <Ho-Ro@users.noreply.github.com>
@Ho-Ro
Copy link
Member

Ho-Ro commented May 9, 2019

New clean fix available.

@Ho-Ro
Copy link
Member

Ho-Ro commented May 9, 2019

Finally closed with v2.04 (0ffadc7). Avoids also artefacts when switching channels - extremely annoying when the traces are long time on screen due to using digital phosphor.

@Ho-Ro Ho-Ro closed this as completed May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Something isn't working Volunteers wanted NO SUPPORT unless someone wants to help!
Projects
None yet
Development

No branches or pull requests

2 participants