ROOT crash when parameter by value passed to slot: StrDup #7195

ferdymercury · 2021-02-12T14:00:41Z

Describe the bug

According to https://root.cern.ch/root/htmldoc/guides/users-guide/WritingGUI.html#event-processing-signals-and-slots one can pass parameters by value to your slots.

Connect(myButton, "Pressed()","TH1",hist, "Draw(=\"LEGO\")");

For example, I was trying:
myTGCheckBox->Connect("Clicked()","TGTextButton", myButton, "SetText(=\"LEGOlegoemoryABCDEF\")");

But when I click on my checkbox, ROOT crashes then with:

 *** Break *** segmentation violation
===========================================================
There was a crash.
This is the entire stack trace of all threads:
===========================================================
#0  0x00007f7c3b4d0457 in __GI___waitpid (pid=14103, stat_loc=stat_loc
entry=0x7ffcd65f6a98, options=options
entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:30
#1  0x00007f7c3b43b177 in do_system (line=<optimized out>) at ../sysdeps/posix/system.c:149
#2  0x00007f7c3c078960 in TUnixSystem::StackTrace() () from /opt/root_bld/lib/libCore.so
#3  0x00007f7c3c07b4d5 in TUnixSystem::DispatchSignals(ESignals) () from /opt/root_bld/lib/libCore.so
#4  <signal handler called>
#5  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
#6  0x00007f7c3bf806be in StrDup(char const*) () from /opt/root_bld/lib/libCore.so
#7  0x00007f7c3c7c10ea in TGHotString::TGHotString(char const*) () from /opt/root_bld/lib/libGui.so
#8  0x00007f7c3c6967ad in TGTextButton::SetText(TString const&) () from /opt/root_bld/lib/libGui.so
#9  0x00007f7c2b3b903c in ?? ()
#10 0x0000563f9e2c0f30 in ?? ()
#11 0x0000563f9da66200 in ?? ()
#12 0x0000000000000000 in ?? ()
===========================================================


The lines below might hint at the cause of the crash.
You may get help by asking at the ROOT forum https://root.cern.ch/forum
Only if you are really convinced it is a bug in ROOT then please submit a
report at https://root.cern.ch/bugs Please post the ENTIRE stack trace
from above as an attachment in addition to anything else
that might help us fixing this issue.
===========================================================
#5  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
#6  0x00007f7c3bf806be in StrDup(char const*) () from /opt/root_bld/lib/libCore.so
#7  0x00007f7c3c7c10ea in TGHotString::TGHotString(char const*) () from /opt/root_bld/lib/libGui.so
#8  0x00007f7c3c6967ad in TGTextButton::SetText(TString const&) () from /opt/root_bld/lib/libGui.so
#9  0x00007f7c2b3b903c in ?? ()
#10 0x0000563f9e2c0f30 in ?? ()
#11 0x0000563f9da66200 in ?? ()
#12 0x0000000000000000 in ?? ()
===========================================================

If I try to pass other string combinations, then ROOT sometimes crash, sometimes not but puts weird characters or only part of the string in the TGTextButton.

Expected behavior

My TGTextButton would get the passed string correctly when the checkbox is clicked.

To Reproduce

Create a GUI with a TGCheckButton and a TGTextBox
Connect the checkbutton click with a SetText(="somelongstring") slot with argument passed by value.
Run and see crash

Setup

ROOT version: 6.23/01
Operating system: Linux
Self-built

The text was updated successfully, but these errors were encountered:

bellenot · 2021-02-15T07:55:29Z

Please use ChangeText instead:

   myTGCheckBox->Connect("Clicked()", "TGTextButton", myButton, "ChangeText(=\"LEGOlegoemoryABCDEF\")");

ferdymercury · 2021-02-15T09:12:39Z

Thanks for the hint. I just tried that, now I do not get a crash, but rather a warning and the button name is not changed:

Error in <TClingCallFunc::exec>: The method ChangeText is called without an object.

bellenot · 2021-02-15T09:22:16Z

Well, this simple example works just fine for me on Windows and CentOS7:

