Hello Will,
On Thu, Mar 13, 2025 at 1:25 PM Will Deacon will@kernel.org wrote:
On Thu, Mar 13, 2025 at 11:43:22AM +0000, Alessandro Carminati wrote:
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h index 28be048db3f6..044c5e24a17d 100644 --- a/arch/arm64/include/asm/bug.h +++ b/arch/arm64/include/asm/bug.h @@ -11,8 +11,14 @@
#include <asm/asm-bug.h>
+#ifdef HAVE_BUG_FUNCTION +# define __BUG_FUNC __func__ +#else +# define __BUG_FUNC NULL +#endif
#define __BUG_FLAGS(flags) \
asm volatile (__stringify(ASM_BUG_FLAGS(flags)));
asm volatile (__stringify(ASM_BUG_FLAGS(flags, %c0)) : : "i" (__BUG_FUNC));Why is 'i' the right asm constraint to use here? It seems a bit odd to use that for a pointer.
I received this code as legacy from a previous version. In my review, I considered the case when HAVE_BUG_FUNCTION is defined: Here, __BUG_FUNC is defined as __func__, which is the name of the current function as a string literal. Using the constraint "i" seems appropriate to me in this case.
However, when HAVE_BUG_FUNCTION is not defined: __BUG_FUNC is defined as NULL. Initially, I considered it literal 0, but after investigating your concern, I found:
``` $ echo -E "#include <stdio.h>\n#include <stddef.h>\nint main() {\nreturn 0;\n}" | aarch64-linux-gnu-gcc -E -dM - | grep NULL #define NULL ((void *)0) ```
I realized that NULL is actually a pointer that is not a link time symbol, and using the "i" constraint with NULL may result in undefined behavior.
Would the following alternative definition for __BUG_FUNC be more convincing?
``` #ifdef HAVE_BUG_FUNCTION #define __BUG_FUNC __func__ #else #define __BUG_FUNC (uintptr_t)0 #endif ``` Let me know your thoughts.
Will
-- --- 172