On 05/10/2018 13:43, Daniel Hellstrom wrote:
On 2018-10-05 13:01, Sebastian Huber wrote:
On 05/10/2018 12:44, Daniel Hellstrom wrote:

On 2018-10-05 09:41, Sebastian Huber wrote:
On 05/10/2018 09:38, Daniel Hellstrom wrote:

On 2018-10-05 09:34, Sebastian Huber wrote:
On 05/10/2018 09:25, Daniel Hellstrom wrote:
On 2018-10-05 09:12, Sebastian Huber wrote:
On 05/10/2018 08:57, Daniel Hellstrom wrote:
This fixes the following test failures on LEON3 UP/SMP when
built using clang compiler:
  * fsjffs2gc01
  * jffs2_fserror
  * jffs2_fspermission
  * jffs2_fsrdwr
  * jffs2_fstime

The problem is probably in the rbtree_postorder_for_each_entry_safe(). Coverity Scan has also an issue with this macro. It is on my todo list. Please do not check in this patch.
Ok I will not push it. Yes I was also thinking so. To use offsetof() macro could be an option maybe, however the node type input is not known to the rbtree_postorder_for_each_entry_safe() function even though it only used from jffs2 code.

The Linux code uses typeof. We could do this also:

diff --git a/cpukit/include/linux/rbtree.h b/cpukit/include/linux/rbtree.h
index 53c777e8c1..8fc575240f 100644
--- a/cpukit/include/linux/rbtree.h
+++ b/cpukit/include/linux/rbtree.h
@@ -126,12 +126,12 @@ static inline struct rb_node *rb_parent( struct rb_node *node )
   for ( \
     node = _RBTree_Postorder_first( \
       (RBTree_Control *) root, \
-      (size_t) ( (char *) &node->field - (char *) node ) \
+      offsetof( __typeof__( *node ), field ) \
     ); \
     node != NULL && ( \
       next = _RBTree_Postorder_next( \
         &node->field, \
-        (size_t) ( (char *) &node->field - (char *) node ) \
+        offsetof( __typeof__( *node ), field ) \
       ), \
       node != NULL \
     ); \

It shouldn't be a big problem since at least GCC and clang supports it. It is also a standard C++ operator. It is already used in

cpukit/libfs/src/jffs2/include/linux/list.h

Ok, I will give it a try.


Thanks, please try this patch:

https://lists.rtems.org/pipermail/devel/2018-October/023181.html

Yes I can confirm it fixed all the 5 tests on gr712rc with clang. It also removed the two warnings in jffs2 code related.

I have updated my patch to only include the third warning fix then:

---
 cpukit/libfs/src/jffs2/src/readinode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpukit/libfs/src/jffs2/src/readinode.c b/cpukit/libfs/src/jffs2/src/readinode.c
index 519b0d6..ecdf335 100644
--- a/cpukit/libfs/src/jffs2/src/readinode.c
+++ b/cpukit/libfs/src/jffs2/src/readinode.c
@@ -35,7 +35,7 @@ static int check_node_data(struct jffs2_sb_info *c, struct jffs2_tmp_dnode_info
     struct jffs2_raw_node_ref *ref = tn->fn->raw;
     int err = 0, pointed = 0;
     struct jffs2_eraseblock *jeb;
-    unsigned char *buffer;
+    unsigned char *buffer = NULL;
     uint32_t crc, ofs, len;
     size_t retlen;

What is this for a warning? Some sort of uninitialized variable warning? This code is identical to the Linux upstream. This is probably a clang bug.

buffer will never be uninitialized from what I see, so yes maybe its a clang bug. This is the warning:


sparc-gaisler-rtems5-clang --pipe -DHAVE_CONFIG_H   -I.. -I/home/daniel/git/rtems/rcc-1.3/rtems/build/gr712rc_drvmgr_test_up/sparc-gaisler-rtems5/c/gr712rc/include -I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/cpukit/include -I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/cpukit/score/cpu/sparc/include -I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/cpukit/libnetworking -I/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/include -Wno-pointer-sign -mcpu=gr712rc -O2 -g -ffunction-sections -fdata-sections -Wall -Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes -Wnested-externs -MT src/jffs2/src/libjffs2_a-readinode.o -MD -MP -MF src/jffs2/src/.deps/libjffs2_a-readinode.Tpo -c -o src/jffs2/src/libjffs2_a-readinode.o `test -f 'src/jffs2/src/readinode.c' || echo '/home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/'`src/jffs2/src/readinode.c /home/daniel/git/rtems/rcc-1.3/rtems/kernel/c/src/../../cpukit/libfs/src/jffs2/src/readinode.c:80:6: warning: variable 'buffer' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
        if (!pointed) {
            ^~~~~~~~

We have

int pointed = 0;

and the address of pointed is not taken, so this condition will be always true. I am not sure what the constraints of the -Wsometimes-uninitialized warnings are. Maybe they should trigger in case code modifications elsewhere might lead to the offending condition. Coverity Scan doesn't complain about this one.

Please have a look at this patch:

https://lists.rtems.org/pipermail/devel/2018-October/023190.html

In contrast to the buffer = NULL initialization this patch entirely avoids the problematic path which must the compiler follow to deduces this warning.

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to