master: Build failure in malloc with GCC 7
Commit Message
On 07/12/2017 02:29 PM, Andreas Schwab wrote:
> On Jul 12 2017, Florian Weimer <fweimer@redhat.com> wrote:
>
>> The attached patch adds an assert which reveals to the GCC optimizers
>> that the global_max_fast can never MAX_FAST_SIZE, so this particular
>> issue goes away. However, there is a cost in terms of code size because
>> it affects many places in malloc.
>
> Can __builtin_unreachable avoid the runtime overhead?
Interesting idea. I have never used __builtin_unreachable before, I think.
Current master:
text data bss dec hex filename
64592 2392 104 67088 10610 /root/build/malloc/malloc.o
My second patch:
text data bss dec hex filename
64424 2392 104 66920 10568 /root/build/malloc/malloc.o
Attached patch:
text data bss dec hex filename
64504 2392 104 67000 105b8 /root/build/malloc/malloc.o
Looking at the GIMPLE dumps, the code is somewhat improved over my
second patch.
Thanks,
Florian
Comments
On Wed, Jul 12, 2017 at 8:58 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/12/2017 02:29 PM, Andreas Schwab wrote:
>> On Jul 12 2017, Florian Weimer <fweimer@redhat.com> wrote:
>>
>>> The attached patch adds an assert which reveals to the GCC optimizers
>>> that the global_max_fast can never MAX_FAST_SIZE, so this particular
>>> issue goes away. However, there is a cost in terms of code size because
>>> it affects many places in malloc.
>>
>> Can __builtin_unreachable avoid the runtime overhead?
>
> Interesting idea. I have never used __builtin_unreachable before, I think.
I think this code should be defensive against the possibility of
global_max_fast somehow getting corrupted. What about
+static inline INTERNAL_SIZE_T
+get_max_fast (void)
+{
+ /* If this function ever returns a value larger than MAX_FAST_SIZE,
+ _int_malloc will make out-of-bounds array accesses. It should be
+ impossible for global_max_fast to become larger than than
+ MAX_FAST_SIZE, but as an extra precaution, limit the value here
+ as well. */
+ if (global_max_fast > MAX_FAST_SIZE)
+ return MAX_FAST_SIZE;
+ return global_max_fast;
+}
On Wed, Jul 12, 2017 at 10:27 AM, Zack Weinberg <zackw@panix.com> wrote:
> + impossible for global_max_fast to become larger than than
Or without the double 'than',
+static inline INTERNAL_SIZE_T
+get_max_fast (void)
+{
+ /* If this function ever returns a value larger than MAX_FAST_SIZE,
+ _int_malloc will make out-of-bounds array accesses. It should be
+ impossible for global_max_fast to become larger than MAX_FAST_SIZE,
+ but as an extra precaution, limit the value here as well. */
+ if (global_max_fast > MAX_FAST_SIZE)
+ return MAX_FAST_SIZE;
+ return global_max_fast;
+}
On 08/03/2017 03:31 AM, Victor Rodriguez wrote:
> On Wed, Jul 12, 2017 at 5:43 PM, DJ Delorie <dj@redhat.com> wrote:
>
>> I tested this patch and can measure no discernable performance
>> difference, so LGTM :-)
>>
>>
> Thanks for the patch , it works fine with 2.26 release
>
> Any reason why is not merged on master for 2.26 ?
I didn't get an ack from the RM.
Anyway, I pushed it now. I will also backport it to the 2.26 branch.
Thanks,
Florian
@@ -1658,6 +1658,9 @@ typedef struct malloc_chunk *mfastbinptr;
#define arena_is_corrupt(A) (((A)->flags & ARENA_CORRUPTION_BIT))
#define set_arena_corrupt(A) ((A)->flags |= ARENA_CORRUPTION_BIT)
+/* Maximum size of memory handled in fastbins. */
+static INTERNAL_SIZE_T global_max_fast;
+
/*
Set value of max_fast.
Use impossibly small value if 0.
@@ -1668,8 +1671,20 @@ typedef struct malloc_chunk *mfastbinptr;
#define set_max_fast(s) \
global_max_fast = (((s) == 0) \
? SMALLBIN_WIDTH : ((s + SIZE_SZ) & ~MALLOC_ALIGN_MASK))
-#define get_max_fast() global_max_fast
+static inline INTERNAL_SIZE_T
+get_max_fast (void)
+{
+ /* Tell the glibc optimizers that global_max_fast is never larger
+ than MAX_FAST_SIZE. This avoids out-of-bounds array accesses in
+ _int_malloc after constant propagation of the size parameter.
+ (The code never executes because malloc preserves the
+ global_max_fast invariant, but the optimizers may not recognize
+ this.) */
+ if (global_max_fast > MAX_FAST_SIZE)
+ __builtin_unreachable ();
+ return global_max_fast;
+}
/*
----------- Internal state representation and initialization -----------
@@ -1797,9 +1812,6 @@ static struct malloc_par mp_ =
#endif
};
-/* Maximum size of memory handled in fastbins. */
-static INTERNAL_SIZE_T global_max_fast;
-
/*
Initialize a malloc_state struct.