Valgrind problem with root 5.17/02

Dear Rooters

I have just tested my program with the official root version 5.17/02 using valgrind
and got the following error message:

------XTreeSet::WriteTree------
==12801== 
==12801== Conditional jump or move depends on uninitialised value(s)
==12801==    at 0x1BAD7E53: fill_window (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BAD8295: deflate_fast (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BAD736A: deflate (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BAD42B7: R__zip (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1CC9A86C: TKey::TKey(TObject const*, char const*, int, TDirectory*) (in /home/Rabbitus/ROOT/root/lib/libRIO.so)
==12801==    by 0x1CC819FC: TFile::CreateKey(TDirectory*, TObject const*, char const*, int) (in /home/Rabbitus/ROOT/root/lib/libRIO.so)
==12801==    by 0x1CC757E4: TDirectoryFile::WriteTObject(TObject const*, char const*, char const*, int) (in /home/Rabbitus/ROOT/root/lib/libRIO.so)
==12801==    by 0x1BA3941B: TObject::Write(char const*, int, int) const (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BA37EA2: TObject::Write(char const*, int, int) (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1DD0C762: XTreeSet::WriteTree(TTree*, int, int) (in /home/Rabbitus/ROOT/rootcode/xps-x.x.x/src_0.3.5/xps.so)
==12801==    by 0x1DD8AF76: XGCProcesSet::AdjustBackground(int, TTree**, int&, TTree**) (in /home/Rabbitus/ROOT/rootcode/xps-x.x.x/src_0.3.5/xps.so)
==12801==    by 0x1DD8A0F2: XGCProcesSet::Preprocess(char const*) (in /home/Rabbitus/ROOT/rootcode/xps-x.x.x/src_0.3.5/xps.so)
==12801== 
==12801== More than 30000 total errors detected.  I'm not reporting any more.
==12801== Final error counts will be inaccurate.  Go fix your program!
==12801== Rerun with --error-limit=no to disable this cutoff.  Note
==12801== that errors may occur in your program without prior warning from
==12801== Valgrind, because errors are no longer being displayed.
==12801== 

What does this message mean, is this a problem with tree->Write()?

Since I am not able to tell if this is a problem with my program or with the new
root version, I have also tested a macro which fills and reads a tree.
The attached macro “macroTree1.C” is a modified version of tutorial “hsimple.C”.
Interestingly, this causes valgrind errors, too. I have attached the complete
output as file “valgrind_output.txt”.

Can you tell me what the problem could be in both cases?

Best regards
Christian
valgrind_output.txt (83 KB)
macroTree1.C (4.15 KB)

Hi,

the valgrind reports on TString in tree1w(int) are most probably issues with your code. I ha da look at the R__zip issue once; the reasonw as that the zip algorithm was looking at bytes that were not meant to be zipped (overruns). In the end it didn’t matter as it does not write them out. But zeroing these additional bytes removed the valgrind warning (with seemingly identical zip results). So I’d say: a good candidate for the suppression file, just like the X11 reports that you might get. The leak reports are all but two expected; I have forwarded these two reports to the relevant people.

Axel.

You have a bug in your program. Replace the line

void SetString(const char *str) {fString = TString(str,32);} by

void SetString(const char *str) {fString = str;}
fString = TString(str,32) is not only inefficient (requires an intermediate object), but it is also wrong, because you try to copy the 32 first characters of str and in your calling program str is created with less than 32 chars.

Rene

Dear Axel, dear Rene

Thank you for your explanations.
I must admit that it was not quite clear to me what Axel means, but luckily Rene made the point.

I would have never believed, that calling TString(str,32) is a bug. In my program I have
this type of code at a couple of places, because I thought, when (according to the database
description) a string can have at maximum 32 characters, I will be on the save side, when
calling TString(str, 32).

In my program I have e.g. the following code fragment:

      if (idxstr) {
         k = idxstr->GetIndex();
         ann->SetName(arrName[k], (arrName[k]).Length());
         ann->SetSymbol(arrSymb[k], (arrSymb[k]).Length());
      } else {
         ann->SetName("NA", 2);
         ann->SetSymbol("NA", 2);
      }//if

According to your comment, this code should at least be correct?
Do you think, that in this case it is also better to replace the code with:

      if (idxstr) {
         k = idxstr->GetIndex();
         ann->SetName(arrName[k]);
         ann->SetSymbol(arrSymb[k]);
      } else {
         ann->SetName("NA");
         ann->SetSymbol("NA");
      }//if

Best regards
Christian

Dear Axel, dear Rene

When testing different parts of my program I still get the following error message:

------XTreeSet::WriteTree------
==12801== 
==12801== Conditional jump or move depends on uninitialised value(s)
==12801==    at 0x1BAD7E53: fill_window (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BAD8295: deflate_fast (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BAD736A: deflate (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BAD42B7: R__zip (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1CC9A86C: TKey::TKey(TObject const*, char const*, int, TDirectory*) (in /home/Rabbitus/ROOT/root/lib/libRIO.so)
==12801==    by 0x1CC819FC: TFile::CreateKey(TDirectory*, TObject const*, char const*, int) (in /home/Rabbitus/ROOT/root/lib/libRIO.so)
==12801==    by 0x1CC757E4: TDirectoryFile::WriteTObject(TObject const*, char const*, char const*, int) (in /home/Rabbitus/ROOT/root/lib/libRIO.so)
==12801==    by 0x1BA3941B: TObject::Write(char const*, int, int) const (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1BA37EA2: TObject::Write(char const*, int, int) (in /home/Rabbitus/ROOT/root/lib/libCore.so)
==12801==    by 0x1DD0C762: XTreeSet::WriteTree(TTree*, int, int) (in /home/Rabbitus/ROOT/rootcode/xps-x.x.x/src_0.3.5/xps.so)
==12801==    by 0x1DD8AF76: XGCProcesSet::AdjustBackground(int, TTree**, int&, TTree**) (in /home/Rabbitus/ROOT/rootcode/xps-x.x.x/src_0.3.5/xps.so)
==12801==    by 0x1DD8A0F2: XGCProcesSet::Preprocess(char const*) (in /home/Rabbitus/ROOT/rootcode/xps-x.x.x/src_0.3.5/xps.so)

I get the error when executing the line “tree->Write(”", TObject::kOverwrite)".

However, when running valgrind with option “–error-limit=no” I realized that I get this
message only for the very first tree that I store in TFile, but not for all other trees.

What does this message mean and how can I get rid of it?

Best regards
Christian

Hi,

as I said: “I had a look at the R__zip issue once; the reasonw as that the zip algorithm was looking at bytes that were not meant to be zipped (overruns). In the end it didn’t matter as it does not write them out. But zeroing these additional bytes removed the valgrind warning (with seemingly identical zip results). So I’d say: a good candidate for the suppression file, just like the X11 reports that you might get.” Check valgrind’s doc on how to generate a suppression file.

Cheers, Axel.

The message from valgrind means that you are streaming one object that contains uninitialized data.

Rene

Dear Rene, dear Axel

Before calling “tree->Write()” I am calling the following function:

void XGeneChipHyb::AddDataTreeInfo(TTree *tree, const char *name, Option_t *option)
{
   XDataTreeInfo *info = new XDataTreeInfo(name, "");

   info->SetTitle(info->ClassName());
   info->SetOption(option);
   info->SetTreeSetName(GetName());
   info->SetTreeSetClass(ClassName());
   info->AddUserInfo(this);

   tree->GetUserInfo()->Add(info);
}//AddDataTreeInfo

By default, option is set to <option = “”;>. Could this be the reason?

Best regards
Christian

Coul you comment the line
info->AddUserInfo(this);
and run valgring again? If valgrind does not complain, it will mean that you have some undefined data in the class that you attach to the user info.

Rene

Dear Rene

This did not help, even commenting “tree->GetUserInfo()->Add(info);” did not solve the problem.

BTW, when running my library as part of my R package, valgrind does not complain at all.
I am not sure what this means.

Best regards
Christian

Christian,

Could you send the strict minimum set of files reproducing the problem?

Rene

Dear Rene

Thank you very much for your willingness to evaluate my problem. I will attach a downgraded version
of my program as private message, together with the valgrind output.

Best regards
Christian

Christian,

I do not see any leaks when following your instructions to reproduce the problem.
I untarred your file
make
at thsi point I have your shared lib xps, then I run root.exe under valgrind. Here is the result of the session (under Linux SLC4, gcc3.4.6).

Rene

root [0] .L macroImportData.C
root [1] Init()
root [2] ImportDataTest3("DataTest3")
Warning: No directory given to store root file:
         Using working directory </home/brun/rootcmz/xps4Rene>
Creating new file </home/brun/rootcmz/xps4Rene/DataTest3_cel.root>...
Opening file <SchemeTest3.root> in <READ> mode...
Importing </home/brun/rootcmz/xps4Rene/TestA1.CEL> as <TestA1.cel>...
<15876> records imported...
   hybridization statistics:
      1 cells with minimal intensity 548.7
      1 cells with maximal intensity 46266
New dataset <DataSet> is added to Content...
Importing </home/brun/rootcmz/xps4Rene/TestA2.CEL> as <TestA2.cel>...
<15876> records imported...
   hybridization statistics:
      1 cells with minimal intensity 555.8
      1 cells with maximal intensity 46265.2
root [3] .q
==28606==
==28606== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 78 from 3)
==28606== malloc/free: in use at exit: 5,398,552 bytes in 75,486 blocks.
==28606== malloc/free: 834,736 allocs, 759,250 frees, 41,736,236 bytes allocated.
==28606== For counts of detected errors, rerun with: -v
==28606== searching for pointers to 75,486 not-freed blocks.
==28606== checked 8,311,592 bytes.

Dear Rene

Thank you very much, I am really glad to hear this.

The question remains, why I get these messages:
Maybe, Fedora Core 4 is already too old and/or the valgrind version coming with FC4 is too old. I have never updated FC4.
(Sometimes in the future I want to upgrade to C7)

Best regards
Christian

I was seeing the same valgrind warnings with root 5.16/00 and 5.17/04 with our code, and I managed to track it down and produce a simple example which works reproducibly on my Scientific Linux 4.5 machine. It’s really just a feature of the zlib library, as described in ZLIB FAQ #36.

You can suppress the warning with a valgrind “suppression rule”. I’ve attached a suitable valgrind suppression rule below. It’s similar to but much more restrictive than a rule the python developers use. Just add “–suppressions=zlib_deflate.supp” to your valgrind options.

If anyone is really interested in the gory details, the “conditional jump or move” that upsets valgrind is the one that’s immediately followed by the comment “If n is not on any hash chain, prev[n] is garbage but its value will never be used” in deflate.c. It would be possible to add some extra initialization to deflate.c (and slow down the compression slightly) just to avoid the valgrind warning, but I think using the suppression rule is better.

Happy valgrinding!
zlib_deflate.supp.txt (209 Bytes)
demo_zip_valgrind_issue_8.C (614 Bytes)

Let me stress again that is NOT a feature of the zipping library but the sign that you are streaming an object containing uninitialized data.

Rene

Sorry, I should have been more clear. Yes, streaming out an object containing uninitialized data will produce a valgrind warning which is a sign of a problem in the code of the user’s application, as in the improperly initialized TString case discussed near the start of this thread. Typically, these messages will refer to the conditional jump or move occurring “longest_match”, and in addition, if you’ve really got a problem, you’ll get a message warning “Syscall param write(buf) points to uninitialised byte(s)”. If you see that, you know you’ve got a problem.

Futhermore and additionally, the zip library will indeed sometimes produce a “conditional jump or move” warning from fill_window in a very specific case where it has processed many similar byte strings and needs to clear an internal table, as demonstrated by the example I attached which does not have any uninitialized data in the objects being streamed. These warnings from valgrind are a result of super-optimization of the deflate algorithm by the ZLIB authors, and is a feature not a bug (according to them), as alluded to in the ZLIB FAQ and in the comment in the source code of deflate.c.

It is important to not let the second issue cloud the first one, which is why I suggest using the suppression rule (which I think was Axel’s point, too). The suppression rule I provided is specific enough that streaming out an object with uninitialized data will still produce a valgrind error while the super-optimized piece of the deflate code will not.

I hope that’s helpful.

Best regards,
Glenn.

Glenn,

Thanks for posting this convincing report.

Rene

Thank you, next time I encounter this problem I will try your suppression file.

Best regards
Christian

Hi,

it’s now part of ROOT’s suppression file $ROOTSYS/etc/valgrind-root.supp.

Cheers, Axel.