Lv2: Fix overflow and enum visualization #5811

JohannesLorenz · 2020-11-29T21:28:40Z

Unfortunately, the developer who wrote #5788 forgot the testing phase for their PR 👎 . This fix does (citing the commit log):

Fix arithmetic overflow in Lv2Ports::Meta::get() in case min and
max are not set
Fix combo boxes with >16 values being wrongly visualized as knobs
Rename Lv2Ports::Vis enum value None to Generic

First: Let's wait for CI.
Code review: optional.
Testing: Please test the #5788 PR here again, with the instructions from there. Please also check that the arithmetic overflow is gone.

* Fix arithmetic overflow in `Lv2Ports::Meta::get()` in case min and max are not set * Fix combo boxes with >16 values being wrongly visualized as knobs * Rename `Lv2Ports::Vis` enum value `None` to `Generic`

LmmsBot · 2020-11-29T21:35:05Z

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

Linux (AppImage): lmms-1.3.0-alpha.1.29%2Bgf9a06f2-linux-x86_64.AppImage (build link)

Windows

Windows 32-bit: lmms-1.3.0-alpha.1.29%2Bgf9a06f2a3-mingw-win32.exe (build link)
Windows 64-bit: lmms-1.3.0-alpha.1.29%2Bgf9a06f2a3-mingw-win64.exe (build link)
Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

macOS

macOS : lmms-1.3.0-alpha.1.29%2Bgf9a06f2a3-mac10.13.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://10941-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.29%2Bgf9a06f2-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10941?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://10942-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.29%2Bgf9a06f2a3-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10942?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://10943-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.29%2Bgf9a06f2a3-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10943?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/0p79d4ep0b7ub9y7/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36568061"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/86pmhpdevc94317h/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36568061"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://10940-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.29%2Bgf9a06f2a3-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/10940?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "dcaba3a69bcde9898e49bd2a5fec89c754aba584"}

zonkmachine · 2020-11-29T21:40:13Z

Same overflow.

Thread 1 "lmms" received signal SIGFPE, Arithmetic exception.
Lv2Ports::Meta::get (this=0x7fffffffd890, plugin=0x555555f960f0, portNum=4) at /home/zonkmachine/builds/lmms/src/core/lv2/Lv2Ports.cpp:221
221				if (m_max - m_min > 15.0f)
(gdb)

* Fix arithmetic overflow in `Lv2Ports::Meta::get()` in case min and
  max are not set

I don't think the problem is that they're not set but that they are set to the max/min values for a float. max - (-min) => 2*max => overflow. Why not set m_max/m_min to something smaller? I made it run by setting them to 1.0f/-1.0f .

Logs

Click to expand

Thread 1 "lmms" received signal SIGFPE, Arithmetic exception.
Lv2Ports::Meta::get (this=0x7fffffffd890, plugin=0x555555f960f0, portNum=4) at /home/zonkmachine/builds/lmms/src/core/lv2/Lv2Ports.cpp:221
221				if (m_max - m_min > 15.0f)
(gdb) bt full
#0  Lv2Ports::Meta::get(LilvPluginImpl const*, unsigned long) (this=0x7fffffffd890, plugin=0x555555f960f0, portNum=4)
    at /home/zonkmachine/builds/lmms/src/core/lv2/Lv2Ports.cpp:221
        isToggle = false
        minN = 0x0
        maxN = 0x0
        def = std::unique_ptr = {get() = 0x0}
        defN = 0x0
        min = std::unique_ptr = {get() = 0x0}
        max = std::unique_ptr = {get() = 0x0}
        takeRangeValue = {__this = 0x7fffffffd890, __issue = @0x7fffffffd6f8, __portName = "latency"}
        portIssues = std::vector of length 0, capacity 0
        issue = {__portIssues = std::vector of length 0, capacity 0}
        man = 0x555556057ef0
        lilvPort = 0x5555567880a0
        portFunc = {__plugin = @0x7fffffffd6d8, __lilvPort = @0x7fffffffd708, __man = @0x7fffffffd700}
        hasProperty = {__portFunc = @0x7fffffffd760}
        isA = {__portFunc = @0x7fffffffd760}
        portName = "latency"
        m_min_set = {__this = 0x7fffffffd890}
        m_max_set = {__this = 0x7fffffffd890}
#1  0x0000555555849d48 in Lv2Proc::check(LilvPluginImpl const*, std::vector >&)
    (plugin=0x555555f960f0, issues=std::vector of length 0, capacity 0) at /home/zonkmachine/builds/lmms/src/core/lv2/Lv2Proc.cpp:85
        meta = 
          {m_type = Lv2Ports::Type::Control, m_flow = Lv2Ports::Flow::Output, m_vis = Lv2Ports::Vis::None, m_logarithmic = false, m_optional = false, m_used = true, m_def = 0, m_min = -3.40282347e+38, m_max = 3.40282347e+38, m_sampleRate = false}
        tmp = std::vector of length 0, capacity 0
        portMustBeUsed = true
        portNum = 4

