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

[Alex <alexinbeijing@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3446 bytes --]

On Mon, Jun 15, 2015 at 5:26 AM, Rich Felker <dalias@libc.org> wrote:

> On Sun, Jun 14, 2015 at 09:06:16PM +0200, Alex wrote:
> > Thanks for the reply! Comments below:
> >
> > On Sun, Jun 14, 2015 at 6:37 AM, Rich Felker <dalias@libc.org> wrote:
> >
> > > On Fri, Jun 05, 2015 at 10:39:18AM +0200, Alex Dowad wrote:
> > > > diff --git a/Makefile b/Makefile
> > > > index 2eb7b30..9b55fd8 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
> > > > -     $(CC) $(CFLAGS_ALL_STATIC) -c -o $@ $<
> > > > +ifeq ($(ADD_CFI),yes)
> > > > +     LC_ALL=C awk -f tools/add-cfi.$(ARCH).awk $< | $(CC)
> $(ASFLAGS) -x
> > > assembler -c -o $@ -
> > > > +else
> > > > +     $(CC) $(ASFLAGS) -c -o $@ $<
> > > > +endif
> > >
> > > Removing $(CFLAGS_STATIC_ALL) here is a regression. -Wa,--noexecstack
> > > is necessary to prevent the kernel from giving us an executable stack
> > > when asm files are linked. We could move it to a separate ASFLAGS, but
> > > the patch doesn't do this, and unless there's a real need to avoid
> > > passing CFLAGS, I'd rather not add more vars. (In this case, needing
> > > the new var would be a silent security regression for anyone building
> > > without re-running configure.)
> > >
> >
> > The reason for not passing CFLAGS is because clang chokes on "-g" when
> > assembling code with CFI directives. I also thought that ASFLAGS might
> be a
> > useful customization point for people who want to edit config.mak to
> create
> > a custom build. But you are the judge of that.
> >
> > Since it seems that CFLAGS is needed, would it be acceptable to bypass
> the
> > issue by saying that clang users simply won't be able to do debug builds
> of
> > musl until their compiler is fixed? The current state of LLVM's CFI
> > generation is so bad that debug builds probably won't be useful anyways.
>
> Could you elaborate on what happens? I'm not opposed to this approach
> as long as either (1) the configure test successfully determines that
> CFI gen doesn't work on clang, or (2) the 'choking' just produces bad
> CFI, but doesn't break the build.
>

The assembler errors out and doesn't produce any output. I have made the
test in ./configure more robust now, to work around this problem. Insertion
of .cfi directives will not occur when building with clang, until it is
fixed.


> > If that is a sticking point, I might put together a patch for LLVM and
> see
> > if they want it. Unfortunately, I have already discovered a bunch of
> other
> > problems with LLVM which would be nice to fix, but time for developing
> and
> > polishing patches is limited...
>
> Why is -g even being processes for asm? Are they trying to
> auto-generate CFI when it's not present? I think this really needs to
> be fixed in any case since there are plenty of .s files that _do_ have
> CFI and build systems that use -g. All this points to clang's internal
> assembler being not-widely-tested and not ready for serious use... :(
>

GAS silently disables auto-generation of debug info as soon as it sees an
explicit debug directive. Clang gets ornery, digs its heels in, and says:
"forget it, you aren't getting nothing from me if you tell me to generate
debug info but then provide your own".

Posting v9 patch now.

[-- Attachment #2: Type: text/html, Size: 4556 bytes --]