Hi,
Philip Withnall wrote:
> We’ve seen a compiler warning in the vasnprintf module (as copied into
> GLib) when building with GCC 14 (on msys2, although I don’t think
> that’s relevant).
>
> ```
> ../glib/gnulib/vasnprintf.c: In function 'multiply':
> ../glib/gnulib/vasnprintf.c:382:21: error: allocation of insufficient
> size '1' for type 'mp_limb_t' {aka 'unsigned int'} with size '4' [-
> Werror=alloc-size]
> 382 | dest->limbs = (mp_limb_t *) malloc (1);
> | ^
> cc1.exe: all warnings being treated as errors
> ```
>
> This is for the following code in vasnprintf.c:
> ```
> if (len1 == 0)
> {
> /* src1 or src2 is zero. */
> dest->nlimbs = 0;
> dest->limbs = (mp_limb_t *) malloc (1);
> }
> ```
>
> Given that it’s setting `nlimbs = 0`, I suspect that the allocation is
> a dummy and doesn’t actually need to contain a full `mp_limb_t`.
Correct.
> The compiler can’t know that though.
Correct. Only a runtime instrumentation (like valgrind or ASAN) can
distinguish between an allocation need of 0 words vs. 1 word.
> The fix we’ve put together for GLib is here, and makes the obvious
> switch from allocating one byte to allocating `sizeof (mp_limb_t)`:
> https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4066/diffs?commit_id=cbc58085455b699e9ccd73f30eed72510ead5813
This approach has the drawback that runtime instrumentations will
not detect if we accidentally read or write the first element of
this array.
> ... go with an
> alternative like using a pragma to ignore the -Walloc-size warning
> around that area of code.
I considered this. But -Walloc-size is sometimes a useful warning, and
4 to 7 lines of code to disable this warning is a pain.
In the end, I'm removing this 'malloc (1)' memory allocation; this should
speed up the code in some cases.
2024-05-15 Bruno Haible <br...@clisp.org>
vasnprintf: Avoid a dummy memory allocation.
* lib/vasnprintf.c (NOMEM_PTR): New macro.
(multiply, divide): Return it instead of NULL in case of memory
allocation failure.
(scale10_round_decimal_decoded): Update.
diff --git a/lib/vasnprintf.c b/lib/vasnprintf.c
index de20445894..6f06273081 100644
--- a/lib/vasnprintf.c
+++ b/lib/vasnprintf.c
@@ -408,6 +408,9 @@ is_infinite_or_zerol (long double x)
#if NEED_PRINTF_LONG_DOUBLE || NEED_PRINTF_DOUBLE
+/* An indicator for a failed memory allocation. */
+# define NOMEM_PTR ((void *) (-1))
+
/* Converting 'long double' to decimal without rare rounding bugs requires
real bignums. We use the naming conventions of GNU gmp, but vastly simpler
(and slower) algorithms. */
@@ -428,8 +431,8 @@ typedef struct
} mpn_t;
/* Compute the product of two bignums >= 0.
- Return the allocated memory in case of success, NULL in case of memory
- allocation failure. */
+ Return the allocated memory (possibly NULL) in case of success, NOMEM_PTR
+ in case of memory allocation failure. */
static void *
multiply (mpn_t src1, mpn_t src2, mpn_t *dest)
{
@@ -457,7 +460,7 @@ multiply (mpn_t src1, mpn_t src2, mpn_t *dest)
{
/* src1 or src2 is zero. */
dest->nlimbs = 0;
- dest->limbs = (mp_limb_t *) malloc (1);
+ dest->limbs = NULL;
}
else
{
@@ -469,7 +472,7 @@ multiply (mpn_t src1, mpn_t src2, mpn_t *dest)
dlen = len1 + len2;
dp = (mp_limb_t *) malloc (dlen * sizeof (mp_limb_t));
if (dp == NULL)
- return NULL;
+ return NOMEM_PTR;
for (k = len2; k > 0; )
dp[--k] = 0;
for (i = 0; i < len1; i++)
@@ -500,8 +503,8 @@ multiply (mpn_t src1, mpn_t src2, mpn_t *dest)
the remainder.
Finally, round-to-even is performed: If r > b/2 or if r = b/2 and q is odd,
q is incremented.
- Return the allocated memory in case of success, NULL in case of memory
- allocation failure. */
+ Return the allocated memory (possibly NULL) in case of success, NOMEM_PTR
+ in case of memory allocation failure. */
static void *
divide (mpn_t a, mpn_t b, mpn_t *q)
{
@@ -572,7 +575,7 @@ divide (mpn_t a, mpn_t b, mpn_t *q)
final rounding of q.) */
roomptr = (mp_limb_t *) malloc ((a_len + 2) * sizeof (mp_limb_t));
if (roomptr == NULL)
- return NULL;
+ return NOMEM_PTR;
/* Normalise a. */
while (a_len > 0 && a_ptr[a_len - 1] == 0)
@@ -708,7 +711,7 @@ divide (mpn_t a, mpn_t b, mpn_t *q)
if (tmp_roomptr == NULL)
{
free (roomptr);
- return NULL;
+ return NOMEM_PTR;
}
{
const mp_limb_t *sourceptr = b_ptr;
@@ -1306,7 +1309,7 @@ scale10_round_decimal_decoded (int e, mpn_t m, void
*memory, int n)
mpn_t denominator;
void *tmp_memory;
tmp_memory = multiply (m, pow5, &numerator);
- if (tmp_memory == NULL)
+ if (tmp_memory == NOMEM_PTR)
{
free (pow5_ptr);
free (memory);
@@ -1379,7 +1382,7 @@ scale10_round_decimal_decoded (int e, mpn_t m, void
*memory, int n)
/* Here y = round (x * 10^n) = z * 10^extra_zeroes. */
- if (z_memory == NULL)
+ if (z_memory == NOMEM_PTR)
return NULL;
digits = convert_to_decimal (z, extra_zeroes);
free (z_memory);
