lwIP - A Lightweight TCP/IP stack - Bugs: bug #48300, Private mempools allocate foreign...
You are not allowed to post comments on this tracker with your current authentication level.
bug #48300: Private mempools allocate foreign memory
Submitter: | chrysn <chrysn> | ||
Submitted: | Thu 23 Jun 2016 03:00:05 PM UTC | ||
Category: | None | Severity: | 3 - Normal |
Item Group: | Faulty Behaviour | Status: | Fixed |
Privacy: | Public | Assigned to: | None |
Open/Closed: | Closed | Planned Release: | 2.0.0 Beta2 |
lwIP version: | git head |
Jump to the original submission
Thu 07 Jul 2016 11:40:56 AM UTC, comment #11: |
Dirk Ziegelmeier <dziegel> |
Wed 06 Jul 2016 10:57:53 AM UTC, comment #10: In assembling the patch, I found that there has been already a start of the mechanisms required in place -- the LWIP_MEM_ALIGN_BUFFER macro already did the right thing by adding (alignment-1) bytes to a size, it was just never used, so a first patch makes candidates use it.
|
chrysn <chrysn> |
Sun 03 Jul 2016 07:58:07 PM UTC, comment #9: To comment #7: Does the waste of memory really matter? We pad each pool element to MEM_ALIGNMENT - which means we waste NumberOfElements*(MEM_ALIGNMENT/2) in the average case anyway.
|
Dirk Ziegelmeier <dziegel> |
Fri 01 Jul 2016 05:34:48 AM UTC, comment #8: I'm just going through the uses of MEM_ALIGN*, and I am preparing a proposed patch (that would save memory if cc.h provides a way to align allocations, and otherwise keep allocating too much) over the weekend. |
chrysn <chrysn> |
Thu 30 Jun 2016 08:23:33 PM UTC, comment #7: I'd like to fix this properly before releasing the next beta: we don't support allocating all pools in one block any more, so we always allocate "MEM_ALIGNMENT-1" bytes too much for each pool... |
Simon Goldschmidt <goldsimon> |
Fri 24 Jun 2016 01:14:08 PM UTC, comment #6: Adding these bytes is a waste of memory. A better fix would be to enforce alignment on these buffers. We would need PORTABLE alignment macros like for packing...
|
Dirk Ziegelmeier <dziegel> |
Fri 24 Jun 2016 11:06:07 AM UTC, comment #5: The solution in 2c1bd36 works; thank you for the fast fix! |
chrysn <chrysn> |
Fri 24 Jun 2016 08:48:16 AM UTC, comment #4: #define LWIP_MEMPOOL_DECLARE(name,num,size,desc) u8_t memp_memory_ ## name ## _base \
|
Dirk Ziegelmeier <dziegel> |
Fri 24 Jun 2016 07:29:24 AM UTC, comment #3: The patch did not have the desired effect; the symptom persists, and applying it didn't even change the symbol table (`nm -S` output); given I don't have memp debugging enabled (which might probably catch the issue), MEMP_SIZE is 0, so the `(MEMP_SIZE + MEMP_ALIGN(size))` term is already rounded, so thus is `(num) * ...`.
|
chrysn <chrysn> |
Thu 23 Jun 2016 06:22:19 PM UTC, comment #2: Thank you for reporting. Please check whether the fix I just committed works for you. |
Dirk Ziegelmeier <dziegel> |
Thu 23 Jun 2016 03:31:23 PM UTC, comment #1: The issue can be worked around by adding `__attribute__((aligned(MEM_ALIGNMENT)))` after the u8_t of LWIP_MEMPOOL_DECLARE -- but that's a GCC'ism, and as I only see those in cc.h for `PACK_STRUCT_STRUCT`, I assume that the general intention is to keep such compiler-isms there; the hard thing here (as compared to `PACK_STRUCT_STRUCT`, which I assume is only used for saving memory) is that such a feature can't be generally assumed to be present on all compilers.
|
chrysn <chrysn> |
Thu 23 Jun 2016 03:00:05 PM UTC, original submission:
The memory access to pools declared using LWIP_MEMPOOL_DECLARE can be flawed when MEM_ALIGNMENT is used. This was observed on an ARM Cortex device on which MEM_ALIGNMENT was set to 8 for reasons outside the architecture. Two pools are allocated in parallel, and memset'ing what was obtained from one of the pools wiped the tab of the other.
|
chrysn <chrysn> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 13 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2016-07-07 | dziegel | Status | In Progress | Fixed | |
Open/Closed | Open | Closed | |||
2016-07-06 | chrysn | Attached File | - | Added 0001-Use-LWIP_MEM_ALIGN_BUFFER-macro.patch, #37729 | |
Attached File | - | Added 0002-Introduce-LWIP_ALIGN_SUFFIX-macro.patch, #37730 | |||
Attached File | - | Added 0003-demo-only-Be-strict-about-when-memory-should-already.patch, #37731 | |||
2016-06-30 | goldsimon | Status | Ready For Test | In Progress | |
2016-06-30 | goldsimon | Planned Release | None | 2.0.0 Beta2 | |
2016-06-27 | goldsimon | Planned Release | 2.0.0 Beta2 | ||
2016-06-24 | dziegel | Status | Confirmed | Ready For Test | |
2016-06-24 | dziegel | Status | Fixed | Confirmed | |
Open/Closed | Closed | Open | |||
2016-06-23 | dziegel | Status | None | Fixed | |
Open/Closed | Open | Closed |
Powered by Savane 3.13-02a9.
Corresponding source code
Fixed in another way as your proposal, but should work for you, too