[PATCH v3 0/8] Memory tagging support

[Siddhesh Poyarekar]

On 11/23/20 9:12 PM, Richard Earnshaw via Libc-alpha wrote:
> This is the third iteration of the patch set to enable memory tagging
> in glibc's malloc code.  It mainly addresses the following issues raised
> during the previous review:
> 
>   - Clean up/add some internal API documentation
>   - Remove ROUND_UP_ALLOCATION_SIZE and use some conditionalized code
>     in checked_request2size instead.
>   - Support MALLOC_CHECK_ in conjuction with _MTAG_ENABLE.

I tried to review but the patchset doesn't build on x86_64 because 
__libc_mtag_tag_region() and __libc_mtag_address_get_tag() are macros:

arena.c: In function ‘ptmalloc_init’:
arena.c:342:22: error: ‘__libc_mtag_tag_region’ undeclared (first use in 
this function); did you mean ‘__default_tag_region’?
   342 |       __tag_region = __libc_mtag_tag_region;
       |                      ^~~~~~~~~~~~~~~~~~~~~~
       |                      __default_tag_region

I'll review 1, 2. 6 and 7 (and the overall design) anyway since I 
suspect you'll only end up changing 3 and 8 to fix this failure. 
Hopefully that will help you make forward progress.

> The first two issues are addressed in patch 4 of this series, and the
> third in patch 5.  I intend to merge patches 3, 4 and 5 into a single
> update to the malloc code before the final commit; I've kept them
> separate for now to (hopefully) simplify the review.

Please merge them in first; it is actually confusing to review because I 
have to then refer across three patches to see what's fixed in 3/8.

Besides, having the version of the patch in review being the same as the 
one that's committed helps since we can then auto-close patchwork patches.

> I spent quite a bit of time while working on these looking at whether
> the code could be refactored in order to reduce the places where
> SIZE_SZ was being added (in different multiples) to various pointers.
> I eventually concluded that this wasn't significantly improving the
> readability of the code, but one change has survived - I've replaced
> usage of 2 * SIZE_SZ with CHUNK_HDR_SZ when it is clear that this is
> referring to the header block at the start of a chunk.

I skimmed through the patches and I think this is a useful change, thanks.

Siddhesh