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.gouaillar...@gmail.com> wrote:
> Ralph,
>
>
> On Wed, May 28, 2014 at 9:53 PM, Ralph Castain <r...@open-mpi.org> 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.gouaillar...@gmail.com> 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
> de...@open-mpi.org
> 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
