[llvm-commits] new linux/llvm patches

[PaX Team]

On 12 May 2012 at 13:48, Benjamin Kramer wrote:

Hello Ben and thanks for the quick commits, much appreciated!

> It looks like there are a lot of undocumented hacks in there. It would
> be nice to have some descriptions of the behavior where gcc and clang
> diverge so we can rule out missing features/bugs in clang. Since the
> goal is to eventually get this into the vanilla kernel we better have
> a good explanation for every single character changed ;) 

here's some past discussion of the situation:

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2010-October/011742.html
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-April/014424.html
https://events.linuxfoundation.org/slides/2011/lfcs/lfcs2011_llvm_lelbach.pdf

many issues have been resolved since, here's a quick summary of some of the
remaining ones (see also PR4068):

- intentional lack of support for nested functions in clang/llvm (it also makes no
  sense for kernel code because kernel stacks are typically non-executable therefore
  if a trampoline was emitted there, the kernel would just oops).

- missing support for -fcall-saved-reg (and similar) in clang/llvm (this is also
  a problem even for gcc when one does an LTO build as this information is not
  saved in the metadata). the workaround here is on the linux side where i simply
  fall back on the normal code that doesn't rely on special calling conventions.

- missing support for global register variables in clang/llvm, the workaround is
  fortunately simple enough in the linux case.

- intentionally missing support for variable length arrays in structures in clang/llvm.

- the stricter checks in integrated-as found some incorrect linux assembly code:
  - non-zero padding bytes used for a variable in .bss,
  - duplicate labels,
  - unsupported syntax for some insns (inl (%dx) vs. inl %dx),
  - using insns not available for the requested architecture (-march=i386/i586 mixup)
  - missing insn suffixes (where GNU as has a default but llvm doesn't)

- due to different inlining strategies from gcc, more functions need to be prevented
  from getting inlined lest the frame size get too big (which is a problem for the
  kernel due to the limited kernel stack space).

- -fcatch-undefined-behavior does actually catch some incorrect code at runtime
   in linux (e.g., bitops.h).

- clang doesn't like this construct:

crypto/shash.c:70:56: error: 'aligned' attribute ignored when parsing type
        return len + (mask & ~(__alignof__(u8 __attribute__ ((aligned))) - 1));
                                                              ^~~~~~~
- everything else i forget now ;)

> >   - elf-emit-R_X86_64_32S.patch: this is a fix for a link-time problem (wrong
> >     relocation type gets emitted for some insns on amd64), i'm sure there's a
> >     better way to identify all the affected insns but for now i just used a 
> >     simple grep you can find in the patch to produce the full list.
> 
> Ugly, this information should be tablegen'd somehow.

yes but the current code is still incorrect, so something should be done for now...

> >   - handle-empty-archive.patch: this is a small fix for LTO to be able to accept
> >     empty .a archives.
> 
> Looks ok, it could probably pushed down into the archive logic. I'll see if I can whip up a test case and commit this.

i wrote this code 2 years ago while playing with LTO'ing linux (which produces some
empty archives along the way) but if memory serves, the problem is that the initial
read of 64 bytes will already fail at a very high level (where i patch now) and no
lower level code gets to look at the empty .a archive header.

> >   - integrated-as-single-quoted-string.patch: this makes integrated-as accept
> >     strings enclosed in single quotes (something the GNU as accepts as well).
> 
> I'm pretty sure this is incorrect. I did a simple test with my local
> gas and it didn't accept any single quoted strings. Maybe there is a
> special flag or it accepts them only in some contexts? 

hmm, i swear i ran into a problem with this but now i can't reproduce it with linux
or simple examples, so i withdraw this one.

> >     integrated-as-octa.patch
> 
> Not entirely sure about this one, I'll poke at it later.

this is only used in linux in one particular way:

  .octa 0xsome128bitconstant

accepting arbitrary expressions here requires a whole lot more infrastructure changes
i'm afraid...