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

[clang code-gen] #pragma FENV_ACCESS leaking across template declarations/definitions #63063

Closed
nicolerabjohn opened this issue Jun 1, 2023 · 8 comments
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior clang:codegen platform:linux

Comments

@nicolerabjohn
Copy link
Contributor

nicolerabjohn commented Jun 1, 2023

We have encountered an assertion being hit in codegen relating to FENV_ACCESS:

Assertion failed: (CGF.CurFuncDecl == nullptr || CGF.Builder.getIsFPConstrained() || isa<CXXConstructorDecl>(CGF.CurFuncDecl) || isa<CXXDestructorDecl>(CGF.CurFuncDecl) || (NewExceptionBehavior == llvm::fp::ebIgnore && NewRoundingBehavior == llvm::RoundingMode::NearestTiesToEven)) && "FPConstrained should be enabled on entire function", file  clang/lib/CodeGen/CodeGenFunction.cpp, line 163, void clang::CodeGen::CodeGenFunction::CGFPOptionsRAII::ConstructorHelper(clang::FPOptions)()

This is reproducible with:

template <class> struct a {
  a(int, char);
  void b() { c() ?: 0; }
  bool c();
};
#pragma STDC FENV_ACCESS ON
void d() {
  a<char> e(1, 'j');
  e.b();
}

Basically, as it stands, it looks like when a template is declared under the scope of this pragma being set to one value, and then instantiated under the scope of this pragma being set to another, the value of the pragma at instantiation is taking precedence, which can cause this assertion as well as undefined behaviour when you flip the values. For example,

#pragma STDC FENV_ACCESS ON
template <typename>
int b() {
  int x;
  if ((float)0xFFFFFFFF != (float)0x100000000) {
    x = 1;
  }
  return x;
}
#pragma STDC FENV_ACCESS OFF
int f() { return b<void>(); }

exposes the same issue without the assertion being tripped. With Wuninitialized on, this shows as "uninitialized". As the definition is under the scope of this pragma being turned on, FENV_ACCESS should have the rounding mode set to dynamic, which would lead this expected warning to be "sometimes uninitialized". This proves that the pragma being set above the instantiation is taking precedence rather than the pragma above the definition being preserved.

FYI @hubert-reinterpretcast

@nicolerabjohn nicolerabjohn added bug Indicates an unexpected problem or unintended behavior clang:codegen platform:linux labels Jun 1, 2023
@nicolerabjohn nicolerabjohn self-assigned this Jun 1, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 1, 2023

@llvm/issue-subscribers-clang-codegen

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 1, 2023

@llvm/issue-subscribers-bug

@hubert-reinterpretcast
Copy link
Collaborator

hubert-reinterpretcast commented Jun 1, 2023

#pragma STDC FENV_ACCESS ON
template <typename>
int b() {
  int x;
  if ((float)0xFFFFFFFF != (float)0x100000000) {
    x = 1;
  }
  return x;
}
#pragma STDC FENV_ACCESS OFF
int f() { return b<void>(); }

It isn't just the warning. As https://godbolt.org/z/xoTa7zWeM shows, the possibility that the rounding mode causes the comparison to succeed is not reflected in the generated code.

@hubert-reinterpretcast
Copy link
Collaborator

hubert-reinterpretcast commented Jun 1, 2023

Basically, as it stands, it looks like when a template is declared under the scope of this pragma being set to one value, and then instantiated under the scope of this pragma being set to another, the value of the pragma at instantiation is taking precedence

I think it is more accurate to say that the value of the pragma at instantiation is used for some purposes but not others.

For example, with

#include <float.h>
#pragma STDC FENV_ACCESS ON
template <class> float b(float x) { return (float)(x*FLT_MAX)/FLT_MAX; }
#pragma STDC FENV_ACCESS OFF
void d(float x) { b<char>(x); }

we see the FENV_ACCESS ON being respected: https://godbolt.org/z/7aEEKnqxP

@AaronBallman
Copy link
Collaborator

CC @zahiraam and @rjmccall because this is currently being worked on in https://reviews.llvm.org/D152818

The currently final comment on that is:

I'd guess we're not resetting the pragma state appropriately when instantiating templates. Expressions should be governed by the pragmas in scope at the point of definition, not at the point of instantiation.

but it sounds like there may be more complexity based on what @hubert-reinterpretcast found.

@rjmccall
Copy link
Contributor

There's more complexity, but it's consistent with my diagnosis; it just means that some of our representations preserve information separately from the original context while others rely on the pragmas being set.

@spavloff
Copy link
Collaborator

#64605 (comment) describes the possible mechanism of the leaking.

@hubert-reinterpretcast
Copy link
Collaborator

Resolved by fde5924.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior clang:codegen platform:linux
Projects
None yet
Development

No branches or pull requests

6 participants