Skip to content

Commit 075ae82

Browse files
markallemarkalle
committed
flatten fixes (count vs flatten divergence)
This bug is hit by the same code I used long ago, darray_bug_romio.c. I'm not totally sure on the archeology of when the bug reappeared, but I think it was because of changes to COMBINER_RESIZED in ADIOI_Flatten that weren't mirrored in ADIOI_Count_contiguous_blocks. Count and Flatten are very similar functions, but they diverge a lot around the edges and in degenerate cases. The two changes I made are 1. A book-keeping rule in Flatten is *curr_index should be updated in sync with whatever is being added to flat->indices[] and flat->blocklens[]. There were a couple places (when counts[] arrays have 0s) where that wasn't the case (see the "nonzeroth" var in this commit). 2. The main change I made was to reset flat->count based on Flatten's curr_index. This is because the divergence between the two functions is too great to reliably fix. 3. A third change is just a safety valve, using a FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT() macro just in case the Count function returns a smaller value than Flatten ends up trying to write into the array, and related to this I got rid of the various --(flat->count) lines, since that var now represents the allocated size of the array, up until right after the Flatten function when it's reset to represent the data written to the array like it used to be. Signed-off-by: Mark Allen <[email protected]>
1 parent 099cdb6 commit 075ae82

File tree

1 file changed

+68
-18
lines changed
  • ompi/mca/io/romio321/romio/adio/common

1 file changed

+68
-18
lines changed

‎ompi/mca/io/romio321/romio/adio/common/flatten.c‎

Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,16 @@ void ADIOI_Flatten_datatype(MPI_Datatype datatype)
168168
DBG_FPRINTF(stderr,"ADIOI_Flatten_datatype:: ADIOI_Flatten\n");
169169
#endif
170170

171+
/*
172+
* Setting flat->count to curr_index, since curr_index is the most fundamentally
173+
* correct updated value that represents what's in the indices/blocklens arrays.
174+
* It would be nice if the counter function and the flatten function were in sync,
175+
* but the numerous cases that decrement flat->count in the flatten function show
176+
* that syncing them is a hack, and as long as the counter doesn't under-count
177+
* it's good enough.
178+
*/
179+
flat->count = curr_index;
180+
171181
ADIOI_Optimize_flattened(flat);
172182
#endif
173183
/* debug */
@@ -184,6 +194,36 @@ void ADIOI_Flatten_datatype(MPI_Datatype datatype)
184194
#endif
185195
}
186196

