Sync from rust 2025/06/02 #694

antoyo · 2025-06-02T18:36:27Z

TODO:

Stop skipping test_total_cmp
"Remove this code" in common.rs
Use get_simple_function_f128 for fmodf128?
Only call get_simple_function_f128 and get_simple_function_f128_2args when it is the symbols for the supported f128 builtins?
Failing core tests:
- dyn_type_name
- test_maybe_uninit_short

- Rename `USED` to `USED_COMPILER` to better reflect its behavior. - Reorder some items to group the used and allocator flags together - Renumber them without gaps

…a itself

There is no safety contract and I don't think any of them can actually cause UB in more ways than passing malicious source code to rustc can. While LtoModuleCodegen::optimize says that the returned ModuleCodegen points into the LTO module, the LTO module has already been dropped by the time this function returns, so if the returned ModuleCodegen indeed points into the LTO module, we would have seen crashes on every LTO compilation, which we don't. As such the comment is outdated.

atomic_load intrinsic: use const generic parameter for ordering We have a gazillion intrinsics for the atomics because we encode the ordering into the intrinsic name rather than making it a parameter. This is particularly bad for those operations that take two orderings. Let's fix that! This PR only converts `load`, to see if there's any feedback that would fundamentally change the strategy we pursue for the const generic intrinsics. The first two commits are preparation and could be a separate PR if you prefer. `@BoxyUwU` -- I hope this is a use of const generics that is unlikely to explode? All we need is a const generic of enum type. We could funnel it through an integer if we had to but an enum is obviously nicer... `@bjorn3` it seems like the cranelift backend entirely ignores the ordering?

This avoids having to get the function signature.

FractalFir · 2025-06-26T19:30:23Z

Out of curiosity: could I help with this, somehow?

antoyo · 2025-06-26T19:40:40Z

Yes, if you are interested in debugging this.

My hunch was that the first issue to fix is related to this hack.
I wanted to fix cg_ssa in order to not require it anymore, but the MCP process is not done yet.

FractalFir · 2025-06-27T18:26:15Z

Reduced dyn_type_name issue to this:

#![feature(core_intrinsics)]
extern crate core;
pub const fn u32_type_name() -> &'static str {
    core::intrinsics::type_name::<u32>()
}
fn main() {
    let s = u32_type_name();
    assert!(s.len() < 100, "{}",s.len());   
}

The assert will fail with a garbage length.

thread 'main' panicked at test.rs:10:5:
94096628601670

Calling the intrinsic directly works fine, as does building with MIR opt level=2.

Seems like if this function gets inlined, everything works out fine.

Maybe an ABI bug?

EDIT: the slice in question usually points to invalid memory, so both elements seem corrupted.

Here is the GIMPLE of u32_type_name in the synced version:

__attribute__((visibility ("default")))
struct struct u32_type_name ()
{
  struct struct D.4023;
  struct struct undefined;
  size_t loadedValue3;
  signed char * loadedValue2;
  struct struct undefined;
  signed char stack_var_1[16];

  try
    {
      start:
      undefined.field0_TODO = &global.11;
      undefined.field1_TODO = 3;
      stack_var_1.7_1 = &stack_var_1;
      MEM[(struct struct * *)stack_var_1.7_1] = &undefined;
      stack_var_1.8_2 = &stack_var_1;
      loadedValue2 = MEM[(signed char * *)stack_var_1.8_2];
      _3 = &stack_var_1 + 8;
      loadedValue3 = MEM[(size_t *)_3];
      undefined.field0_TODO = loadedValue2;
      undefined.field1_TODO = loadedValue3;
      D.4023 = undefined;
      return D.4023;
    }
  finally
    {
      undefined = {CLOBBER(eos)};
      undefined = {CLOBBER(eos)};
      stack_var_1 = {CLOBBER(eos)};
    }
}

And here is the working version, which seems to be considerably longer.

__attribute__((visibility ("default")))
struct struct u32_type_name ()
{
  struct struct D.4022;
  struct struct undefined;
  size_t loadedValue3;
  signed char * loadedValue2;
  struct struct undefined;
  signed char stack_var_1[16];

  try
    {
      start:
      undefined.field0_TODO = &global.11;
      undefined.field1_TODO = 3;
      stack_var_1.7_1 = &stack_var_1;
      _2 = undefined.field0_TODO;
      MEM[(signed char * *)stack_var_1.7_1] = _2;
      _3 = &stack_var_1 + 8;
      _4 = undefined.field1_TODO;
      MEM[(size_t *)_3] = _4;
      stack_var_1.8_5 = &stack_var_1;
      loadedValue2 = MEM[(signed char * *)stack_var_1.8_5];
      _6 = &stack_var_1 + 8;
      loadedValue3 = MEM[(size_t *)_6];
      undefined.field0_TODO = loadedValue2;
      undefined.field1_TODO = loadedValue3;
      D.4022 = undefined;
      return D.4022;
    }
  finally
    {
      undefined = {CLOBBER(eos)};
      undefined = {CLOBBER(eos)};
      stack_var_1 = {CLOBBER(eos)};
    }
}

FractalFir · 2025-06-27T21:38:11Z

I think I understand the issue now... at least partially. If we count memory reads / writes, we'll see an interesting difference.

In the new version, we perform 1 write of type struct struct **, and 2 reads of different kind of pointers.
We thus only copy half of the fat pointer, leaving rest undefined.

We also... copy the pointer to the structure, instead of the structure itself, for some reason?

 undefined.field1_TODO = 3;
      stack_var_1.7_1 = &stack_var_1;
      MEM[(struct struct * *)stack_var_1.7_1] = &undefined;
      stack_var_1.8_2 = &stack_var_1;
      loadedValue2 = MEM[(signed char * *)stack_var_1.8_2];
      _3 = &stack_var_1 + 8;
      loadedValue3 = MEM[(size_t *)_3];
      undefined.field0_TODO = loadedValue2;
      undefined.field1_TODO = loadedValue3;

The old code performed 2 writes, 2 reads. It copied both the metadata and the data pointer.

 stack_var_1.7_1 = &stack_var_1;
      _2 = undefined.field0_TODO;
      MEM[(signed char * *)stack_var_1.7_1] = _2;
      _3 = &stack_var_1 + 8;
      _4 = undefined.field1_TODO;
      MEM[(size_t *)_3] = _4;
      stack_var_1.8_5 = &stack_var_1;
      loadedValue2 = MEM[(signed char * *)stack_var_1.8_5];
      _6 = &stack_var_1 + 8;
      loadedValue3 = MEM[(size_t *)_6];

My guess here is this: the write was supposed to be of type struct struct*(writing struct struct to a stack var) but somebody accidentally changed it to struct struct**(writing the adress of the intrinsic result to a stack_var).

I don't yet know why this is broken, but at least I know what is broken.

antoyo · 2025-06-27T21:55:48Z

We also... copy the pointer to the structure, instead of the structure itself, for some reason?

Yeah, this is also what I observed.
With the hack I mentioned above, we will add dereference in some cases like here. From what I remembered when I looked at this, we would need to also dereference somewhere else, via the same hack to fix this issue.

All of this is very brittle, which is why I would want to fix cg_ssa to remove this hack at some point in the future.

FractalFir · 2025-06-27T21:59:57Z

We also... copy the pointer to the structure, instead of the structure itself, for some reason?

Yeah, this is also what I observed. With the hack I mentioned above, we will add dereference in some cases like here. From what I remembered when I looked at this, we would need to also dereference somewhere else, via the same hack to fix this issue.

All of this is very brittle, which is why I would want to fix cg_ssa to remove this hack at some point in the future.

What does const_undef even do, out of curiosity?

FractalFir · 2025-06-27T22:01:37Z

Why is it creating a local var? Should it not create a constant instead?

antoyo · 2025-06-27T22:02:21Z

Why is it creating a local var? Should it not create a constant instead?

In GCC, we can only assign to an LValue, which is why a variable is created.

antoyo · 2025-06-27T22:05:04Z

What does const_undef even do, out of curiosity?

The doc-comment on this function mentions:

    /// Generate an uninitialized value (matching uninitialized memory in MIR).
    /// Whether memory is initialized or not is tracked byte-for-byte.

FractalFir · 2025-06-27T22:10:44Z

I was more so speaking about what it does on LLVM level, and what does Rustc expect from it. What type is it supposed to return(pointer to a value or a value?), etc.

Now I understand. We are trying to replicate:

 %2 = insertvalue { ptr, i64 } poison, ptr %_0.0, 0, !dbg !241
  %3 = insertvalue { ptr, i64 } %2, i64 %_0.1, 1, !dbg !241

In GCC. However, we cant do so, because we cant add a field to poison, in Builder::insert_value.

FractalFir · 2025-06-28T15:52:35Z

I am having a bit of a bad(?) idea. Could we first try syncing to an ealier version of nightly, just to see at what point things broke?

We know that something went wrong between 2025/05/21 and 2025/06/02 - that is over 10 days. Since nighties are supposed to be daily, we could see how far we can go before things break.

At that point, we'd know something broke at a certain day, and have an easier time figuring out what went wrong. We would only have a day or so of changes to comb trough.

