Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix 128bits ctlz intrinsincs UB #635

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dvermd
Copy link

@dvermd dvermd commented Feb 21, 2025

Fixes #604

This is the GIMPLE code generated

      if (param0 == 0) goto then; else goto else;
      then:
      zeros = 128;
      goto after;
      else:
      _2 = param0 >> 64;
      _3 = (size_t) _2;
      if (_3 != 0) goto ctlz_then; else goto ctlz_else;
      after:
      stack_var_1.15_4 = &stack_var_1;
      MEM[(unsigned int *)stack_var_1.15_4] = zeros;
      stack_var_1.16_5 = &stack_var_1;
      loadedValue2 = MEM[(unsigned int *)stack_var_1.16_5];
      D.4444 = loadedValue2;
      return D.4444;
      ctlz_then:
      _6 = param0 >> 64;
      _7 = (size_t) _6;
      _8 = __builtin_clzll (_7);
      zeros = (unsigned int) _8;
      goto ctlz_after;
      ctlz_else:
      _9 = (size_t) param0;
      _10 = __builtin_clzll (_9);
      _11 = (unsigned int) _10;
      zeros = _11 + 64;
      goto ctlz_after;
      ctlz_after:
      zeros = zeros;
      goto after;

@dvermd
Copy link
Author

dvermd commented Feb 21, 2025

It looks like the some CI jobs are failing on

thread 'rustc' panicked at src/int.rs:176:13:
assertion failed: a_type.dyncast_array().is_some()

which is some code I didn't touch.

Can my changes have some side effects ?
I see that #631 also have these jobs failing. Perhaps it's not my changes which bring these failures.

Any hints ?

@antoyo
Copy link
Contributor

antoyo commented Feb 21, 2025

Not sure it's related. I opened a new dummy PR to check if the tests pass, though.

@antoyo
Copy link
Contributor

antoyo commented Feb 21, 2025

The CI passed, so the problem is in here.

I suspect this is because you changed the return type: it used to return an array type and your change makes it return a u32.

@dvermd
Copy link
Author

dvermd commented Feb 23, 2025

The return type of the if width == 128 was already a u32 because the block ended with

                let res = self.context.new_array_access(self.location, result, index);

                return self.gcc_int_cast(res.to_rvalue(), result_type);

which will get an element in the array and cast it as result_type which is u32.

Edit: formatting

@dvermd
Copy link
Author

dvermd commented Feb 23, 2025

Ok, finally found the line pointing in my change let leading_zeroes = self.add(low_leading_zeroes, sixty_four); where low_leading_zeroesand sixty_four are of different types.

The problem I see is that the if branch should have been taken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UB in the implementation of the 128 bit ctlz intrinsic
2 participants