Skip to content

Manipulating value of labels may cause a crash in debug mode.#6258

Merged
JorisGoosen merged 5 commits into
jasp-stats:developmentfrom
boutinb:makeLabelGreatAgain
Jun 16, 2026
Merged

Manipulating value of labels may cause a crash in debug mode.#6258
JorisGoosen merged 5 commits into
jasp-stats:developmentfrom
boutinb:makeLabelGreatAgain

Conversation

@boutinb

@boutinb boutinb commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Fixes https://github.com/jasp-stats/INTERNAL-jasp/issues/3236
A unit test is added in testdebugdata.

First fix was to add rememberCurrentOrigValDisplay when _labelMapUpdates is called.

But a second fix is necessary:

 The real bug: _labelByValDis keys are built via origValDisplay() which calls processLabel(label, value). For a numeric label whose display equals its value (e.g., _label="6",
  origVal=6), processLabel("6","6") returns "" — so the map key is ("6","").

  But when labelDisplayChanged is later called with previousDisplay="6" (the raw _label), it was constructing oldValDis = ("6","6") — which doesn't exist in the map, failing the
  assert at line 1806.

  The same mismatch existed in labelValDisplayChanged and labelsRemoveByIntsId. Three fixes applied:

  1. labelDisplayChanged:1799 — wrap previousDisplay in Label::processLabel(previousDisplay, origValStr)
  2. labelValDisplayChanged:1822 — wrap previousDisplay in Label::processLabel(previousDisplay, oldOrigValS)
  3. labelsRemoveByIntsId:1159 — replace std::make_pair(label->originalValueAsString(), label->label()) with label->origValDisplay(), which already applies processLabel internally

@boutinb boutinb requested a review from JorisGoosen June 9, 2026 14:35
Comment thread CommonData/column.cpp Outdated
Lets try it, because I also still get the crash

Co-authored-by: Shun Wang <shuonwang@gmail.com>
@JorisGoosen

Copy link
Copy Markdown
Contributor

Looks like this at least fixes the original crash. Im looking into this a bit more to see if I can add some more tests for this

Comment thread Desktop/utilities/settings.cpp Outdated
@JorisGoosen

Copy link
Copy Markdown
Contributor

I need to test these changes but my worksystem is now in a state of limbo because im recom-piling webengine a bunch, but its crashing now. The dialog really neeeds to be tested and checked still so dont merge

@JorisGoosen JorisGoosen force-pushed the makeLabelGreatAgain branch from 9686af3 to 5cced61 Compare June 16, 2026 10:13
@JorisGoosen JorisGoosen merged commit d38176a into jasp-stats:development Jun 16, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants