[cfe-dev] linux/i386 and mregparm

[Eli Friedman]

On Mon, Feb 28, 2011 at 12:51 PM,  <pageexec at freemail.hu> wrote:
> Hi folks,
>
> continuing the effort of getting clang to compile the linux kernel, i recently
> got it to work on i386 as well. in the following i'll describe some issues i
> ran into.

Have you seen http://llvm.org/bugs/show_bug.cgi?id=4068 ?  Please file
bugs related to the kernel as "blocking" that bug.

> 1. patching linux
>
> the first attached patch is against linux 2.6.36.4 (well, it's against the
> PaX patch but should mostly apply to vanilla as well). it should also work
> on amd64 but i didn't try it this time. compared to a few months ago, there
> are some changes needed on both sides again.
>
> on the linux side, clang (albeit inadvertantly) caught a bug where a function
> declaration had a different section attribute than its definition and apparently
> clang takes the former while gcc takes the latter into account in the end (and
> neither warned about the fact ;). other changes in linux were needed due to clang
> bugs and features that i'll elaborate on in the next sections.

Please file a bug for the section attribute thing.

> 2. patching clang
>
> the second attached patch is against r126479 and is needed to fix a bug that
> was introduced, or more precisely, unearthed by the recent -mregparm support
> patch. in particular, before -mregparm was supported, the only way to change
> a function's parameter passing convention was to use a function attribute, so
> in lieu of such an attribute the default regparm=0 was used.
>
> consequently, specifying an explicit regparm=0 attribute had no real effect
> and clang had no special treatment for this case. now with -mregparm the default
> regparm value can be changed for a given compilation unit therefore the old
> assumption that the default regparm value is 0 no longer holds. this becomes
> a problem when one uses a non-0 mregparm and wants to override it to 0 for some
> functions.
>
> just such a case arises with linux (due to another clang/llvm bug/feature, see
> below) so this needed fixing to get a proper kernel. i hope i got the implementation
> right, and would appreciate help with writing a test case as i'm not familiar
> with the test system.

Please put this into a separate email; it's likely to get lost in
here.  (Also, I remember seeing a reference to a similar patch, but
I'm not sure where.)

> 3. builtin functions and regparm interaction
>
> the above mentiond bug/feature is documented in bug #3997 already, basically
> the issue is that when llvm emits builtin functions calls, it has to use some
> calling convention and arguably the desired regparm value may not be available
> or even well defined at that stage. to this i'd like to add two observations:
>
>  - with mregparm support it'd be possible to pass this value down to the llvm
>   layer and take it into account when generating said functions calls,
>
>  - gcc does this already albeit it gives a warning when in hosted mode, but not
>   in freestanding mode
>
> since for now builtins are always emitted with regparm=0, normal callers must
> follow the same so in linux these functions need an explicit attribute to override
> the mregparm=3 as used for everything else in the kernel. this is manageable but
> was somewhat annoying to debug one by one ;) and also i don't think it'd be accepted
> upstream.

Bug 3997 covers part of it, but part of it looks like a separate
issue; please file a bug on the issue with strcpy etc.

> 4. unimplemented gcc command line switches
>
> there're some switches that are used by the linux Makefiles but not yet implemented
> in clang. beyond the noise (unless surpressed with -Qunused-arguments) there's actually
> a problem caused by one of them: -fno-optimize-sibling-calls. it appears that this
> optimization cannot be controlled in llvm and as a result some assumptions that the
> kernel makes about the call chain depth at certain points will be invalid (some
> tracing/logging code wants to look up the callers 2-3 levels up and due to this
> optimization it can result in the code dereferencing userland frame pointer values).
> for now i worked this around at the few places i ran into so far, but this should be
> fixed in llvm and clang i think.

Please file a bug for fno-optimize-sibling-calls.  Please file bugs
for any other useful flags that are unimplemented.  Note that clang
will sometimes complain that supported arguments are irrelevant to the
requested operation.

> 5. -Wformat false positives
>
> there's some recent change here which makes clang complain about a lot of format
> strings that are valid in linux (being a freestanding environment and sporting its
> own format string parser, not to mention extensions). i don't know what the right
> solution here would be though, but this causes lots of noise during compilation
> (and turning the warning off may miss real problems).

Your description doesn't make it clear why clang is warning but gcc isn't.

> 6. weak functions and optimization
>
> it appears to be yet another bug in that when a weak function with an empty body
> is encountered in a compilation unit, the optimizer assumes that that's there is
> to this function and omits passing arguments to calls to the weak function (and
> presumably non-empty bodies would trigger other kind of optimizations not necessarily
> valid for the overrides).
>
> obviously this is incorrect since the whole point of a weak function is that it
> can be overridden in another compilation unit and therefore no assumptions can be
> made about it in the optimizer. this particular problem arises in linux in a few
> places as weak functions are sometimes used to implement arch specific overrides.

Fixed in r126720.

> 7. bounds checking false positives
>
> this came up in the signal handling code, in particular there's a _NSIG_WORDS define
> that's used like this:
>
>   switch (_NSIG_WORDS) {
>   case 4: /*...*/
>   case 2: /*...*/
>   case 1: /*...*/
>   }
>
> and the different cases index into an array. now the problem is that _NSIG_WORDS
> is 2 for i386 but clang still evaluates case 4 and warns about the out-of-bounds
> array accesses in there. a similar false positive arises in expressions like this:
>
>  sizeof(long) == 8 ? /*...*/ : /*...*/ ;
>
> where the code meant for the 64 bit archs gets evaluated even on 32 bit archs
> and usually gives some warning, depending on the exact statements used. it'd be
> nice to fix this somehow.

Please file a bug.  Although, there have been very recent changes
here, so please update to trunk first.

> 8. rip relative addressing in mcmodel=kernel
>
> this is amd64 related but i thought i'd mention it here. the third attached patch
> allows llvm to generate rip relative accesses for kernel mode code as well (this
> is what gcc does too), and this in turn reduces the size of a relocatable kernel.
>
> this may be a linux specific feature, basically as its name says, this allows the
> kernel image to be loaded at a suitably aligned but otherwise arbitrary address in
> memory where the kernel will relocate itself (some post-link processing collects
> and creates a special section with relocation info, its size can be reduced with
> rip relative addressing).
>
> about the the commented out chunk in X86::isOffsetSuitableForCodeModel, i'm not
> sure if such checking makes sense for kernel mode, so as a quick hack i just got
> rid of it, but i don't know what the right solution there would be.
>
> also i was lazy to separate it out, the llvm Makefile patch simply makes the svn
> update process more verbose so that one can actually see which module is being
> updated, feel free to ignore it but i found it useful for myself ;).

I don't think mcmodel=kernel has gotten much use; that said, please
send the patch separately to the llvm-commits list.

> 9. integrated-as support and linux
>
> last but not least, it'd be nice one day to allow the use of integrated-as with
> linux as well, but that requires implementing support for a few directives, i
> recall pushsection/popsection at least but there was also some issue with the
> assembler not detecting the proper size for bit operations (even though it could
> have deduced them from the arguments like it does for some insns already).

There are a couple of issues attached to
http://llvm.org/bugs/show_bug.cgi?id=4068 related to assembly; if you
have others, please file.  Note that there are some issues with gas
assuming the size of an operation when it really shouldn't, and we
don't want to mimic those bugs in LLVM.

-Eli