JohannesLorenz · 2020-11-29T22:44:14Z

@zonkmachine The line that your gdb crashes in is not part of this PR anymore 😃 . Did you forget to rebuild?

It should really work now.

zonkmachine · 2020-11-29T22:51:16Z

It works.

Did you forget to rebuild?

No, I think I was on the wrong commit somehow. I thought i 'fetched'... :p

JohannesLorenz · 2020-11-29T23:39:08Z

@zonkmachine OK, take your time for testing, and let me know when you think this PR - and the stuff from the old logarithmic PR - are ready for merge.

JohannesLorenz · 2020-11-29T23:39:39Z

I'll leave this open for 5 days anyways.

JohannesLorenz · 2020-12-03T03:46:57Z

I'm getting the rpmalloc issue with FPE here. I think you mentioned that problem already in discord @zonkmachine ? Any solutions to that?

lmms: lmms/master/src/3rdparty/rpmalloc/rpmalloc/rpmalloc/rpmalloc.c:1248: _rpmalloc_span_finalize: Assertion `span->list_size == span->used_count' failed.

zonkmachine · 2020-12-03T03:56:14Z

Any solutions to that?

I was just looking into that. I don't know how to touch /src/3rdparty/rpmalloc/CMakeLists.txt from cmake but this should work.

diff --git a/src/3rdparty/rpmalloc/CMakeLists.txt b/src/3rdparty/rpmalloc/CMakeLists.txt
index 047c32678..601d2f1eb 100644
--- a/src/3rdparty/rpmalloc/CMakeLists.txt
+++ b/src/3rdparty/rpmalloc/CMakeLists.txt
@@ -36,7 +36,7 @@ if (CMAKE_BUILD_TYPE STREQUAL "Debug")
                set(ENABLE_VALIDATE_ARGS ON)
        endif()
        target_compile_definitions(rpmalloc
-               PRIVATE -DENABLE_ASSERTS=1 -DENABLE_VALIDATE_ARGS=${ENABLE_VALIDATE_ARGS}
+               PRIVATE -DENABLE_ASSERTS=0 -DENABLE_VALIDATE_ARGS=${ENABLE_VALIDATE_ARGS}
        )
 endif()

JohannesLorenz · 2020-12-03T04:00:02Z

I think we need to merge this branch to master now and rebase the Lv2 options PR, in order to debug it with FPE? Right now, the options PR always hangs at the m_max - m_min...

zonkmachine · 2020-12-03T04:01:04Z

Please do. Now.

Lv2: Fix overflow and enum visualization

dcaba3a

* Fix arithmetic overflow in `Lv2Ports::Meta::get()` in case min and max are not set * Fix combo boxes with >16 values being wrongly visualized as knobs * Rename `Lv2Ports::Vis` enum value `None` to `Generic`

JohannesLorenz requested a review from zonkmachine November 29, 2020 21:28

JohannesLorenz mentioned this pull request Nov 29, 2020

Lv2: Use port-property "logarithmic" #5788

Merged

JohannesLorenz merged commit ddf69fe into LMMS:master Dec 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lv2: Fix overflow and enum visualization #5811

Lv2: Fix overflow and enum visualization #5811

JohannesLorenz commented Nov 29, 2020

LmmsBot commented Nov 29, 2020 •

edited

Loading

Linux

Windows

macOS

zonkmachine commented Nov 29, 2020 •

edited

Loading

JohannesLorenz commented Nov 29, 2020

zonkmachine commented Nov 29, 2020

JohannesLorenz commented Nov 29, 2020

JohannesLorenz commented Nov 29, 2020

JohannesLorenz commented Dec 3, 2020 •

edited

Loading

zonkmachine commented Dec 3, 2020

JohannesLorenz commented Dec 3, 2020

zonkmachine commented Dec 3, 2020

Lv2: Fix overflow and enum visualization #5811

Lv2: Fix overflow and enum visualization #5811

Conversation

JohannesLorenz commented Nov 29, 2020

LmmsBot commented Nov 29, 2020 • edited Loading

Linux

Windows

macOS

zonkmachine commented Nov 29, 2020 • edited Loading

Logs

JohannesLorenz commented Nov 29, 2020

zonkmachine commented Nov 29, 2020

JohannesLorenz commented Nov 29, 2020

JohannesLorenz commented Nov 29, 2020

JohannesLorenz commented Dec 3, 2020 • edited Loading

zonkmachine commented Dec 3, 2020

JohannesLorenz commented Dec 3, 2020

zonkmachine commented Dec 3, 2020

LmmsBot commented Nov 29, 2020 •

edited

Loading

zonkmachine commented Nov 29, 2020 •

edited

Loading

JohannesLorenz commented Dec 3, 2020 •

edited

Loading