void testSignalSlot()
{
   TGMainFrame *mainframe = new TGMainFrame(gClient->GetRoot(), 300, 170);
   TGCheckButton *myTGCheckBox = new TGCheckButton(mainframe, new TGHotString("Change Text"), -1);
   mainframe->AddFrame(myTGCheckBox, new TGLayoutHints(kLHintsCenterX | kLHintsCenterY));
   TGTextButton *myButton = new TGTextButton(mainframe, "&Any Text");
   mainframe->AddFrame(myButton, new TGLayoutHints(kLHintsExpandX | kLHintsCenterY));
   myTGCheckBox->Connect("Clicked()", "TGTextButton", myButton, "ChangeText(=\"LEGOlegoemoryABCDEF\")");
   mainframe->MapSubwindows();
   mainframe->Layout();
   mainframe->MapWindow();
}

ferdymercury · 2021-02-15T10:21:11Z

Thanks for the nice example. It works on Ubuntu, too.
(Sorry, my last message was an error on my side, I was passing a nullptr as myButton).

Why is SetText() leading to a crash?

Maybe we could add a short comment here in the documentation:
https://root.cern/doc/master/classTGTextButton.html#aa2d8b609e96fb43c0a11b6b9ece52c83

"If you use a signal-slot connection with inline argument passed as (text) value, use ChangeText instead of SetText".

bellenot · 2021-02-15T10:27:58Z

Well, I'll check, but the signatures of SetText are : void TGTextButton::SetText(const TString &new_label) and void SetText (TGHotString *new_label). I.e. no char * (or const char *) argument. But I agree, ROOT should complain about it, somehow...

ferdymercury · 2021-02-15T10:35:29Z

Thanks for checking.

My second use case was a combobox selection leading to a tgtextbutton change. I tried it with your example, but it is not working.

Do you get a similar crash on Windows?

#include "TGFrame.h"
#include "TGButton.h"
#include "TGComboBox.h"

void testSignalSlot()
{
   TGMainFrame *mainframe = new TGMainFrame(gClient->GetRoot(), 300, 170);
   TGCheckButton *myTGCheckBox = new TGCheckButton(mainframe, new TGHotString("Change Text"), -1);
   mainframe->AddFrame(myTGCheckBox, new TGLayoutHints(kLHintsCenterX | kLHintsCenterY));
   TGComboBox *myTGComboBox = new TGComboBox(mainframe, "Select...");
   myTGComboBox->AddEntry("test1", 0);
   myTGComboBox->AddEntry("test2", 1);
   mainframe->AddFrame(myTGComboBox, new TGLayoutHints(kLHintsExpandX | kLHintsCenterY));
   TGTextButton *myButton = new TGTextButton(mainframe, "&Any Text");
   mainframe->AddFrame(myButton, new TGLayoutHints(kLHintsExpandX | kLHintsCenterY));
   myTGCheckBox->Connect("Clicked()", "TGTextButton", myButton, "ChangeText(=\"LEGOlegoemoryABCDEF\")");
   myTGComboBox->Connect("Selected(Int_t)", "TGTextButton", myButton, "ChangeText(=\"LEGOlegoemoryABCDEF\")");
   mainframe->MapSubwindows();
   mainframe->Layout();
   mainframe->MapWindow();
}


root -l testSignalSlot.cpp+
root [0] 
Processing testSignalSlot.cpp+...
Info in <TUnixSystem::ACLiC>: creating shared library /tmp/./testSignalSlot_cpp.so
root [1] 
 *** Break *** segmentation violation



===========================================================
There was a crash.
This is the entire stack trace of all threads:
===========================================================
#0  0x00007f49a0ab5457 in __GI___waitpid (pid=3907, stat_loc=stat_loc
entry=0x7ffe8def7918, options=options
entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:30
#1  0x00007f49a0a20177 in do_system (line=<optimized out>) at ../sysdeps/posix/system.c:149
#2  0x00007f49a1660340 in TUnixSystem::StackTrace() () from /opt/root_bld/lib/libCore.so
#3  0x00007f49a1662eb5 in TUnixSystem::DispatchSignals(ESignals) () from /opt/root_bld/lib/libCore.so
#4  <signal handler called>
#5  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
#6  0x00007f49a1561565 in TString::TString(char const*) () from /opt/root_bld/lib/libCore.so
#7  0x00007f499261ff70 in TGTextButton::SetTitle(char const*) () from /opt/root_bld/lib/libGui.so
#8  0x00007f4993663032 in ?? ()
#9  0x0000000000000000 in ?? ()
===========================================================


