UCS/QUEUE: Fix ucs_queue_for_each() with armclang #2968

yosefe · 2018-10-19T11:49:44Z

armclang assumes an address of struct field cannot be NULL, so the exit condition in ucs_queue_for_each() does not work. Fix by checking the pointer to the struct instead of its member.

swx-jenkins1 · 2018-10-19T12:52:22Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5444/ for details.

mellanox-github · 2018-10-19T14:30:49Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8007/ for details (Mellanox internal link).

shamisp · 2018-10-19T14:46:03Z

@yosefe looks like clang thing, not arm specific.
@nSircombe please take a look.

shamisp · 2018-10-19T15:09:48Z

Also, since this is compilation issues, IMHO it should be back ported to v1.4.x tree.

nSircombe · 2018-10-19T16:06:26Z

Yes, the issue also effects clang.

Had a quick go with clang (LLVM5) and Armclang (a version built off of LLVM7), both now fail on test_callbackq_noflags.remove_if...

[----------] 2 tests from test_callbackq_noflags, where TypeParam = 
[ RUN      ] test_callbackq_noflags.oneshot
[       OK ] test_callbackq_noflags.oneshot (0 ms)
[ RUN      ] test_callbackq_noflags.remove_if
[man-08:32748:0:32748]   callbackq.c:180  Assertion `idx <= last_idx' failed

yosefe · 2018-10-22T08:59:53Z

@nSircombe @shamisp i've tried with 'Arm C/C++/Fortran Compiler version 18.4.2 (build number 4) (based on LLVM 5.0.2)' and did not see a problem with 'test_callbackq_noflags.remove_if'
Do we have any system with remote access which shows the issue?

swx-jenkins1 · 2018-10-22T13:20:09Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5452/ for details.

mellanox-github · 2018-10-22T13:36:00Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8028/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T13:36:00Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8018/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T13:36:00Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8022/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T13:36:00Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8026/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T13:36:00Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8016/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T13:36:00Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8021/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T13:36:01Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8025/ for details (Mellanox internal link).

swx-jenkins1 · 2018-10-22T14:16:43Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5453/ for details.

mellanox-github · 2018-10-22T15:17:58Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8029/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T15:18:14Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8033/ for details (Mellanox internal link).

mellanox-github · 2018-10-22T15:18:21Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8032/ for details (Mellanox internal link).

swx-jenkins1 · 2018-10-22T15:24:28Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5456/ for details.

swx-jenkins1 · 2018-10-22T16:09:15Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5457/ for details.

nSircombe · 2018-10-22T16:46:14Z

Huh, ok. …let me try again on a clean checkout and get back to you. I don’t know if there are any suitable & affected systems with remote access.

swx-jenkins1 · 2018-10-22T17:19:45Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5460/ for details.

swx-jenkins1 · 2018-10-22T17:59:49Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5461/ for details.

shamisp · 2018-10-22T18:06:26Z

@yosefe Do we need backport this ?

mellanox-github · 2018-10-22T19:16:21Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8035/ for details (Mellanox internal link).

swx-jenkins1 · 2018-10-22T19:48:22Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5465/ for details.

mellanox-github · 2018-10-22T20:32:14Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8045/ for details (Mellanox internal link).

swx-jenkins1 · 2018-10-22T20:56:54Z

Test FAILed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5472/ for details.

mellanox-github · 2018-10-22T23:27:10Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8047/ for details (Mellanox internal link).

yosefe · 2018-10-23T05:11:58Z

@hoopoepg can you take a look again?

hoopoepg · 2018-10-23T06:25:50Z

test/gtest/ucs/test_datatype.cc

-    ucs_queue_push(&head, &elem3->queue);
-    ucs_queue_push(&head, &elem4->queue);
-
+    for (int i = 0; i < 4; ++i) {


minor:

for (int i = 0; i < elems.size(); ++i) {

swx-jenkins1 · 2018-10-23T09:38:04Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/5483/ for details.

mellanox-github · 2018-10-23T12:06:53Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/8055/ for details (Mellanox internal link).

shamisp · 2018-10-23T16:00:37Z

@nSircombe did it resolve the problem ?
@yosefe backport ?

nSircombe · 2018-10-23T22:44:29Z

@shamisp - yep, it builds and runs with the latest release (Armclang 18.4.2) - thanks @yosefe!

#2968 backport

UCS/QUEUE: Fix ucs_queue_for_each() with armclang

be69732

armclang assumes an address of struct field cannot be NULL, so the exit condition in ucs_queue_for_each() does not work. Fix by checking the pointer to the struct instead of its member.

yosefe added the Bugfix label Oct 19, 2018

hoopoepg approved these changes Oct 19, 2018

View reviewed changes

yosefe added 2 commits October 22, 2018 14:34

TEST/DATATYPE: Fix memory leak in case ASSERT_EQ fails

cebfcdb

CONTRIB/JENKINS: Run gtest with armclang

e5a4b96

yosefe added the Critical v1.4.x label Oct 22, 2018

hoopoepg approved these changes Oct 23, 2018

View reviewed changes

TEST/DATATYPE: Add const def for number of pushed elements

0d26720

hoopoepg approved these changes Oct 23, 2018

View reviewed changes

yosefe merged commit 589f5c0 into openucx:master Oct 23, 2018

yosefe deleted the topic/ucs-queue-iter-fix-segv branch October 23, 2018 12:58

shamisp mentioned this pull request Oct 23, 2018

#2968 backport #2977

Merged

shamisp added a commit that referenced this pull request Oct 24, 2018

Merge pull request #2977 from shamisp/topic/v140/2968_backport

7c80fad

#2968 backport

UCS/QUEUE: Fix ucs_queue_for_each() with armclang #2968

UCS/QUEUE: Fix ucs_queue_for_each() with armclang #2968

Conversation

yosefe commented Oct 19, 2018 • edited Loading

swx-jenkins1 commented Oct 19, 2018

mellanox-github commented Oct 19, 2018

shamisp commented Oct 19, 2018

shamisp commented Oct 19, 2018

nSircombe commented Oct 19, 2018 • edited Loading

yosefe commented Oct 22, 2018

swx-jenkins1 commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

swx-jenkins1 commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

swx-jenkins1 commented Oct 22, 2018

swx-jenkins1 commented Oct 22, 2018

nSircombe commented Oct 22, 2018 via email • edited Loading

swx-jenkins1 commented Oct 22, 2018

swx-jenkins1 commented Oct 22, 2018

shamisp commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

swx-jenkins1 commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

swx-jenkins1 commented Oct 22, 2018

mellanox-github commented Oct 22, 2018

yosefe commented Oct 23, 2018

hoopoepg Oct 23, 2018

Choose a reason for hiding this comment

swx-jenkins1 commented Oct 23, 2018

mellanox-github commented Oct 23, 2018

shamisp commented Oct 23, 2018

nSircombe commented Oct 23, 2018

yosefe commented Oct 19, 2018 •

edited

Loading

nSircombe commented Oct 19, 2018 •

edited

Loading

nSircombe commented Oct 22, 2018 via email •

edited

Loading