Prevent register clobbering on x86_64 threadlocal access #30202

On the x86_64 self hosted backend, thread locals are accessed through
__tls_get_addr on PIC. Usually this goes through a fast path which does
not lose any registers, however in some cases (notably any dlopened
library on my machine) this can take a slow path which calls out to C
ABI functions

Catch this case and backup registers as necessary

Fixes #30183

Some concerns I have with this PR, but don't know if are warranted due to unfamiliarity with the codebase

1. the knowledge of the __tls_get_addr call and it's behavior are now spread across multiple areas. I was kinda just going with the flow here copying the other spill list for coff/pic
2. I'm unsure if I should be pulling the register list from somewhere, right now I just wrote down the 6 I see in systemv's callconv
3. My gut says that the real correct answer is to re-use the logic in genCall() somehow. Like in this case this should be 1-1 to a C ABI fn call. I could see an argument that it's a very specific case though so dodging all that logic is fine

the knowledge of the __tls_get_addr call and it's behavior are now spread across multiple areas.

This is true, but I think it's fine for now. After all, the old (incorrect) clobber lists were also duplicating information about the TLS lowering.

I'm unsure if I should be pulling the register list from somewhere
[...]
My gut says that the real correct answer is to re-use the logic in genCall() somehow

The relevant lines of genCall are these two:

tryself.spillEflagsIfOccupied();tryself.spillCallerPreservedRegs(fn_info.cc,call_info.err_ret_trace_reg);

The first one is actually missing from TLS logic, that should probably also be there. But focusing on the second: because we're calling a C ABI function, we know that err_ret_trace_reg is .none (you can search through resolveCallingConventionValues to confirm that), and looking at the body of spillCallerPreservedRegs we see that this case is basically equivalent to self.spillRegisters(abi.getCallerPreservedRegs(cc_tag)); and here the calling convention is gonna be .x86_64_sysv, so we should be able to do cg.spillRegisters(abi.getCallerPreservedRegs(.x86_64_sysv)).

I just spent a little while looking at all of the current TLS lowerings and figuring out the correct clobbers. I haven't tested it even slightly, but I'm pretty sure this would be the fully-correct logic:

if(is_threadlocal)switch(cg.target.ofmt){.elf=>if(cg.mod.pic){// LD model: `__tls_get_addr` uses the standard ABItrycg.spillRegisters(abi.getCallerPreservedRegs(.x86_64_sysv));trycg.spillEflagsIfOccupied();}else{// LE model: manual lowering uses these two registerstrycg.spillRegisters(&.{.rdi,.rax});},.coff=>{// manual lowering uses these registerstrycg.spillRegisters(&.{.rdi,.rax});},.macho=>switch(cg.target.arch){.x86=>{// `tlv_get_addr` returns in eax, clobbers ecx, preserves other GPRstrycg.spillRegisters(&.{.rax,.rcx});trycg.spillEflagsIfOccupied();},.x86_64=>{// `tlv_get_addr` returns in rax, preserves other GPRstrycg.spillRegisters(&.{.rax});trycg.spillEflagsIfOccupied();},},else=>unreachable,};

(regarding that last Mach-O case, the backend doesn't really support arch == .x86 right now, but no harm working towards that support a little bit)

Thanks for taking a peek :)

The first one is actually missing from TLS logic, that should probably also be there

Oops, agreed.

because we're calling a C ABI function, we know that err_ret_trace_reg is .none

Ah yeah I thought it was more complicated and this block was related below as well

 if (call_info.err_ret_trace_reg != .none) {
 if (self.inst_tracking.getPtr(err_ret_trace_index)) |err_ret_trace| {
 if (switch (err_ret_trace.short) {
 .register => |reg| call_info.err_ret_trace_reg != reg,
 else => true,
 }) {
 try self.register_manager.getReg(call_info.err_ret_trace_reg, err_ret_trace_index);
 try reg_locks.append(self.register_manager.lockReg(call_info.err_ret_trace_reg));
 try self.genSetReg(call_info.err_ret_trace_reg, .usize, err_ret_trace.short, .{});
 err_ret_trace.trackMaterialize(err_ret_trace_index, .{
 .long = err_ret_trace.long,
 .short = .{ .register = call_info.err_ret_trace_reg },
 });
 }
 }
 }

but now I'm noticing the difference between call_info.err_ret_trace_index and self.err_ret_trace_reg. Somehow I got those tangled in my head

Agreed with your x86_64 impl, and choosing to trust you on the others. (with a s/target.arch/cpu.target.arch/ and an extra unreachable).

You missed the other two spillEflagsIfOccupied calls @sphaerophoria

😬I actually looked at them and for some reason they registered in my head as already correct. That's somehow more embarrassing than just forgetting. Sorry for wasting CI time