The lines below might hint at the cause of the crash.
You may get help by asking at the ROOT forum https://root.cern.ch/forum
Only if you are really convinced it is a bug in ROOT then please submit a
report at https://root.cern.ch/bugs Please post the ENTIRE stack trace
from above as an attachment in addition to anything else
that might help us fixing this issue.
===========================================================
#5  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
#6  0x00007f49a1561565 in TString::TString(char const*) () from /opt/root_bld/lib/libCore.so
#7  0x00007f499261ff70 in TGTextButton::SetTitle(char const*) () from /opt/root_bld/lib/libGui.so
#8  0x00007f4993663032 in ?? ()
#9  0x0000000000000000 in ?? ()
===========================================================

If I run it with the QtCreator debugger, it completely freezes the mouse, I have to open a terminal and call pkill qtcreator. (Which is something I rarely had needed to do before.)

bellenot · 2021-02-15T13:46:15Z

Do you get a similar crash on Windows?

Yes. I will debug it

ferdymercury · 2021-02-16T13:41:48Z

This is what my debugger shows before it freezes:

It looks as if the Selected(Int_t) signal argument (integer) is being passed as a string?.

I was able to reproduce the crash isolatedly in a ROOT terminal:

const char* cs = reinterpret_cast<const char*>(0x1);
strlen(cs)
#0  0x00007fa539040457 in __GI___waitpid (pid=20783, stat_loc=stat_loc
entry=0x7ffc922ae768, options=options
entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:30
#1  0x00007fa538fab177 in do_system (line=<optimized out>) at ../sysdeps/posix/system.c:149
#2  0x00007fa539d4d5c4 in TUnixSystem::Exec (this=0x55bdc96f17c0, shellcmd=0x55bdca31f240 "/opt/root_bld/etc/gdb-backtrace.sh 20254 1>&2") at /opt/root_src/core/unix/src/TUnixSystem.cxx:2120
#3  0x00007fa539d4de75 in TUnixSystem::StackTrace (this=0x55bdc96f17c0) at /opt/root_src/core/unix/src/TUnixSystem.cxx:2411
#4  0x00007fa532b992f1 in TCling__PrintStackTrace () at /opt/root_src/core/metacling/src/TCling.cxx:324
#5  0x00007fa532cecc2b in TClingCallbacks::PrintStackTrace (this=0x55bdc9c9adc0) at /opt/root_src/core/metacling/src/TClingCallbacks.cxx:921
#6  0x00007fa532d5dfc8 in cling::MultiplexInterpreterCallbacks::PrintStackTrace() () from /opt/root_bld/lib/libCling.so
#7  0x00007fa532d5d902 in cling_runtime_internal_throwIfInvalidPointer () from /opt/root_bld/lib/libCling.so
#8  0x00007fa53a595075 in ?? ()
#9  0x00007ffc922b0b70 in ?? ()
#10 0x00007fa532b99375 in TCling__ResetInterpreterMutex () at /opt/root_src/core/metacling/src/TCling.cxx:351
#11 0x00007fa532ddb133 in cling::IncrementalExecutor::executeWrapper(llvm::StringRef, cling::Value*) const () from /opt/root_bld/lib/libCling.so
#12 0x00007fa532d5fcf0 in cling::Interpreter::RunFunction(clang::FunctionDecl const*, cling::Value*) () from /opt/root_bld/lib/libCling.so
#13 0x00007fa532d60d58 in cling::Interpreter::EvaluateInternal(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, cling::CompilationOptions, cling::Value*, cling::Transaction**, unsigned long) () from /opt/root_bld/lib/libCling.so
#14 0x00007fa532d60f2b in cling::Interpreter::process(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, cling::Value*, cling::Transaction**, bool) () from /opt/root_bld/lib/libCling.so
#15 0x00007fa532e38e25 in cling::MetaProcessor::process(llvm::StringRef, cling::Interpreter::CompilationResult&, cling::Value*, bool) () from /opt/root_bld/lib/libCling.so
#16 0x00007fa532ba4e14 in HandleInterpreterException (metaProcessor=0x55bdc9e8b7a0, input_line=0x55bdca3173a0 "#line 1 \"ROOT_prompt_6\"\nstrlen(cs)", compRes=
0x7ffc922b1024: cling::Interpreter::kSuccess, result=0x7ffc922b1070) at /opt/root_src/core/metacling/src/TCling.cxx:2433
#17 0x00007fa532ba5aa6 in TCling::ProcessLine (this=0x55bdc9765c30, line=0x55bdca174110 "#line 1 \"ROOT_prompt_6\"\nstrlen(cs)", error=0x7ffc922b1588) at /opt/root_src/core/metacling/src/TCling.cxx:2591
#18 0x00007fa539bb646a in TApplication::ProcessLine (this=0x55bdc97497e0, line=0x55bdca174110 "#line 1 \"ROOT_prompt_6\"\nstrlen(cs)", sync=false, err=0x7ffc922b1588) at /opt/root_src/core/base/src/TApplication.cxx:1471
#19 0x00007fa53a245bb9 in TRint::ProcessLineNr (this=0x55bdc97497e0, filestem=0x7fa53a255f2f "ROOT_prompt_", line=0x7ffc922b1649 "strlen(cs)", error=0x7ffc922b1588) at /opt/root_src/core/rint/src/TRint.cxx:751
#20 0x00007fa53a245484 in TRint::HandleTermInput (this=0x55bdc97497e0) at /opt/root_src/core/rint/src/TRint.cxx:612
#21 0x00007fa53a242df1 in TTermInputHandler::Notify (this=0x55bdca28cb00) at /opt/root_src/core/rint/src/TRint.cxx:132
#22 0x00007fa53a2473f7 in TTermInputHandler::ReadNotify (this=0x55bdca28cb00) at /opt/root_src/core/rint/src/TRint.cxx:124
#23 0x00007fa539d4b871 in TUnixSystem::CheckDescriptors (this=0x55bdc96f17c0) at /opt/root_src/core/unix/src/TUnixSystem.cxx:1322
#24 0x00007fa539d4aae7 in TUnixSystem::DispatchOneEvent (this=0x55bdc96f17c0, pendingOnly=false) at /opt/root_src/core/unix/src/TUnixSystem.cxx:1077
#25 0x00007fa539c2dc49 in TSystem::InnerLoop (this=0x55bdc96f17c0) at /opt/root_src/core/base/src/TSystem.cxx:404
#26 0x00007fa539c2d9de in TSystem::Run (this=0x55bdc96f17c0) at /opt/root_src/core/base/src/TSystem.cxx:354
#27 0x00007fa539bb6fea in TApplication::Run (this=0x55bdc97497e0, retrn=false) at /opt/root_src/core/base/src/TApplication.cxx:1623
#28 0x00007fa53a2447ef in TRint::Run (this=0x55bdc97497e0, retrn=false) at /opt/root_src/core/rint/src/TRint.cxx:463
#29 0x000055bdc8f70a99 in main (argc=1, argv=0x7ffc922b3ac8) at /opt/root_src/main/src/rmain.cxx:30
Error in <HandleInterpreterException>: Trying to access a pointer that points to an invalid memory address..
Execution of your code was aborted.
ROOT_prompt_6:1:8: warning: invalid memory pointer passed to a callee:
strlen(cs)
       ^~
