Bug 1211853 - gcc-13 fails to compile kernel's arch/x86/purgatory/sha256.o with "error: invalid 'asm': operand is not a condition code, invalid operand code 'c'"
Summary: gcc-13 fails to compile kernel's arch/x86/purgatory/sha256.o with "error: inv...
Status: RESOLVED FIXED
Alias: None
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Kernel (show other bugs)
Version: Current
Hardware: Other Other
: P5 - None : Normal (vote)
Target Milestone: ---
Assignee: Jiri Slaby
QA Contact: E-mail List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-31 09:35 UTC by Jiri Slaby
Modified: 2024-04-19 11:33 UTC (History)
1 user (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
preprocessed source (1.40 MB, text/plain)
2023-05-31 09:35 UTC, Jiri Slaby
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jiri Slaby 2023-05-31 09:35:22 UTC
Created attachment 867309 [details]
preprocessed source

The current -next kernel fails to build with gcc-13:
>   CC      arch/x86/purgatory/sha256.o
> In file included from <command-line>:
> /dev/shm/jslaby/linux/include/crypto/sha256_base.h: In function ‘lib_sha256_base_do_update.constprop.isra’:
> /dev/shm/jslaby/linux/include/linux/compiler_types.h:332:20: error: invalid 'asm': operand is not a condition code, invalid operand code 'c'
>   332 | #define asm_inline asm __inline
>       |                    ^~~
> /dev/shm/jslaby/linux/arch/x86/include/asm/bug.h:28:9: note: in expansion of macro ‘asm_inline’
>    28 |         asm_inline volatile("1:\t" ins "\n"                             \
>       |         ^~~~~~~~~~
> /dev/shm/jslaby/linux/arch/x86/include/asm/bug.h:83:9: note: in expansion of macro ‘_BUG_FLAGS’
>    83 |         _BUG_FLAGS(ASM_UD2, __flags, ASM_REACHABLE);            \
>       |         ^~~~~~~~~~
> /dev/shm/jslaby/linux/include/asm-generic/bug.h:107:17: note: in expansion of macro ‘__WARN_FLAGS’
>   107 |                 __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
>       |                 ^~~~~~~~~~~~
> /dev/shm/jslaby/linux/include/asm-generic/bug.h:134:17: note: in expansion of macro ‘__WARN_printf’
>   134 |                 __WARN_printf(TAINT_WARN, format);                      \
>       |                 ^~~~~~~~~~~~~
> /dev/shm/jslaby/linux/include/linux/once_lite.h:31:25: note: in expansion of macro ‘WARN’
>    31 |                         func(__VA_ARGS__);                              \
>       |                         ^~~~
> /dev/shm/jslaby/linux/include/asm-generic/bug.h:152:9: note: in expansion of macro ‘DO_ONCE_LITE_IF’
>   152 |         DO_ONCE_LITE_IF(condition, WARN, 1, format)
>       |         ^~~~~~~~~~~~~~~
> /dev/shm/jslaby/linux/include/linux/fortify-string.h:641:9: note: in expansion of macro ‘WARN_ONCE’
>   641 |         WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size,          \
>       |         ^~~~~~~~~
> /dev/shm/jslaby/linux/include/linux/fortify-string.h:693:26: note: in expansion of macro ‘__fortify_memcpy_chk’
>   693 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
>       |                          ^~~~~~~~~~~~~~~~~~~~
> /dev/shm/jslaby/linux/include/crypto/sha256_base.h:69:17: note: in expansion of macro ‘memcpy’
>    69 |                 memcpy(sctx->buf + partial, data, len);
>       |                 ^~~~~~

The preprocessed file is attached. Build with:

$ gcc-13 -O2 -fstrict-flex-arrays=3  -mcmodel=large -S ../my/x.c -o /dev/null 
../my/x.c: In function ‘lib_sha256_base_do_update.constprop.isra’:
../my/x.c:26435:7: error: invalid 'asm': operand is not a condition code, invalid operand code 'c'
26435 |  do { asm volatile("1:\t"
      |       ^~~

$ gcc-13 --version
gcc-13 (SUSE Linux) 13.1.1 20230522 [revision dd36656ada05731c069ecd5b1878380294fb1f3e]
$ rpm -qi gcc13|grep Distr
Distribution: devel:gcc / SLE-15

That is on a SLE-15 machine.

On TW, the error is slightly different (why BTW?):
$ gcc-13 -O2 -fstrict-flex-arrays=3  -mcmodel=large -S /tmp/x.c -o /dev/null
/tmp/x.c: In function ‘lib_sha256_base_do_update.constprop.isra’:
/tmp/x.c:26435:7: warning: ‘asm’ operand 0 probably does not match constraints
26435 |  do { asm volatile("1:\t"
      |       ^~~
/tmp/x.c:26435:7: error: impossible constraint in ‘asm’

The same version there:
$ gcc-13 --version
gcc-13 (SUSE Linux) 13.1.1 20230522 [revision dd36656ada05731c069ecd5b1878380294fb1f3e]

In anyway, I assume this should work:
echo 'int main() { long addr; asm("leaq %c1 - ., %%rax\n\t" : "=a" (addr) : "i" (__FILE__)); return addr; }' | gcc-13 -O2 -fstrict-flex-arrays=3  -mcmodel=large -S -x c - -o /dev/null
<stdin>: In function ‘main’:
<stdin>:1:25: warning: ‘asm’ operand 1 probably does not match constraints
<stdin>:1:25: error: impossible constraint in ‘asm’


The documentation says about the c opcode modifier:
c  Require a constant operand and print the constant expression with no punctuation.

But apparently, the code  goes via ix86_print_operand()'s:
/* Meaning of CODE:
...
   C -- print opcode suffix for set/cmov insn.
   c -- like C, but print reversed condition
Comment 1 Jiri Slaby 2023-05-31 09:59:14 UTC
(In reply to Jiri Slaby from comment #0)
> The preprocessed file is attached. Build with:
> 
> $ gcc-13 -O2 -fstrict-flex-arrays=3  -mcmodel=large -S ../my/x.c -o
> /dev/null 

I forgot to add, using gcc 12 works:
gcc-12 -O2 -mcmodel=large -S ../my/x.c -o /dev/null 

Nevertheless, I bisected the problem to this kernel commit:
commit df8fc4e934c12b906d08050d7779f292b9c5c6b5 (HEAD, refs/bisect/bad)
Author: Kees Cook <keescook@chromium.org>
Date:   Wed May 17 16:18:11 2023 -0700

    kbuild: Enable -fstrict-flex-arrays=3

So it is likely that with -mcmodel=large we are exposing some #UB which is now exposed by -fstrict-flex-arrays=3. Andreas Schwab suggests __FILE__ is not a constant with -mcmodel=large. What is the proper opcode to make this work also under these circumstances?
Comment 2 Andreas Schwab 2023-05-31 10:09:43 UTC
So the bug is really in the x86 BUG macro, which is incompatible with -mcmodel=large, and -fstrict-flex-arrays=3 just triggers it for "field \"sctx->buf + partial\" at include/crypto/sha256_base.h:52".
Comment 3 Jiri Slaby 2023-05-31 10:59:00 UTC
Yes, there is a thread at:
https://lore.kernel.org/all/202305301658.BF6ECF65C@keescook/

And a patch:
https://lore.kernel.org/all/20230531003345.never.325-kees@kernel.org/

So this is invalid from gcc's POV.
Comment 4 Richard Biener 2023-05-31 11:01:56 UTC
(In reply to Jiri Slaby from comment #0)
> The documentation says about the c opcode modifier:
> c  Require a constant operand and print the constant expression with no
> punctuation.
> 
> But apparently, the code  goes via ix86_print_operand()'s:
> /* Meaning of CODE:
> ...
>    C -- print opcode suffix for set/cmov insn.
>    c -- like C, but print reversed condition

The main documentation applies to constant operands while when the operand
is not constant we go the machine specific way.  Generic code does

            else if (letter == 'c')
              {
                if (CONSTANT_ADDRESS_P (operands[opnum]))
                  output_addr_const (asm_out_file, operands[opnum]);
                else
                  output_operand (operands[opnum], 'c');

where output_operand uses the target interpretation of 'c'.  I agree this
is a bit confusing and Andreas is right that it's the asm()s fault since
with -mcmodel=large __FILE__ and friends are no longer "constant"
according to x86 definition of constant.

With -mcmodel=large __code_model_large__ is defined to 1 so maybe you can
fiddle with the asm to make it valid (but it's likely not machine specific?).
Comment 5 Michael Matz 2023-05-31 12:37:37 UTC
Or rather, you should simply stop using -mcmodel=large.  Noone should use it.
If you don't happen to have functions that _individually_ are larger than 2GB
-mcmodel=large is usually the wrong answer to whatever problem its addition
was intended to solve.
Comment 6 Jiri Slaby 2023-06-01 06:42:41 UTC
(In reply to Michael Matz from comment #5)
> Or rather, you should simply stop using -mcmodel=large.  Noone should use it.
> If you don't happen to have functions that _individually_ are larger than 2GB
> -mcmodel=large is usually the wrong answer to whatever problem its addition
> was intended to solve.

large was restored in:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e16c2983fba0fa6763e43ad10916be35e3d8dc05

apparently to avoid R_X86_64_32S relocations:
"""
This type of relocation doesn't work when kexec chooses to place the
purgatory binary in memory that is not reachable with 32 bit
addresses.
"""

Previously, it was present since the original purgatory implementation:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8fc5b4d4121c95482b2583a07863c6b0aba2d9e1

Any idea how to avoid/fix such a relocations other than by mcmodel=large?
Comment 7 Michael Matz 2023-06-01 12:25:30 UTC
(In reply to Jiri Slaby from comment #6)
> 
> large was restored in:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/
> ?id=e16c2983fba0fa6763e43ad10916be35e3d8dc05
> 
> apparently to avoid R_X86_64_32S relocations:
> """
> This type of relocation doesn't work when kexec chooses to place the
> purgatory binary in memory that is not reachable with 32 bit
> addresses.
> """

This sounds like the problem is that the binary blob in question might be loaded at arbitrary addresses in memory, and then references from within that binary to
stuff also within that library don't work anymore if the load-address happens to
be beyond 4GB.  If that was indeed the problem, then the proper solution would
have been to build that blob as position-independend code, similar to shared libraries.  It's not large, but must be fine with arbitrary load addresses.

I have no idea how to properly setup an environment where I would be able to test
this interaction between kexec and loading that blob, preferably with qemu.
If I had I could poke at it a bit to solve this "properly".
Comment 8 Jiri Slaby 2024-04-19 11:33:33 UTC
So at last this is being fixed:
https://lore.kernel.org/all/20240418201705.3673200-2-ardb+git@google.com/

-PURGATORY_CFLAGS := -mcmodel=large -ffreestanding -fno-zero-initialized-in-bss -g0
+PURGATORY_CFLAGS := -mcmodel=small -ffreestanding -fno-zero-initialized-in-bss -g0
+PURGATORY_CFLAGS += -fpic -fvisibility=hidden