assembler .space gives "error: invalid number of bytes" in LLVM 19 but works in 18

[kernigh]

I expect LLVM to assemble this x86-64 test case. It fails in today's git f719771,

$ clang --target=amd64-openbsd -c exam.s    
exam.s:38:9: error: invalid number of bytes
        .space  (p_nop - p_1st) - (. - q_1st), 0xcc
                ^

It also fails with the OpenBSD package of llvm-19.1.7p0, but succeeds with the OpenBSD package of llvm-18.1.8p3.

	.section .p
p_1st:
0:	pause
	lfence
	jmp	0b

	.section .q
q_1st:
	addl	11,%eax
	addl	22,%eax

q_cli:
	cli
0:	pause
	lfence
	jmp	0b

	.section .p
	.space	(q_cli - q_1st) - (. - p_1st), 0xcc
	cli

	.section .q
q_sti:
	sti

	.section .p
	.space	(q_sti - q_1st) - (. - p_1st), 0xcc
	sti
	addl	33,%eax
	addl	44,%eax
p_nop:
	nop

	.section .q
0:	pause
	lfence
	jmp	0b
	.space	(p_nop - p_1st) - (. - q_1st), 0xcc
	nop

This test case has 2 sections (.p and .q). Each .space should evaluate to .space 7, 0xcc. (Then cli in .p and cli in .q will have the same offset, and same for sti and nop.) LLVM 18 emits the correct .o file, for which llvm-objdump -d -j.p -j.q exam.o shows 3 groups of 7 bytes of 0xcc = int3.

I need a jmp 0b to reproduce the error in LLVM 19 or git. If I change the 1st jmp 0b (on line 5) to incl %eax (both instructions are 2 bytes), then the error goes away, and each .space inserts the correct 7 bytes.

The wrong "error: invalid number of bytes" comes from a check if (Size < 0) in llvm/lib/MC/MCAssembler.cpp. I modified LLVM 19 to print the Size in each check. For this test case, I got

• exam.s:19 if 7 < 0
• exam.s:38 if -15 < 0, error
• exam.s:27 if 7 < 0
• exam.s:19 if 7 < 0
• exam.s:27 if 7 < 0
• exam.s:38 if 7 < 0

It visited the check if (Size < 0) twice for each .space line. The 1st visit to line 38 got the wrong size -15 and gave the error, though the 2nd visit got the correct size 7. The 1st visits with line 38 before 27 are in the wrong order; you can't calculate the size at line 38 unless you know the size at line 27. After I changed the 1st jmp 0b to incl %eax, it visited 27 before 38.

[brad0]

@MaskRay @echristo

[kernigh]

I modified my test case by splitting line 38 .space (p_nop - p_1st) - (. - q_1st), 0xcc into 2 lines .space (p_nop - p_1st), 0xf8 and .space (. - q_1st), 0xf9. Then an assertion failed,

assertion "getContext().hadError() || OS.tell() - Start == getSectionAddressSize(*Sec)" failed: file "/home/gkoehler/park/llvm-project/llvm/lib/MC/MCAssembler.cpp", line 872, function "writeSectionData"
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: clang --target=amd64-openbsd -c exam.s
clang: error: clang integrated assembler command failed with exit code 134 (use -v to see invocation)
clang version 20.0.0git (git@github.com:llvm/llvm-project.git f719771f251d7c30eca448133fe85730f19a6bd1)
Target: amd64-unknown-openbsd
Thread model: posix
InstalledDir: /home/gkoehler/park/llvm-project/build/bin
Build config: +assertions
clang: note: diagnostic msg: Error generating preprocessed source(s) - no preprocessable inputs.

The exam.s for this assertion failure is

	.section .p
p_1st:
0:	pause
	lfence
	jmp	0b

	.section .q
q_1st:
	addl	11,%eax
	addl	22,%eax

q_cli:
	cli
0:	pause
	lfence
	jmp	0b

	.section .p
	.space	(q_cli - q_1st) - (. - p_1st), 0xcc
	cli

	.section .q
q_sti:
	sti

	.section .p
	.space	(q_sti - q_1st) - (. - p_1st), 0xcc
	sti
	addl	33,%eax
	addl	44,%eax
p_nop:
	nop

	.section .q
0:	pause
	lfence
	jmp	0b
	.space	(p_nop - p_1st), 0xf8
	.space	(. - q_1st), 0xf9
	nop

I had passed -DLLVM_ENABLE_ASSERTIONS=On to cmake. My configuration is,

exec cmake \
  -DCMAKE_DISABLE_FIND_PACKAGE_Backtrace=ON \
  -DCMAKE_DISABLE_FIND_PACKAGE_LibXml2=ON \
  -DLLVM_ENABLE_PIC=OFF -DLLVM_ENABLE_PROJECTS=clang\;lld \
  -DLLVM_DEFAULT_TARGET_TRIPLE=powerpc-unknown-openbsd \
  -DCMAKE_BUILD_TYPE="Release" -DLLVM_ENABLE_ASSERTIONS=On \
  -G Ninja ../llvm

[kernigh]

I suspect that llvm miscalculated p_nop - p_1st because p_nop was 22 below the correct value. This would wrongly place p_nop between the cli at exam.s:20 and the .space at exam.s:27. It's as if the size of lines 27 .space, 28 sti, 29 addl 33, 30 addl 44 was zero instead of the correct size.

[brad0]

cc @MaskRay

[echristo]

