I think the following change to str.c, in str_strcpy_internal(), fixes the valgrind issues. It was easier to track down after the calloc() was changed to malloc(), since before it only happened after a realloc(). $ diff -u rbibutils/src/str.c rbibutils-fixed/src/str.c --- rbibutils/src/str.c 2021-06-16 04:13:48.000000000 -0700 +++ rbibutils-fixed/src/str.c 2021-08-06 18:38:56.763556500 -0700 @@ -217,8 +217,8 @@ unsigned long size = str_initlen; assert( s ); if ( minsize > str_initlen ) size = minsize; - // 2021-06-16 was: s->data = (char *) malloc( sizeof( *(s->data) ) * size ); - // changing to calloc() to avoid this kind of error from valgrind: + s->data = (char *) malloc( sizeof( *(s->data) ) * size ); + // tried changing to calloc() to avoid this kind of error from valgrind: // > bibConvert(fn_med, bib, informat = "med") // ==16041== Conditional jump or move depends on uninitialised value(s) // ==16041== at 0x10CCF2A3: xml_processtag (xml.c:174) @@ -249,8 +249,9 @@ // // TODO: // The data is not really left uninitialised and there may be a better way to let the compiler know. + // WWD: fixing str_strcpy_internal takes care of memory misuse. // - s->data = (char *) calloc( size, sizeof( *(s->data) ) ); + // s->data = (char *) calloc( size, sizeof( *(s->data) ) ); if ( !s->data ) { error("Error. Cannot allocate memory in str_initalloc, requested %lu characters.\n\n", size ); // error("\n"); // error( EXIT_FAILURE ); @@ -552,9 +553,9 @@ // Georgi: this fixes the warning about truncation in strncpy // strcpy cannot be used here since at least one of the calls below // passes a non-NULL terminated 'p' - strncpy( s->data, p, n + 1); - // strncpy( s->data, p, n ); - // s->data[n] = '\0'; + // strncpy( s->data, p, n + 1); // WWD: ??? + strncpy( s->data, p, n ); + s->data[n] = '\0'; s->len = n; }
I don't know why valgrind isn't working for you. -Bill On Mon, Aug 2, 2021 at 4:23 PM Georgi Boshnakov < georgi.boshna...@manchester.ac.uk> wrote: > Thanks for looking into this. I probably didn’t make it clear that I am > not questioning the errors on CRAN but my own configuration. I have pretty > good idea where the error comes from, since the only change from the > previous CRAN version was in medin.c (function medin_readf from line 94 and > most probably line 144-145). I think also that all errors have medin.c on > the trace. > > > > It would be very helpful if somebody can spot from the description of my > configuration in the original email where I have gone wrong in the setup of > R with valgrind. > > > > >realloc() does not initialize memory. str.c contains a comment about > replacing malloc() with calloc() to avoid… > > > > // The data is not really left uninitialised and there may > be a better way to let the compiler know. > > // > > s->data = (char *) calloc( size, sizeof( *(s->data) ) ); > > if ( !s->data ) { > > error("Error. Cannot allocate memory in str_initalloc, > requested %lu characters.\n\n", size ); > > // error("\n"); // error( EXIT_FAILURE ); > > } > > s->data[0]='\0'; > > s->dim=size; > > s->len=0; > > > > My comment is indeed sloppy but the first byte is initialised to zero and > the rest is used for writing only > > (valgrind has no way to know, of course, and it is fair to question how a > human can possibly be sure). > > > > > > Thanks again, > > Georgi Boshnakov > > > > > > *From:* Bill Dunlap <williamwdun...@gmail.com> > *Sent:* 02 August 2021 19:48 > *To:* Georgi Boshnakov <georgi.boshna...@manchester.ac.uk> > *Cc:* r-package-devel@r-project.org > *Subject:* Re: [R-pkg-devel] can't reproduce 'Additional issues' on CRAN > with valgrind > > > > I ran the tests of rbibutils-2.2.2, using the latest devel version of R > configured to use valgrind, with > > > > R --debugger=valgrind --debugger-args=--track-origins=yes --quiet -e > 'testthat::test_package("rbibutils")' > > > > I saw a lot of 'conditional jump depends on uninitialized value' errors: > > > > ==27280== Conditional jump or move depends on uninitialised value(s) > ==27280== at 0x11C420B7: UnknownInlinedFun (xml.c:178) > ==27280== by 0x11C420B7: xml_parse (xml.c:241) > ==27280== by 0x11C514EF: medin_processf (medin.c:683) > ==27280== by 0x11C67ADE: UnknownInlinedFun (bibcore.c:479) > ==27280== by 0x11C67ADE: bibl_read.part.0 (bibcore.c:862) > ==27280== by 0x11C68C66: bibprog (bibprog.c:36) > ==27280== by 0x11C69327: any2xml_main (any2xml.c:127) > ==27280== by 0x307639: do_dotCode (dotcode.c:1814) > ==27280== by 0x2B4600: bcEval.lto_priv.0 (eval.c:7128) > ==27280== by 0x2DADB7: Rf_eval (eval.c:740) > ==27280== by 0x2DD28E: R_execClosure.lto_priv.0 (eval.c:1910) > ==27280== by 0x2DE1EC: Rf_applyClosure (eval.c:1836) > ==27280== by 0x2DAFD3: Rf_eval (eval.c:863) > ==27280== by 0x2D7F22: do_begin (eval.c:2530) > ==27280== Uninitialised value was created by a heap allocation > ==27280== at 0x483DFAF: realloc (in > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > ==27280== by 0x11C38DB2: str_realloc.part.0 (str.c:90) > ==27280== by 0x11C3B63D: UnknownInlinedFun (str.c:85) > ==27280== by 0x11C3B63D: UnknownInlinedFun (str.c:543) > ==27280== by 0x11C3B63D: UnknownInlinedFun (str.c:551) > ==27280== by 0x11C3B63D: UnknownInlinedFun (str.c:547) > ==27280== by 0x11C3B63D: UnknownInlinedFun (str.c:607) > ==27280== by 0x11C3B63D: str_segcpy (str.c:585) > ==27280== by 0x11C4DDFA: medin_readf (medin.c:144) > ==27280== by 0x11C67A96: UnknownInlinedFun (bibcore.c:471) > ==27280== by 0x11C67A96: bibl_read.part.0 (bibcore.c:862) > ==27280== by 0x11C68C66: bibprog (bibprog.c:36) > ==27280== by 0x11C69327: any2xml_main (any2xml.c:127) > ==27280== by 0x307639: do_dotCode (dotcode.c:1814) > > > > realloc() does not initialize memory. str.c contains a comment about > replacing malloc() with calloc() to avoid similar problems that valgrind > found. It states that valgrind is mistaken, that that memory really is > initialized. Hmm. > > > > I also see the error about reading off the end of a malloc'ed array: > > > > ==27280== Invalid read of size 1 > ==27280== at 0x11C420A8: UnknownInlinedFun (xml.c:174) > ==27280== by 0x11C420A8: xml_parse (xml.c:241) > ==27280== by 0x11C514EF: medin_processf (medin.c:683) > ==27280== by 0x11C67ADE: UnknownInlinedFun (bibcore.c:479) > ==27280== by 0x11C67ADE: bibl_read.part.0 (bibcore.c:862) > ==27280== by 0x11C68C66: bibprog (bibprog.c:36) > ==27280== by 0x11C69327: any2xml_main (any2xml.c:127) > ==27280== by 0x307639: do_dotCode (dotcode.c:1814) > ==27280== by 0x2B4600: bcEval.lto_priv.0 (eval.c:7128) > ==27280== by 0x2DADB7: Rf_eval (eval.c:740) > ==27280== by 0x2DD28E: R_execClosure.lto_priv.0 (eval.c:1910) > ==27280== by 0x2DE1EC: Rf_applyClosure (eval.c:1836) > ==27280== by 0x2DAFD3: Rf_eval (eval.c:863) > ==27280== by 0x2D7F22: do_begin (eval.c:2530) > ==27280== Address 0x125028af is 0 bytes after a block of size 10,927 > alloc'd > ==27280== at 0x483DD99: calloc (in > /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) > ==27280== by 0x11C38DF2: str_initalloc (str.c:253) > ==27280== by 0x11C3B66A: UnknownInlinedFun (str.c:541) > ==27280== by 0x11C3B66A: UnknownInlinedFun (str.c:551) > ==27280== by 0x11C3B66A: UnknownInlinedFun (str.c:547) > ==27280== by 0x11C3B66A: UnknownInlinedFun (str.c:607) > ==27280== by 0x11C3B66A: str_segcpy (str.c:585) > ==27280== by 0x11C4DDFA: medin_readf (medin.c:144) > > > > > > On Mon, Aug 2, 2021 at 10:11 AM Georgi Boshnakov < > georgi.boshna...@manchester.ac.uk> wrote: > > An update to my package rbibutils triggered 'Additional issues' on CRAN > from valgrind, CRAN Package Check Results for Package rbibutils ( > r-project.org)< > https://cran.r-project.org/web/checks/check_results_rbibutils.html>. > > However, running the checks with -use-valgrind on my machine gives zero > errors. > I am on Ubuntu (the latest stable version), my R-devel installation was > updated earlier today and I rebuild it adding the > --with-valgrind-instrumentation=2 to the call to configure. > Valgrind coming with Ubuntu is 3.5.0, so I installed its latest version > from github (also no errors). > > rbibutils has no dependencies, except that testthat is used for testing. > > What am I missing? Do I need to have a separate library for the > instrumented R-devel version? > > As a side note, is there a way to find out if R has been built with > valgrind? It seems that 'capabilities()' and 'R CMD config' do not offer > this information. > > Thanks, > Georgi Boshnakov > > [[alternative HTML version deleted]] > > ______________________________________________ > R-package-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-package-devel > > [[alternative HTML version deleted]] ______________________________________________ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel