Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] Trunk (RDMA and VT) warnings
From: Ralph Castain (rhc_at_[hidden])
Date: 2014-05-29 09:41:26


Not seeing it on fresh checkout of today's trunk head, so something may have resolved it since last test

On May 28, 2014, at 10:27 PM, Gilles Gouaillardet <gilles.gouaillardet_at_[hidden]> wrote:

> Ralph,
>
>
> On Wed, May 28, 2014 at 9:53 PM, Ralph Castain <rhc_at_[hidden]> wrote:
> gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-4)
> ./configure --prefix=/home/common/openmpi/build/svn-trunk --enable-mpi-java --enable-orterun-prefix-by-default
>
> More inline below
>
>
> this looks like an up-to-date CentOS box.
> i am unable to reproduce the warnings (may be uninitialized in this function) with a similar box :-(
>
>
> On May 27, 2014, at 9:29 PM, Gilles Gouaillardet <gilles.gouaillardet_at_[hidden]> wrote:
> so far, it seems this is a false positive/compiler bug that could be triggered by inlining
>> /* i could not find any path in which the variable is used unitialized */
>
> I just glanced at the first one (line 221 of osc_rdma_data_move.c), and I can see what the compiler is complaining about - have gotten this in other places as well. The problem is that you pass the address of ptr into a function without first initializing the value of ptr itself. There is no guarantee (so far as the compiler can see) that this function will in fact set the value of ptr - you are relying solely on the fact that (a) you checked that function at one point in time and saw that it always gets set to something if ret == OMPI_SUCCESS, and (b) nobody changed that function since you checked.
>
> Newer compilers seem to be getting more defensive about such things and starting to "bark" when they see it. I think you are correct that inlining also impacts that situation, though I've also been seeing it when the functions aren't inlined.
>
>
> i wrote the simple test program :
>
> #include <string.h>
>
> char * mystring = "hello";
> static inline int setif(int mustset, char **ptr) {
> if (!mustset) {
> return 1;
> }
> *ptr = mystring;
> return 0;
> }
>
> void good(int mustset) {
> char * ptr;
> char buf[256];
> if (setif(mustset, &ptr) == 0) {
> memcpy(buf, ptr, 6);
> }
> }
>
> void bad(int mustset) {
> char * ptr;
> char buf[256];
> if (setif(mustset, &ptr) != 0) {
> memcpy(buf, ptr, 6);
> }
> }
>
> please note that :
> - the setif function is declared 'inline'
> - the setif will set *ptr only if the 'mustset' parameter is nonzero and then return 0
> - the setif will leave *ptr unmodified if the 'mustset' parameter is zero and then return 1
>
> it is trivial that the 'good' function is ok whereas the 'bad' function has an issue :
> the compiler has a way to figure out that ptr will be uninitialized when invoking memcpy
> (since setif returned a non zero status)
>
> gcc -Wall -O0 test.c
> does not complain
>
> gcc -Wall -O1 test.c *does* complain
> test.c:24: warning: ‘ptr’ may be used uninitialized in this function
>
> if the 'inline' keyword is omitted, -O2 is needed to get a compiler warning.
>
> bottom line, an optimized build (-O3 -finline-functions) correctly issues a warning.
> i checked osc_rdma_data_move.c and osc_rdma_frag.h again and again and i could not find how ptr can be uninitialized in ompi_osc_rdma_control_send if
> ompi_osc_rdma_frag_alloc returned OMPI_SUCCESS
> /* not to mention i am unable to reproduce the warning */
>
> about the compiler getting more defensive :
>
> { int rank;
> MPI_Comm_rank(MPI_COMM_WORLD, &rank);
> rank++;
> }
>
> i never saw a compiler issue a warning about rank that could be used uninitialized
>
>
> Not sure what to suggest here - hate to add initialization steps in that sequence....
>
> me too, and i do not see any warnings from the compiler
>
> can you please confirm you can reproduce the issue on the most up to date trunk revision , on a x86_64 box (never knows ...) ?
> then can you send the output of
>
> cd ompi/mca/osc/rdma
> touch osc_rdma_data_move.c
> make -n osc_rdma_data_move.lo
>
>
> Cheers,
>
> Gilles
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel
> Link to this post: http://www.open-mpi.org/community/lists/devel/2014/05/14902.php