Hmm.. if this works in 18, but not 19, would you mind bisecting the repo? I could probably give a little more help if I knew which patch broke it off hand :)

[MaskRay]

A direct JMP/JCC can be encoded either in 2 bytes with a 8-bit relative offset or 6 bytes with a 32-bit relative offset (span-dependent instruction)
When used together with .space with a variable size, the assembler has to perform complex relaxation.

-mrelax-all assembles successfully but it is probably not what you want. You could replace jmp 0b with an assembler macro that expands to the hard-coded 2 bytes as a workaround.

[kernigh]

Hmm.. if this works in 18, but not 19, would you mind bisecting the repo?

Sorry, my build is too slow. My fastest builder is a 4-core AMD Zen+ desktop, which takes a few hours to build llvm-project. I count 19089 commits in release/18.x..release/19.x, which is a little over 2**14, so I might need 14 builds to bisect them. If I can do 4 builds per day, I might need 4 days. That's too long, so no, I won't bisect llvm.

-mrelax-all assembles successfully but it is probably not what you want.

Thanks for the hint. clang -mrelax-all changes the 1st amd64 jmp from 2 bytes eb f9 to 5 bytes eb f6 ff ff ff. I didn't know that jmp had 2 sizes. I can now insert prints like errs() << "line\n" and compare clang with and without -mrelax-all.

My test case has 3 jmps, but I don't need the last jmp to reproduce the failure, so here is a simplified test case with only 2 jmps,

	.section .p
p_1st:
0:	pause
	lfence
	jmp	0b

	.section .q
q_1st:
	addl	11,%eax
	addl	22,%eax

q_cli:
	cli
0:	pause
	lfence
	jmp	0b

	.section .p
	.space	(q_cli - q_1st) - (. - p_1st), 0xcc
	cli

	.section .q
q_sti:
	sti

	.section .p
	.space	(q_sti - q_1st) - (. - p_1st), 0xcc
	sti
	addl	33,%eax
p_nop:
	nop

	.section .q
	.space	(p_nop - p_1st) - (. - q_1st), 0xcc
	nop

I am not liking the MCAssember::ensureValid function (in llvm/lib/MC/). When my test case fails, I observe a recursion of ensureValid(section .p) calling ensureValid(section .q) calling ensureValid(section .p). This is part of the code,

void MCAssembler::ensureValid(MCSection &Sec) const {
  if (Sec.hasLayout())
    return;
  Sec.setHasLayout(true); ...
  for (MCFragment &F : Sec) { ...
    Offset += computeFragmentSize(F); ...
  }
}

My guess is,

1. Something calls ensureValid(.p).
2. The outer ensureValid(.p) computes the size of the 1st .space without trouble. (Each .space is its own fragment.)
3. The outer ensureValid(.p) tries to compute the size of the 2nd .space.
4. The 2nd space has (q_sti - q_1st) and calls ensureValid(.q) to find q_sti.
5. ensureValid(.q) tries to compute the size of the 3rd space.
6. The 3rd space has (p_nop - p_1st) and calls ensureValid(.p) to find p_nop.
7. The inner ensureValid(.p) sees Sec.hasLayout() and returns. This is wrong, because .p is not yet valid.
8. The 3rd space takes an invalid p_nop from .p, computes a negative size, and reports "error: invalid number of bytes".
9. ensureValid(.q) returns.
10. The outer ensureValid(.p) returns.

If I run clang -mrelax-all, then the outer ensureValid(.p) returns without ever calling ensureValid(.q).

[MaskRay]

ensureValid should be removed.

be5a845 removed ensureValid but the commit was reverted due to a code size regression I haven't analyzed yet.

The commit still applies cleanly and makes your test case work (BTW: you probably want to write .section .p,"ax" to set SHF_ALLOC | SHF_EXECINSTR)

[rnagy]

I've just finished bisecting and it is definitely 9d0754a causing the issue.

[kernigh]

I built llvm both with and without patch be5a845. I saw a code size regression in this amd64 jne,

-     9fb: 75 0e                                jne     0xa0b <yyparse+0x8cb>
+     9fb: 0f 85 0e 00 00 00                    jne     0xa0f <yyparse+0x8cf>

A short .s file with only the jne and the next few instructions doesn't reproduce the size regression. If I find time, I might make an .s that reproduces it.

[MaskRay]

The MCAssembler change 742ecfc ("[MC] Relax MCFillFragment and compute fragment offsets eagerly") has resolved the reproduce. This issue was only revealed by "[MC] Relax fragments eagerly".

// reproduce in the first comment
	.section .p
p_1st:
0:	pause
	lfence
	jmp	0b

	.section .q
q_1st:
	addl	11,%eax
	addl	22,%eax

q_cli:
	cli
0:	pause
	lfence
	jmp	0b

	.section .p
	.space	(q_cli - q_1st) - (. - p_1st), 0xcc
	cli

	.section .q
q_sti:
	sti

	.section .p
	.space	(q_sti - q_1st) - (. - p_1st), 0xcc
	sti
	addl	33,%eax
	addl	44,%eax
p_nop:
	nop

	.section .q
0:	pause
	lfence
	jmp	0b
	.space	(p_nop - p_1st) - (. - q_1st), 0xcc
	nop

Interesting relaxation tests
https://github.com/llvm/llvm-project/blob/main/llvm/test/MC/X86/relax-offset.s
https://github.com/llvm/llvm-project/blob/main/llvm/test/MC/ELF/layout-interdependency2.s