From: Lucas Gabriel Vuotto <lucas@sexy.is>
Subject: Re: exec_elf.h: remove CONCAT definition
To: tech@openbsd.org
Date: Tue, 15 Apr 2025 10:16:49 +0000

After more investigation on how this ends up redefined in net/haproxy,
which was what originally prompted me to take a look at this, jca and tb
agree with this is kind of a whack-a-mole, and I don't feel too strongly
about the rename to ELFCONCAT neither (nor about CONCAT in a public
header, but it isn't used frequently enough for it to be a problem).
Please, ignore this patch.

On Tue, Apr 15, 2025 at 09:02:16AM +0000, Lucas Gabriel Vuotto wrote:
> On Tue, Apr 15, 2025 at 06:34:00AM +0000, Lucas Gabriel Vuotto wrote:
> > Hi tech,
> > 
> > /sys/sys/exec_elf.h defines a CONCAT macro as another name for __CONCAT
> > and uses it in the next 2 lines:
> > 
> >    751	#if defined(ELFSIZE)
> >    752	#define CONCAT(x,y)	__CONCAT(x,y)
> >    753	#define ELFNAME(x)	CONCAT(elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> >    754	#define ELFDEFNNAME(x)	CONCAT(ELF,CONCAT(ELFSIZE,CONCAT(_,x)))
> >    755	#endif
> > 
> > CONCAT tends to be commonly used for this same purpouse, and this header
> > can leak into public. In src tree, CONCAT is defined in:
> > 
> > /usr/src/gnu/gcc/gcc/config/sh/lib1funcs.h
> > /usr/src/gnu/llvm/lldb/tools/debugserver/source/RNBDefs.h
> > /usr/src/gnu/usr.bin/gcc/gcc/config/sh/lib1funcs.asm
> > /usr/src/gnu/usr.bin/gcc/gcc/testsuite/gcc.dg/attr-invalid.c
> > /usr/src/usr.bin/make/defines.h
> > 
> > In ports land, at least net/haproxy defines CONCAT. It's also used as
> > the name on some resources online.
> > 
> > Given that it's a common, and that its definition in exec_elf.h isn't
> > really doing anything, OK to unroll it? I built a kernel successfully;
> > idk if there is another thing I should take a care a look for. Release?
> > Ask for a bulk?
> 
> tb helpfully started a bulk for me, which died quite quickly while
> building /sys/arch/amd64/stand/boot: The definition of CONCAT is needed
> because a macro doesn't expand itself in its definition. Undefining
> CONCAT isn't an option neither, as the expansion happens way later. I
> still think that CONCAT is a too generic name for a public header, so I
> propose renaming it to ELFCONCAT.
> 
> 
> diff refs/heads/master acf5035294bd686b67df0f4fa089e9fa32827a87
> commit - fe0564919a5ad48e136b44be6d5be4142f626e37
> commit + acf5035294bd686b67df0f4fa089e9fa32827a87
> blob - 4781824b4fa893f68921f4e36226bc72cca6cb06
> blob + e67f7e16f697b4b64c2d6ec8ad18cc8a9c2f8496
> --- sys/sys/exec_elf.h
> +++ sys/sys/exec_elf.h
> @@ -749,9 +749,9 @@ struct elf_args {
>  #endif
>  
>  #if defined(ELFSIZE)
> -#define CONCAT(x,y)	__CONCAT(x,y)
> -#define ELFNAME(x)	CONCAT(elf,CONCAT(ELFSIZE,CONCAT(_,x)))
> -#define ELFDEFNNAME(x)	CONCAT(ELF,CONCAT(ELFSIZE,CONCAT(_,x)))
> +#define ELFCONCAT(a,b)	__CONCAT(a,b)
> +#define ELFNAME(x)	ELFCONCAT(elf,ELFCONCAT(ELFSIZE,ELFCONCAT(_,x)))
> +#define ELFDEFNNAME(x)	ELFCONCAT(ELF,ELFCONCAT(ELFSIZE,ELFCONCAT(_,x)))
>  #endif
>  
>  #if defined(ELFSIZE) && (ELFSIZE == 32)
>