@antoyo if I managed to sync to an earlier date(for example, 2025/05/30) would you be down to merging that?

antoyo · 2025-06-28T16:00:18Z

Yes, I would be down to merge that. The instructions for that can be found here.

FractalFir · 2025-06-28T17:36:56Z

Yes, I would be down to merge that. The instructions for that can be found here.

Those instructions don't work for me anymore: I can't build the patched version of git. thread_local is now a C macro, and not a valid type name.

// in builtin/index-pack.c
struct thread_local {
	pthread_t thread;
	struct base_data *base_cache;
	size_t base_cache_used;
	int pack_fd;
};

https://www.gnu.org/software/libc/manual/html_node/ISO-C-Thread_002dlocal-Storage.html

EDIT: unrechable is not a valid function name:

static int unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit, struct object_id *oid)

I will work around this, but I thought I will warn you that this may become an issue with future syncs.

antoyo · 2025-06-28T18:06:55Z

It seems like bjorn might have a forked of git as well: https://github.com/rust-lang/rustc_codegen_cranelift/blob/b2cafc96ec61a2bfa43caf1ef68be0cc348e9140/scripts/rustup.sh#L10
Maybe it works.

antoyo · 2025-06-28T19:18:02Z

@FractalFir: I believe I found a fix and should be able to merge this PR soon.

Noratrieb and others added 19 commits May 24, 2025 20:31

Cleanup CodegenFnAttrFlags

8ad6957

- Rename `USED` to `USED_COMPILER` to better reflect its behavior. - Reorder some items to group the used and allocator flags together - Renumber them without gaps

Pass PlaceRef rather than Bx::Value to codegen_intrinsic_call

0348509

Remove usage of FnAbi in codegen_intrinsic_call

f994713

Remove codegen_unit from MiscCodegenMethods

f6489c3

Remove a couple of uses of interior mutability around statics

4d47f6d

Make predefine methods take &mut self

f297400

Remove methods from StaticCodegenMethods that are not called in cg_ss…

96be6ef

…a itself

Remove unused arg_memory_ty method

0c6f946

get rid of rustc_codegen_ssa::common::AtomicOrdering

dbbfb6e

Use layout field of OperandRef and PlaceRef in codegen_intrinsic_call

5c4b797

This avoids having to get the function signature.

Use layout field of OperandRef in generic_simd_intrinsic

926c8c8

Avoid computing function type for intrinsic instances

6c6c31d

Directly use from_immediate for handling bool

f383b17

Merge branch 'master' into sync_from_rust_2025_06_02

f69d8fc

Update to nightly-2025-06-02

2d7d0ee

Remove unneeded patch

38f134c

Make comment more specific

2e89179

antoyo added 5 commits June 28, 2025 14:44

Fix type_name intrinsic

7c71c83

Merge branch 'master' into sync_from_rust_2025_06_02

5735bb3

Fix to use Function instead of RValue

1449894

Fix for libgccjit 12

eb4d429

Ignore failing test

c7c1622

antoyo added 3 commits June 28, 2025 15:38

Remove commented code

49e65a0

Add TODOs

f876b18

Stop skipping libcore's f16::test_total_cmp

8385f3c

antoyo merged commit b7091ec into master Jun 28, 2025
38 checks passed

antoyo deleted the sync_from_rust_2025_06_02 branch June 28, 2025 20:02

Sync from rust 2025/06/02 #694

Sync from rust 2025/06/02 #694

Uh oh!

Conversation

antoyo commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FractalFir commented Jun 26, 2025

Uh oh!

antoyo commented Jun 26, 2025

Uh oh!

FractalFir commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FractalFir commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antoyo commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FractalFir commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FractalFir commented Jun 27, 2025

Uh oh!

antoyo commented Jun 27, 2025

Uh oh!

antoyo commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FractalFir commented Jun 27, 2025

Uh oh!

FractalFir commented Jun 28, 2025

Uh oh!

antoyo commented Jun 28, 2025

Uh oh!

FractalFir commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antoyo commented Jun 28, 2025

Uh oh!

antoyo commented Jun 28, 2025

Uh oh!

Uh oh!

Uh oh!

antoyo commented Jun 2, 2025 •

edited

Loading

FractalFir commented Jun 27, 2025 •

edited

Loading

FractalFir commented Jun 27, 2025 •

edited

Loading

antoyo commented Jun 27, 2025 •

edited

Loading

FractalFir commented Jun 27, 2025 •

edited

Loading

antoyo commented Jun 27, 2025 •

edited

Loading

FractalFir commented Jun 28, 2025 •

edited

Loading