diff mbox series

[06/16] malloc: Ensure the generic mtag hooks are not used

Message ID 0c6c9c0a9ba50b7caf05366c2a42ecd79b75c455.1614874816.git.szabolcs.nagy@arm.com
State New
Headers show
Series memory tagging improvements | expand

Commit Message

Szabolcs Nagy March 4, 2021, 4:31 p.m. UTC
Use inline functions instead of macros, because macros can cause unused
variable warnings and type conversion issues.  We assume these functions
may appear in the code but only in dead code paths (hidden by a runtime
check), so it's important that they can compile with correct types, but
if they are actually used that should be an error.

Currently the hooks are only used when USE_MTAG is true which only
happens on aarch64 and then the aarch64 specific code is used not this
generic header.  However followup refactoring will allow the hooks to
be used with !USE_MTAG.

Note: the const qualifier in the comment was wrong: changing tags is a
write operation.
---
 sysdeps/generic/libc-mtag.h | 41 ++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 10 deletions(-)

Comments

Szabolcs Nagy March 5, 2021, 12:44 p.m. UTC | #1
The 03/04/2021 20:05, DJ Delorie wrote:
> Szabolcs Nagy <szabolcs.nagy@arm.com> writes:
> > +/* Memory tagging target hooks are only called when memory tagging is
> > +   enabled at runtime.  The generic definitions here must not be used.  */
> > +void __libc_mtag_link_error (void);
> 
> This may make it impossible to compile individual glibc modules with
> -O0, say, for debugging purposes.  I do this quite often with malloc.c
> and do not want to lose that ability.
> 
> Do we trust the compiler's default optimizations (at -O0) to do code
> elimination based on constant conditions?

yes, this breaks -O0, i haven't thought of that.

i thought it's better to guard everything with
one flag in the code than ifdefs (that can be
runtime flag with tagging enabled or constant 0
when disabled) since ifdefed code is not checked
by the compiler.

the link error is to ensure that without tagging
the tagging hooks are not called accidentally.
i can replace that with abort() and then -O0
works but then you only get runtime failure, not
a link time one (which is less useful since it
may be in a very rarely used code path).

alternatively i can remove this safety net or
just go back to ifdefs.
diff mbox series

Patch

diff --git a/sysdeps/generic/libc-mtag.h b/sysdeps/generic/libc-mtag.h
index 1a866cdc0c..e8fc236b6c 100644
--- a/sysdeps/generic/libc-mtag.h
+++ b/sysdeps/generic/libc-mtag.h
@@ -31,22 +31,43 @@ 
 /* Extra flags to pass to mmap() to request a tagged region of memory.  */
 #define __MTAG_MMAP_FLAGS 0
 
+/* Memory tagging target hooks are only called when memory tagging is
+   enabled at runtime.  The generic definitions here must not be used.  */
+void __libc_mtag_link_error (void);
+
 /* Set the tags for a region of memory, which must have size and alignment
-   that are multiples of __MTAG_GRANULE_SIZE.  Size cannot be zero.
-   void *__libc_mtag_tag_region (const void *, size_t)  */
-#define __libc_mtag_tag_region(p, s) (p)
+   that are multiples of __MTAG_GRANULE_SIZE.  Size cannot be zero.  */
+static inline void *
+__libc_mtag_tag_region (void *p, size_t n)
+{
+  __libc_mtag_link_error ();
+  return p;
+}
 
 /* Optimized equivalent to __libc_mtag_tag_region followed by memset.  */
-#define __libc_mtag_memset_with_tag memset
+static inline void *
+__libc_mtag_memset_with_tag (void *p, int c, size_t n)
+{
+  __libc_mtag_link_error ();
+  return memset (p, c, n);
+}
 
 /* Convert address P to a pointer that is tagged correctly for that
-   location.
-   void *__libc_mtag_address_get_tag (void*)  */
-#define __libc_mtag_address_get_tag(p) (p)
+   location.  */
+static inline void *
+__libc_mtag_address_get_tag (void *p)
+{
+  __libc_mtag_link_error ();
+  return p;
+}
 
 /* Assign a new (random) tag to a pointer P (does not adjust the tag on
-   the memory addressed).
-   void *__libc_mtag_new_tag (void*)  */
-#define __libc_mtag_new_tag(p) (p)
+   the memory addressed).  */
+static inline void *
+__libc_mtag_new_tag (void *p)
+{
+  __libc_mtag_link_error ();
+  return p;
+}
 
 #endif /* _GENERIC_LIBC_MTAG_H */