197+
/*
198+
* I don't really expect this to ever trigger, but without the below safety
199+
* valve, the design relies on the Count function coming out >= whatever
200+
* the Flatten function comes up with. There are enough differences between
201+
* the two that it's hard to be positive this will always be true. So every
202+
* time something's added to flat's arrays, let's make sure they're big enough
203+
* and re-alloc if not.
204+
*/
205+
#define FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(idx) \
206+
do { \
207+
if (idx >= flat->count) { \
208+
int new_count = (flat->count * 1.25 + 4); \
209+
ADIO_Offset *new_blocklens = (ADIO_Offset *) \
210+
ADIOI_Malloc(new_count * sizeof(ADIO_Offset)); \
211+
ADIO_Offset *new_indices = (ADIO_Offset *) \
212+
ADIOI_Malloc(new_count * sizeof(ADIO_Offset)); \
213+
if (flat->count) { \
214+
memcpy(new_blocklens, flat->blocklens, \
215+
flat->count * sizeof(ADIO_Offset)); \
216+
memcpy(new_indices, flat->indices, \
217+
flat->count * sizeof(ADIO_Offset)); \
218+
ADIOI_Free(flat->blocklens); \
219+
ADIOI_Free(flat->indices); \
220+
} \
221+
flat->blocklens = new_blocklens; \
222+
flat->indices = new_indices; \
223+
flat->count = new_count; \
224+
} \
225+
} while (0)
226+
187227
/* ADIOI_Flatten()
188228
*
189229
* Assumption: input datatype is not a basic!!!!
@@ -318,6 +358,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
318358
if (prev_index == *curr_index) {
319359
/* simplest case, made up of basic or contiguous types */
320360
j = *curr_index;
361+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
321362
flat->indices[j] = st_offset;
322363
MPI_Type_size_x(types[0], &old_size);
323364
flat->blocklens[j] = top_count * old_size;
@@ -335,6 +376,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
335376
MPI_Type_get_extent(types[0], &lb, &old_extent);
336377
for (m=1; m<top_count; m++) {
337378
for (i=0; i<num; i++) {
379+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
338380
flat->indices[j] = flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
339381
flat->blocklens[j] = flat->blocklens[j-num];
340382
#ifdef FLATTEN_DEBUG
@@ -366,10 +408,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
366408
avoid >2G integer arithmetic problems */
367409
ADIO_Offset blocklength = ints[1], stride = ints[2];
368410
j = *curr_index;
411+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
369412
flat->indices[j] = st_offset;
370413
MPI_Type_size_x(types[0], &old_size);
371414
flat->blocklens[j] = blocklength * old_size;
372415
for (i=j+1; i<j+top_count; i++) {
416+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(i);
373417
flat->indices[i] = flat->indices[i-1] + stride * old_size;
374418
flat->blocklens[i] = flat->blocklens[j];
375419
}
@@ -389,6 +433,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
389433
MPI_Type_get_extent(types[0], &lb, &old_extent);
390434
for (m=1; m<blocklength; m++) {
391435
for (i=0; i<num; i++) {
436+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
392437
flat->indices[j] = flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
393438
flat->blocklens[j] = flat->blocklens[j-num];
394439
j++;
@@ -400,6 +445,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
400445
num = *curr_index - prev_index;
401446
for (i=1; i<top_count; i++) {
402447
for (m=0; m<num; m++) {
448+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
403449
flat->indices[j] = flat->indices[j-num] + stride * ADIOI_AINT_CAST_TO_OFFSET old_extent;
404450
flat->blocklens[j] = flat->blocklens[j-num];
405451
j++;
@@ -429,10 +475,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
429475
avoid >2G integer arithmetic problems */
430476
ADIO_Offset blocklength = ints[1];
431477
j = *curr_index;
478+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
432479
flat->indices[j] = st_offset;
433480
MPI_Type_size_x(types[0], &old_size);
434481
flat->blocklens[j] = blocklength * old_size;
435482
for (i=j+1; i<j+top_count; i++) {
483+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(i);
436484
flat->indices[i] = flat->indices[i-1] + adds[0];
437485
flat->blocklens[i] = flat->blocklens[j];
438486
}
@@ -452,6 +500,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
452500
MPI_Type_get_extent(types[0], &lb, &old_extent);
453501
for (m=1; m<blocklength; m++) {
454502
for (i=0; i<num; i++) {
503+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
455504
flat->indices[j] = flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
456505
flat->blocklens[j] = flat->blocklens[j-num];
457506
j++;
@@ -463,6 +512,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
463512
num = *curr_index - prev_index;
464513
for (i=1; i<top_count; i++) {
465514
for (m=0; m<num; m++) {
515+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
466516
flat->indices[j] = flat->indices[j-num] + adds[0];
467517
flat->blocklens[j] = flat->blocklens[j-num];
468518
j++;
@@ -500,16 +550,15 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
500550
avoid >2G integer arithmetic problems */
501551
ADIO_Offset blocklength = ints[1+i-j], stride = ints[top_count+1+i-j];
502552
if (blocklength > 0) {
553+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
503554
flat->indices[nonzeroth] =
504555
st_offset + stride* ADIOI_AINT_CAST_TO_OFFSET old_extent;
505556
flat->blocklens[nonzeroth] =
506557
blocklength* ADIOI_AINT_CAST_TO_OFFSET old_extent;
507558
nonzeroth++;
508-
} else {
509-
flat->count--; /* don't count/consider any zero-length blocklens */
510559
}
511560
}
512-
*curr_index = i;
561+
*curr_index = nonzeroth;
513562
}
514563
else {
515564
/* indexed type made up of noncontiguous derived types */
@@ -523,14 +572,13 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
523572
for (m=1; m<ints[1]; m++) {
524573
for (i=0, nonzeroth = j; i<num; i++) {
525574
if (flat->blocklens[j-num] > 0) {
575+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
526576
flat->indices[nonzeroth] =
527577
flat->indices[nonzeroth-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
528578
flat->blocklens[nonzeroth] =
529579
flat->blocklens[nonzeroth-num];
530580
j++;
531581
nonzeroth++;
532-
} else {
533-
flat->count --;
534582
}
535583
}
536584
}
@@ -545,26 +593,24 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
545593
avoid >2G integer arithmetic problems */
546594
ADIO_Offset stride = ints[top_count+1+i]-ints[top_count+i];
547595
if (flat->blocklens[j-num] > 0 ) {
596+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
548597
flat->indices[nonzeroth] =
549598
flat->indices[j-num] + stride* ADIOI_AINT_CAST_TO_OFFSET old_extent;
550599
flat->blocklens[nonzeroth] = flat->blocklens[j-num];
551600
j++;
552601
nonzeroth++;
553-
} else {
554-
flat->count--;
555602
}
556603
}
557604
*curr_index = j;
558605
for (m=1; m<ints[1+i]; m++) {
559606
for (k=0, nonzeroth=j; k<basic_num; k++) {
560607
if (flat->blocklens[j-basic_num] > 0) {
608+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
561609
flat->indices[nonzeroth] =
562610
flat->indices[j-basic_num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
563611
flat->blocklens[nonzeroth] = flat->blocklens[j-basic_num];
564612
j++;
565613
nonzeroth++;
566-
} else {
567-
flat->count --;
568614
}
569615
}
570616
}
@@ -610,6 +656,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
610656
/* By using ADIO_Offset we preserve +/- sign and
611657
avoid >2G integer arithmetic problems */
612658
ADIO_Offset blocklength = ints[1];
659+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(i);
613660
if (is_hindexed_block) {
614661
flat->indices[i] = st_offset + adds[i-j];
615662
} else {
@@ -636,6 +683,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
636683
* extent of a type */
637684
MPI_Type_get_extent(types[0], &lb, &old_extent);
638685
}
686+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
639687
flat->indices[j] = flat->indices[j-num] +
640688
ADIOI_AINT_CAST_TO_OFFSET old_extent;
641689
flat->blocklens[j] = flat->blocklens[j-num];
@@ -648,6 +696,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
648696
num = *curr_index - prev_index;
649697
for (i=1; i<top_count; i++) {
650698
for (m=0; m<num; m++) {
699+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
651700
if (is_hindexed_block) {
652701
flat->indices[j] = flat->indices[j-num] +
653702
adds[i] - adds[i-1];
@@ -691,14 +740,13 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
691740
/* By using ADIO_Offset we preserve +/- sign and
692741
avoid >2G integer arithmetic problems */
693742
ADIO_Offset blocklength = ints[1+i-j];
743+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
694744
flat->indices[nonzeroth] = st_offset + adds[i-j];
695745
flat->blocklens[nonzeroth] = blocklength*old_size;
696746
nonzeroth++;
697-
} else {
698-
flat->count--;
699747
}
700748
}
701-
*curr_index = i;
749+
*curr_index = nonzeroth;
702750
}
703751
else {
704752
/* indexed type made up of noncontiguous derived types */
@@ -713,13 +761,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
713761
for (m=1; m<ints[1]; m++) {
714762
for (i=0, nonzeroth=j; i<num; i++) {
715763
if (flat->blocklens[j-num] > 0) {
764+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
716765
flat->indices[nonzeroth] =
717766
flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
718767
flat->blocklens[nonzeroth] = flat->blocklens[j-num];
719768
j++;
720769
nonzeroth++;
721-
} else {
722-
flat->count--;
723770
}
724771
}
725772
}
@@ -731,19 +778,19 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
731778
prev_index = *curr_index;
732779
for (m=0, nonzeroth=j; m<basic_num; m++) {
733780
if (flat->blocklens[j-num] > 0) {
781+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
734782
flat->indices[nonzeroth] =
735783
flat->indices[j-num] + adds[i] - adds[i-1];
736784
flat->blocklens[nonzeroth] = flat->blocklens[j-num];
737785
j++;
738786
nonzeroth++;
739-
} else {
740-
flat->count--;
741787
}
742788
}
743789
*curr_index = j;
744790
for (m=1; m<ints[1+i]; m++) {
745791
for (k=0,nonzeroth=j; k<basic_num; k++) {
746792
if (flat->blocklens[j-basic_num] >0) {
793+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(nonzeroth);
747794
flat->indices[nonzeroth] =
748795
flat->indices[j-basic_num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
749796
flat->blocklens[nonzeroth] = flat->blocklens[j-basic_num];
@@ -781,6 +828,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
781828
j = *curr_index;
782829
flat->indices[j] = st_offset + adds[n];
783830
MPI_Type_size_x(types[n], &old_size);
831+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
784832
flat->blocklens[j] = blocklength * old_size;
785833
#ifdef FLATTEN_DEBUG
786834
DBG_FPRINTF(stderr,"ADIOI_Flatten:: simple adds[%#X] "MPI_AINT_FMT_HEX_SPEC", flat->indices[%#llX] %#llX, flat->blocklens[%#llX] %#llX\n",n,adds[n],j, flat->indices[j], j, flat->blocklens[j]);
@@ -798,6 +846,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
798846
MPI_Type_get_extent(types[n], &lb, &old_extent);
799847
for (m=1; m<ints[1+n]; m++) {
800848
for (i=0; i<num; i++) {
849+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
801850
flat->indices[j] =
802851
flat->indices[j-num] + ADIOI_AINT_CAST_TO_OFFSET old_extent;
803852
flat->blocklens[j] = flat->blocklens[j-num];
@@ -827,6 +876,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
827876
* bound based on the inner type, but the lower bound based on the
828877
* upper type. check both lb and ub to prevent mixing updates */
829878
if (flat->lb_idx == -1 && flat->ub_idx == -1) {
879+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
830880
flat->indices[j] = st_offset + adds[0];
831881
/* this zero-length blocklens[] element, unlike eleswhere in the
832882
* flattening code, is correct and is used to indicate a lower bound
@@ -843,7 +893,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
843893
} else {
844894
/* skipped over this chunk because something else higher-up in the
845895
* type construction set this for us already */
846-
flat->count--;
847896
st_offset -= adds[0];
848897
}
849898

@@ -859,6 +908,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
859908
else {
860909
/* current type is basic or contiguous */
861910
j = *curr_index;
911+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
862912
flat->indices[j] = st_offset;
863913
MPI_Type_size_x(types[0], &old_size);
864914
flat->blocklens[j] = old_size;
@@ -874,6 +924,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
874924
/* see note above about mixing updates for why we check lb and ub */
875925
if ((flat->lb_idx == -1 && flat->ub_idx == -1) || lb_updated) {
876926
j = *curr_index;
927+
FIX_ARRAYS_IN_CASE_OF_WRONG_COUNT(j);
877928
flat->indices[j] = st_offset + adds[0] + adds[1];
878929
/* again, zero-element ok: an upper-bound marker explicitly set by the
879930
* constructor of this resized type */
@@ -882,7 +933,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node *flat,
882933
} else {
883934
/* skipped over this chunk because something else higher-up in the
884935
* type construction set this for us already */
885-
flat->count--;
886936
(*curr_index)--;
887937
}
888938

0 commit comments

Comments
 (0)