[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 Peter Damianov changed: What|Removed |Added CC||peter0x44 at disroot dot org --- Comment #14 from Peter Damianov --- ../../gcc/gcc/../libgcc/libgcov-util.c: In function 'void tag_counters(unsigned int, int)': ../../gcc/gcc/../libgcc/libgcov-util.c:214:59: warning: 'void* calloc(size_t, size_t)' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args] 214 | k_ctrs[tag_ix].values = values = (gcov_type *) xcalloc (sizeof (gcov_type), | ^~ ../../gcc/gcc/../libgcc/libgcov-util.c:214:59: note: earlier argument should specify number of elements, later size of each element ../../gcc/gcc/../libgcc/libgcov-util.c: In function 'void topn_to_memory_representation(gcov_ctr_info*)': ../../gcc/gcc/../libgcc/libgcov-util.c:529:43: warning: 'void* calloc(size_t, size_t)' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Wcalloc-transposed-args] 529 | = (struct gcov_kvp *)xcalloc (sizeof (struct gcov_kvp), n); | ^~~~ ../../gcc/gcc/../libgcc/libgcov-util.c:529:43: note: earlier argument should specify number of elements, later size of each element I found a two more instances of this in libgcov-util.c Personally I don't agree with this warning at all, it's FAR too spammy for what it is, considering it never actually catches a real defect, but rather a minor style choice that (IMO) does not affect readability in any case. Whether I see sizeof in the first or second argument doesn't affect my understanding of the code. In a static analyzer, fine, but I think in GCC, no. But I guess the same solution should be taken for this one, too, if it's an issue in libgfortran also.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 Tobias Burnus changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED CC||burnus at gcc dot gnu.org --- Comment #13 from Tobias Burnus --- FIXED by committing the obvious fix as obvious (comment 12). (Same as the obvious change as proposed by Steven in comment 8.) As the warning is new (since r14-5059-gd880e093d92084) and there is no real compelling reason to change it, I don't think it makes sense to queue it for backporting. Thanks for the report - and to all involved for the insight into the fine print of the C standards.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #12 from CVS Commits --- The master branch has been updated by Tobias Burnus : https://gcc.gnu.org/g:17df6ddcf11aef6d200305d35641a7deb2f430e1 commit r14-5148-g17df6ddcf11aef6d200305d35641a7deb2f430e1 Author: Tobias Burnus Date: Mon Nov 6 11:34:31 2023 +0100 libgfortran: Fix calloc call by swapping arg order [PR112364] The prototype of calloc is void *calloc(size_t nmemb, size_t size); denoting "an array of nmemb objects, each of whose size is size." (C23) In order to follow the meaning of the argument names and to silence a -Walloc-size warning, this commit swaps the order of the two args to read now: calloc (1, sizeof (transfer_queue)); libgfortran/ChangeLog: PR libfortran/112364 * io/async.c (enqueue_transfer, enqueue_done_id, enqueue_done, enqueue_close): Swap 1st and 2nd arg in calloc call.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #11 from Martin Uecker --- This is one possible way to read it. But as written, I think one can easily understand it the other way, because calloc never mentions requested total size but only about space for an array of objects of size 'size'. And then also 7.24.3 continues with "It may then be used to access such an object or an array of such objects.." which would also imply that the "size" in the sentence before refers to the size of an individual object. Anyway, I think it does not really matter for us... IMHO it is still useful to make the code have the logical order as documented for calloc. And if it turns out that the warning annoys too many people in this way, we can still tweak it before the release.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 kargl at gcc dot gnu.org changed: What|Removed |Added Priority|P3 |P4
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #10 from joseph at codesourcery dot com --- The wording refers to "the size requested", which I consider to be the product of two arguments in the case of calloc - not a particular argument to calloc.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #9 from Martin Uecker --- (In reply to jos...@codesourcery.com from comment #7) > I believe "size requested" refers to the product nmemb * size in the case > of calloc, so getting the arguments the "wrong" way round does not affect > the required alignment. The point of the change was to override DR#075 > and stop requiring e.g. 1-byte allocations to be suitably aligned for > larger types, not to make alignment for calloc depend on more than the > product of the two arguments. This may have been the intention but the wording now refers explicitly to the size. To me this also makes sense as calloc talks about allocating an array of objects of size 'size'. The required alignment of the array would not depend on the number of elements but only their size.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364
kargl at gcc dot gnu.org changed:
What|Removed |Added
CC||kargl at gcc dot gnu.org
--- Comment #8 from kargl at gcc dot gnu.org ---
7 comments?
diff --git a/libgfortran/io/async.c b/libgfortran/io/async.c
index 57097438e89..8fa1f0d4ce0 100644
--- a/libgfortran/io/async.c
+++ b/libgfortran/io/async.c
@@ -262,7 +262,7 @@ init_async_unit (gfc_unit *u)
void
enqueue_transfer (async_unit *au, transfer_args *arg, enum aio_do type)
{
- transfer_queue *tq = calloc (sizeof (transfer_queue), 1);
+ transfer_queue *tq = calloc (1, sizeof (transfer_queue));
tq->arg = *arg;
tq->type = type;
tq->has_id = 0;
@@ -284,7 +284,7 @@ int
enqueue_done_id (async_unit *au, enum aio_do type)
{
int ret;
- transfer_queue *tq = calloc (sizeof (transfer_queue), 1);
+ transfer_queue *tq = calloc (1, sizeof (transfer_queue));
tq->type = type;
tq->has_id = 1;
@@ -308,7 +308,7 @@ enqueue_done_id (async_unit *au, enum aio_do type)
void
enqueue_done (async_unit *au, enum aio_do type)
{
- transfer_queue *tq = calloc (sizeof (transfer_queue), 1);
+ transfer_queue *tq = calloc (1, sizeof (transfer_queue));
tq->type = type;
tq->has_id = 0;
LOCK (>lock);
@@ -328,7 +328,7 @@ enqueue_done (async_unit *au, enum aio_do type)
void
enqueue_close (async_unit *au)
{
- transfer_queue *tq = calloc (sizeof (transfer_queue), 1);
+ transfer_queue *tq = calloc (1, sizeof (transfer_queue));
tq->type = AIO_CLOSE;
LOCK (>lock);
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #7 from joseph at codesourcery dot com --- I believe "size requested" refers to the product nmemb * size in the case of calloc, so getting the arguments the "wrong" way round does not affect the required alignment. The point of the change was to override DR#075 and stop requiring e.g. 1-byte allocations to be suitably aligned for larger types, not to make alignment for calloc depend on more than the product of the two arguments.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #6 from Martin Uecker --- For reference, this is were the revised wording in C comes. This is intentional. https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2293.htm
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #5 from Xi Ruoyao --- (In reply to Martin Uecker from comment #4) > Interesting. But independently of alignment, the description of calloc makes > it clear that it allocates an array of nmemb objects of size x. So in any > case I think the code should be changed accordingly, which seems simple > enough. Yes, it definitely needs a change.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #4 from Martin Uecker --- Interesting. But independently of alignment, the description of calloc makes it clear that it allocates an array of nmemb objects of size x. So in any case I think the code should be changed accordingly, which seems simple enough.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #3 from Xi Ruoyao --- (In reply to Martin Uecker from comment #2) > I don't think this is correct. The requirement is "The pointer returned if > the allocation succeeds is suitably aligned so that it may be assigned to a > pointer to any type of object with a fundamental alignment requirement and > size less than or equal to the size requested." > > So it only has to take into account fundamental alignments for objects below > the given size and the fundamental alignment requirement differs for > different objects. Note that there is no "fundamental alignment". > max_align_t would have the "greatest fundamental alignment", but the wording > for allocation functions does not refer to the greatest fundamental > alignment. Hmm, the "and size less than or equal to the size requested" clause does not exist in N1570 (C11), but it does exist in N3054 (C2x). It looks like a subtly backward-incompatible change in the standard...
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 --- Comment #2 from Martin Uecker --- I don't think this is correct. The requirement is "The pointer returned if the allocation succeeds is suitably aligned so that it may be assigned to a pointer to any type of object with a fundamental alignment requirement and size less than or equal to the size requested." So it only has to take into account fundamental alignments for objects below the given size and the fundamental alignment requirement differs for different objects. Note that there is no "fundamental alignment". max_align_t would have the "greatest fundamental alignment", but the wording for allocation functions does not refer to the greatest fundamental alignment.
[Bug libfortran/112364] calloc used incorrectly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112364 Xi Ruoyao changed: What|Removed |Added CC||xry111 at gcc dot gnu.org Status|UNCONFIRMED |NEW Keywords||easyhack Ever confirmed|0 |1 Last reconfirmed||2023-11-03 --- Comment #1 from Xi Ruoyao --- (In reply to Martin Uecker from comment #0) > Note that for the allocated size the order of arguments does not matter, but > - at least according to my understanding - the alignment requirements for > the returned memory may depend on the object size being the second argument. No, per the standard we can assign the result of calloc to T* iff T has a fundamental alignment requirement, i. e. _Alignof (max_align_t) >= _Alignof (T). It's not related to the specified size in calloc call. But anyway in this case the order of arguments is indeed wrong and it should be fixed.