root [7] cs
(const char *) 0x1 <invalid memory address>

If I select the first element of the Combo-Box, then cs = 0 and it does not crash because the if(cs) in the TString constructor.

bellenot · 2021-02-16T13:49:16Z

I didn't have time to look at it yet, but the integer is maybe the argument of the signal Selected(Int_t) trying to be passed to the slot. And it's maybe a limitation of the signal/slot mechanism

ferdymercury · 2021-02-16T13:51:50Z

Yes, I just confirmed it by changing this:
myTGComboBox->Connect("Selected(const char*)", "TGTextButton", myButton, "ChangeText(=\"LEGOlegoemoryABCDEF\")");

Now it does not crash, but instead of changing the text to the one I pass by value, it replaces it with the TGCombo text entry.
Weird... I thought the idea to pass by value was to override this.

bellenot · 2021-02-16T13:55:55Z

I know that the signal/slot mechanism has some limitations, so that might well be one of them.

ferdymercury · 2021-02-16T13:57:46Z

Ok, thanks for the reply. Maybe a note in the documentation could be added specifying that it only works if the signal has the shape of Clicked() or Pressed(), not with extra arguments ?: https://root.cern.ch/root/htmldoc/guides/users-guide/WritingGUI.html#event-processing-signals-and-slots

…slot with arguments by value Fixes root-project#7195

