Re: [PATCH v9] Build process uses script to add CFI directives to x86 asm

[Rich Felker <dalias@libc.org>]

On Mon, Jun 15, 2015 at 08:45:00AM +0200, Alex Dowad wrote:
> diff --git a/Makefile b/Makefile
> index 2eb7b30..b4927f4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -120,7 +120,11 @@ $(foreach s,$(wildcard src/*/$(ARCH)*/*.s),$(eval $(call mkasmdep,$(s))))
>  	$(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $(dir $<)$(shell cat $<)
>  
>  %.o: $(ARCH)/%.s
> +ifeq ($(ADD_CFI),yes)
> +	LC_ALL=C awk -f tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL_STATIC) -x assembler -c -o $@ -
> +else
>  	$(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $<
> +endif
>  
>  %.o: %.c $(GENH) $(IMPH)
>  	$(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $<
> @@ -129,7 +133,11 @@ $(foreach s,$(wildcard src/*/$(ARCH)*/*.s),$(eval $(call mkasmdep,$(s))))
>  	$(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $(dir $<)$(shell cat $<)
>  
>  %.lo: $(ARCH)/%.s
> +ifeq ($(ADD_CFI),yes)
> +	LC_ALL=C awk -f tools/add-cfi.$(ARCH).awk $< | $(CC) $(CFLAGS_ALL_SHARED) -x assembler -c -o $@ -
> +else
>  	$(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $<
> +endif
>  
>  %.lo: %.c $(GENH) $(IMPH)
>  	$(CC) $(CFLAGS_ALL_SHARED) -c -o $@ $<

I think someone mentioned this before -- this still fails to apply CFI
to the asm files obtained via the .sub/SUBARCH system (see the rules
just above the ones you changed). Of course I don't really like
duplicating complex rules, and i386 has no SUBARCHs anyway, so perhaps
we could put off changing this if someone has an idea for how to
eliminate the extra rules for SUBARCHs when we overhaul the build
system...

Alternatively we could perhaps just put the command for asm files in a
makefile variable and set it conditionally depending on ADD_CFI. Then
it could be reused in each place.

> diff --git a/configure b/configure
> index 7b29ae4..a55acd6 100755
> --- a/configure
> +++ b/configure
> @@ -327,6 +327,22 @@ tryflag CFLAGS_MEMOPS -fno-tree-loop-distribute-patterns
>  test "$debug" = yes && CFLAGS_AUTO=-g
>  
>  #
> +# Preprocess asm files to add extra debugging information if debug is
> +# enabled, our assembler supports the needed directives, and the
> +# preprocessing script has been written for our architecture.
> +#
> +printf "checking whether we should preprocess assembly to add debugging information... "
> +if fnmatch '-g*|*\ -g*' "$CFLAGS_AUTO" &&
> +   test -f "tools/add-cfi.$ARCH.awk" &&
> +   printf ".file 1 \"srcfile.s\"\n.line 1\n.cfi_startproc\n.cfi_endproc" | $CC -g -x assembler -c -o /dev/null 2>/dev/null -
> +then
> +  ADD_CFI=yes
> +else
> +  ADD_CFI=no
> +fi
> +printf "%s\n" "$ADD_CFI"
> +
> +#

I'd still like to check $CFLAGS in addition to $CFLAGS_AUTO, since
CFLAGS=-g is the usual way I ask for debug info. (I'm sorry I ever
added the --enable-debug option to configure. :)

I haven't reviewed the awk script much at all, but as long as it seems
to work in practice I'm okay with committing it. One thing I would
like to ask though -- is it pretty robust against bad things happening
if it sees new asm constructs it's not expecting? I'd just like to
avoid regressions if we add new asm later, where it becomes necessary
to delay commits to asm in order to fix bugs in the CFI generation.

Rich