Port of libgit2 to AIX encountering test errors dur to malloc/calloc/realloc of zero length #5714

frmichael · 2020-11-26T16:08:00Z

frmichael
Nov 26, 2020

A second group of test errors on the AIX port concerns ''out of memory'' diagnostics for
network::remote::local::push_delete
network::remote::push::delete_notification
stash::apply::uses_reflog_like_indices_2

As seen in the stack traces, functions stdalloc__reallocarray() and stdalloc__calloc() are being called with nelem equals zero and the resulting malloc(0), calloc(0, n), etc on AIX return a null pointer.

This behaviour is allowed by the POSIX/UNIX standard.

A work around patch (test if zero, set to 1) for the first two test errors in network::remote is :

--- ./src/pack-objects.c.ORIG   2020-06-04 10:46:58 +0200
+++ ./src/pack-objects.c        2020-11-17 11:48:27 +0100
@@ -533,7 +533,7 @@
        size_t i, wo_end, last_untagged;
        git_pobject **wo;
-       if ((wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL)
+      if ((wo = git__mallocarray((pb->nr_objects?pb->nr_objects:1), sizeof(*wo))) == NULL)
                return NULL;
        for (i = 0; i < pb->nr_objects; i++) {

For test error(s) in stash::apply the work around involves several places where alloc(zero) is encountered.

The patch for stash::apply::uses_reflog_like_indices_2 is threefold :

--- ./src/merge.c.ORIG  2020-06-04 10:46:58 +0200
+++ ./src/merge.c       2020-11-17 13:32:51 +0100
@@ -1522,11 +1522,14 @@
        if ((opts->flags & GIT_MERGE_FIND_RENAMES) == 0)
                return 0;
-       similarity_ours = git__calloc(diff_list->conflicts.length,
+       /* Check if nelem/length is zero to avoid calloc(0) */
+       int conflicts_length = (diff_list->conflicts.length?diff_list->conflicts.length:1);
+
+       similarity_ours = git__calloc(conflicts_length,
                sizeof(struct merge_diff_similarity));
        GIT_ERROR_CHECK_ALLOC(similarity_ours);
-       similarity_theirs = git__calloc(diff_list->conflicts.length,
+       similarity_theirs = git__calloc(conflicts_length,
                sizeof(struct merge_diff_similarity));
        GIT_ERROR_CHECK_ALLOC(similarity_theirs);
@@ -1538,7 +1541,7 @@
        if (opts->rename_threshold < 100 && diff_list->conflicts.length <= opts->target_limit) {
                GIT_ERROR_CHECK_ALLOC_MULTIPLY(&cache_size, diff_list->conflicts.length, 3);
-               cache = git__calloc(cache_size, sizeof(void *));
+               cache = git__calloc((cache_size?cache_size:1), sizeof(void *));
                GIT_ERROR_CHECK_ALLOC(cache);
                merge_diff_list_count_candidates(diff_list, &src_count, &tgt_count);

Answered by ethomson

Aug 30, 2021

This should be fixed by #5951

View full answer

neithernut · 2020-11-27T14:06:35Z

neithernut
Nov 27, 2020

Wouldn't it make more sense to handle this inside git__mallocarray and git__calloc? Currently those are macros (defined in src/alloc.h) which invoke allocator callbacks from a global object. One could also be tempted to put the responsibility of handing platform peculiarities or "quirks" on the allocator and add an AIX-specific workaround to the default allocator functions, but that may lead to unpleasant surprises once a user comes along with a custom allocator.

Handling this inside the allocator macros (in src/alloc.h) would also make it way easier (in the long term) to enable such workarounds only for AIX (or platform xyz) if desired.

1 reply

frmichael Nov 27, 2020
Author

Thank you for your response.

If the libgit2 Community prefers that AIX handles this in the macros, we will work on corresponding patches.

ethomson · 2020-11-27T14:56:14Z

ethomson
Nov 27, 2020
Maintainer

Handling this inside the allocator macros (in src/alloc.h) would also make it way easier (in the long term) to enable such workarounds only for AIX (or platform xyz) if desired.

I would argue that these aren't workarounds, libgit2 is simply behaving wrong. Now since we have pluggable allocators now, we could define our interface with those allocators as requiring them to support 0 size allocations and return something. But I think that it is probably easier to define it like the Unix standards and allow 0 size allocations to (correctly) return NULL.

However, instead of trying to allocate non-zero, I think that my preference would be to simply not call the allocator.

Personally, I think that this is much more readable:

    git_pobject **wo = NULL;

    if (pb->nr_objects && (wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL)
        return NULL;

    /* do some work, making sure that we guard based on pb->nr_objects */

    git__free(wo);

0 replies

NattyNarwhal · 2021-07-03T18:40:46Z

NattyNarwhal
Jul 3, 2021

This is late, but -D_LINUX_SOURCE_COMPAT would provide a workaround. In the AIX libc, those will provide a malloc that behaves like glibc's.

1 reply

Helflym Jul 19, 2021

Indeed, -D_LINUX_SOURCE_COMPAT will provide a Linux-like malloc. But this will also change several others functions which is why we would rather avoid it.

ethomson · 2021-08-30T01:08:57Z

ethomson
Aug 30, 2021
Maintainer

This should be fixed by #5951

0 replies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Port of libgit2 to AIX encountering test errors dur to malloc/calloc/realloc of zero length #5714

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Replies: 4 comments 2 replies

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

Port of libgit2 to AIX encountering test errors dur to malloc/calloc/realloc of zero length #5714

Uh oh!

Uh oh!

frmichael Nov 26, 2020

Replies: 4 comments · 2 replies

Uh oh!

neithernut Nov 27, 2020

Uh oh!

frmichael Nov 27, 2020 Author

Uh oh!

Uh oh!

ethomson Nov 27, 2020 Maintainer

Uh oh!

NattyNarwhal Jul 3, 2021

Uh oh!

Helflym Jul 19, 2021

Uh oh!

ethomson Aug 30, 2021 Maintainer

frmichael
Nov 26, 2020

Replies: 4 comments 2 replies

neithernut
Nov 27, 2020

frmichael Nov 27, 2020
Author

ethomson
Nov 27, 2020
Maintainer

NattyNarwhal
Jul 3, 2021

ethomson
Aug 30, 2021
Maintainer