ferdymercury · 2021-02-16T16:05:35Z

I provided a PR for this.
I have tested the new signal in my code and now it does not crash, all works well :)

Thanks for the excellent support.

* fix typos * clarify slot args by value * emit signal with no arguments, for being able to safely connect with slot with arguments by value Fixes #7195 * rename to Changed for avoiding ambiguity/confusion * fix arg-by-value example

…oject#7228) * fix typos * clarify slot args by value * emit signal with no arguments, for being able to safely connect with slot with arguments by value Fixes root-project#7195 * rename to Changed for avoiding ambiguity/confusion * fix arg-by-value example

ferdymercury added the bug label Feb 12, 2021

github-actions bot added this to Needs triage in Triage Feb 12, 2021

eguiraud assigned bellenot Feb 12, 2021

eguiraud added the in:GUI label Feb 12, 2021

eguiraud removed this from Needs triage in Triage Feb 12, 2021

ferdymercury added a commit to ferdymercury/root that referenced this issue Feb 16, 2021

emit signal with no arguments, for being able to safely connect with …

a6fdb1d

…slot with arguments by value Fixes root-project#7195

ferdymercury mentioned this issue Feb 16, 2021

Emit a Changed() signal on TGComboBox whenever entry changes #7228

Merged

bellenot closed this as completed in #7228 Feb 17, 2021

bellenot added this to Issues in Fixed in 6.24/00 via automation Feb 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ROOT crash when parameter by value passed to slot: StrDup #7195

ROOT crash when parameter by value passed to slot: StrDup #7195

ferdymercury commented Feb 12, 2021 •

edited

bellenot commented Feb 15, 2021

ferdymercury commented Feb 15, 2021

bellenot commented Feb 15, 2021

ferdymercury commented Feb 15, 2021

bellenot commented Feb 15, 2021

ferdymercury commented Feb 15, 2021 •

edited

bellenot commented Feb 15, 2021

ferdymercury commented Feb 16, 2021 •

edited

bellenot commented Feb 16, 2021 •

edited

ferdymercury commented Feb 16, 2021

bellenot commented Feb 16, 2021

ferdymercury commented Feb 16, 2021

ferdymercury commented Feb 16, 2021

ROOT crash when parameter by value passed to slot: StrDup #7195

ROOT crash when parameter by value passed to slot: StrDup #7195

Comments

ferdymercury commented Feb 12, 2021 • edited

Describe the bug

Expected behavior

To Reproduce

Setup

bellenot commented Feb 15, 2021

ferdymercury commented Feb 15, 2021

bellenot commented Feb 15, 2021

ferdymercury commented Feb 15, 2021

bellenot commented Feb 15, 2021

ferdymercury commented Feb 15, 2021 • edited

bellenot commented Feb 15, 2021

ferdymercury commented Feb 16, 2021 • edited

bellenot commented Feb 16, 2021 • edited

ferdymercury commented Feb 16, 2021

bellenot commented Feb 16, 2021

ferdymercury commented Feb 16, 2021

ferdymercury commented Feb 16, 2021

ferdymercury commented Feb 12, 2021 •

edited

ferdymercury commented Feb 15, 2021 •

edited

ferdymercury commented Feb 16, 2021 •

edited

bellenot commented Feb 16, 2021 •

edited