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
