Bug 71962 - error: ‘((& x) != 0u)’ is not a constant expression
Summary: error: ‘((& x) != 0u)’ is not a constant expression
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 6.1.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: rejects-valid
: 67762 (view as bug list)
Depends on:
Blocks: constexpr
  Show dependency treegraph
 
Reported: 2016-07-21 18:04 UTC by Jonathan Wakely
Modified: 2024-03-25 15:40 UTC (History)
9 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail: 5.3.1, 6.1.1, 7.1.0, 8.0
Last reconfirmed: 2017-07-21 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2016-07-21 18:04:08 UTC
struct P {
  constexpr P(const int* p) : p(p) { }
  const int* p;
  explicit constexpr operator bool() const { return (bool)p; }
};

int main() {
  static constexpr int x{1};
  constexpr P p{&x};
  static_assert((bool)p, "");
}

This valid program is rejected when ubsan is used:

$ g++ ubsan.cc -fsanitize=undefined
ubsan.cc: In function ‘int main()’:
ubsan.cc:10:3: error: non-constant condition for static assertion
   static_assert((bool)p, "");
   ^~~~~~~~~~~~~
ubsan.cc:4:53: error: ‘((& x) != 0u)’ is not a constant expression
   explicit constexpr operator bool() const { return (bool)p; }
                                                     ^~~~~~~
Comment 1 Jakub Jelinek 2016-07-22 09:04:31 UTC
The thing is that -fsanitize=null (and a couple of other sanitizers) imply -fno-delete-null-pointer-checks, as it doesn't want all the checks it adds removed and fold obviously doesn't fold &var != NULL with -fno-delete-null-pointer-checks.
The testcase fails the same with -fno-delete-null-pointer-checks instead of -fsanitize=undefined.

Not sure what can be done here though?  Either change all places that check flag_delete_null_pointer_checks to use some complex predicate and not change
flag_delete_null_pointer_checks in
  if (opts->x_flag_sanitize & (SANITIZE_NULL | SANITIZE_NONNULL_ATTRIBUTE
                                | SANITIZE_RETURNS_NONNULL_ATTRIBUTE))
    opts->x_flag_delete_null_pointer_checks = 0;
and during constexpr evaluation temporarily clear flag_sanitize, or something similar.
Comment 2 Richard Biener 2016-07-22 11:21:14 UTC
Maybe the sanitizers could use sth not visible to the middle-end for null pointer tests, like a new IFN?
Comment 3 Jakub Jelinek 2016-07-22 11:30:41 UTC
But we still don't want to optimize aggressively based on assumed null pointer checks, after all, that is the whole point of the null sanitization.

Would
static inline bool
delete_null_pointer_checks_p ()
{
  return flag_delete_null_pointer_checks
         && (flag_sanitize & (SANITIZE_NULL | SANITIZE_NONNULL_ATTRIBUTE
                              | SANITIZE_RETURNS_NONNULL_ATTRIBUTE)) == 0;
}
and replacing most of flag_delete_null_pointer_checks with delete_null_pointer_checks_p () be that bad?
Then constexpr evaluation could temporarily clear flag_sanitize (it doesn't want any sanitization while doing constexpr folding).
Comment 4 Jakub Jelinek 2016-07-22 12:53:38 UTC
If we do that, the question is when to (temporarily) enable the null pointer check deletion (unless disabled explicitly with -fno-delete-null-pointer-checks or from the target defaults, like AVR ...).
Because I think there should be a difference between when some constant expression is being evaluated in constexpr contexts (then -fsanitize=undefined should not make a difference on those), and when it is just evaluated as constant expression as an optimization only (maybe_constant_value and the like for warning purposes, or during cp_fully_fold etc.).  In the latter case, I think we should just not fold it if there are the sanitizations etc.
Is ctx->quiet what is relevant, or something different?
Perhaps even the UBSAN_* ifns should not be ignored if just in optimization?
Comment 5 Jonathan Wakely 2017-07-21 12:29:10 UTC
A simpler test:

int main()
{
  static constexpr int x = 0;
  static_assert(bool(&x), "");
}

ubsan.cc: In function ‘int main()’:
ubsan.cc:4:3: error: non-constant condition for static assertion
   static_assert(bool(&x), "");
   ^~~~~~~~~~~~~
ubsan.cc:4:17: error: ‘((& x) != 0)’ is not a constant expression
   static_assert(bool(&x), "");
                 ^~~~~~~~
Comment 6 Martin Sebor 2017-07-21 18:57:23 UTC
The difference between success and failure is due to this bit of code in symtab.c:

  /* With !flag_delete_null_pointer_checks we assume that symbols may
     bind to NULL. This is on by default on embedded targets only.

     Otherwise all non-WEAK symbols must be defined and thus non-NULL or
     linking fails.  Important case of WEAK we want to do well are comdats.
     Those are handled by later check for definition.

     When parsing, beware the cases when WEAK attribute is added later.  */
  if (!DECL_WEAK (decl)
      && flag_delete_null_pointer_checks)
    {
      refuse_visibility_changes = true;
      return true;
    }

But the address of a static local variable can never be null so the test above is unnecessarily restrictive.  The following patch relaxes the test, letting GCC accept the test case even with -fsanitize=undefined.  I didn't spot any obvious failures in the test suite with it.

--- a/gcc/symtab.c
+++ b/gcc/symtab.c
@@ -1937,7 +1937,8 @@ symtab_node::nonzero_address ()
 
      When parsing, beware the cases when WEAK attribute is added later.  */
   if (!DECL_WEAK (decl)
-      && flag_delete_null_pointer_checks)
+      && (flag_delete_null_pointer_checks
+         || (TREE_STATIC (decl) && !DECL_EXTERNAL (decl))))
     {
       refuse_visibility_changes = true;
       return true;
Comment 7 Jakub Jelinek 2017-07-21 19:25:17 UTC
(In reply to Martin Sebor from comment #6)
> The difference between success and failure is due to this bit of code in
> symtab.c:
> 
>   /* With !flag_delete_null_pointer_checks we assume that symbols may
>      bind to NULL. This is on by default on embedded targets only.
> 
>      Otherwise all non-WEAK symbols must be defined and thus non-NULL or
>      linking fails.  Important case of WEAK we want to do well are comdats.
>      Those are handled by later check for definition.
> 
>      When parsing, beware the cases when WEAK attribute is added later.  */
>   if (!DECL_WEAK (decl)
>       && flag_delete_null_pointer_checks)
>     {
>       refuse_visibility_changes = true;
>       return true;
>     }
> 
> But the address of a static local variable can never be null so the test
> above is unnecessarily restrictive.  The following patch relaxes the test,
> letting GCC accept the test case even with -fsanitize=undefined.  I didn't
> spot any obvious failures in the test suite with it.

-fno-delete-null-pointer-checks is an option that allows violating the C requirements and a user variable can live at address 0.  That is something people need on various DSPs etc. where e.g. the data or rodata section can start at NULL.  So this change is wrong.
Whether automatic variables can have NULL addresses is a separate question.

Now, as I said before, we could change the options handling not to force flag_delete_null_pointer_checks = 0; for the sanitizers and review the places that check flag_delete_null_pointer_checks and in most of them check also the corresponding sanitizer options. then we could on a case by case decide what is and what isn't possible.
Comment 8 Martin Sebor 2017-07-21 20:08:37 UTC
(In reply to Jakub Jelinek from comment #7)

The null pointer check inserted by the sanitizer is eventually removed (see below) so there's obviously no point in emitting it to begin with.  The DSP case you mention simply isn't important to worry about, and certainly not worth pessimizing the common case for.  If there really were a need to handle this corner case then it should be provided as a separate feature, under its own option, and be disabled by default (even with -fsantize=undefined).  It should also be tested, which judging by the absence of test suite failures with the patch, it currently isn't.

$ cat a.C && gcc -O2 -S -Wall -Wpedantic -fsanitize=undefined -fdump-tree-ubsan=/dev/stdout -fdump-tree-optimized=/dev/stdout a.C
int f ()
{
  static int i = 1;

  int *p = &i;

  return *p;
}
  

;; Function int f() (_Z1fv, funcdef_no=0, decl_uid=2604, cgraph_uid=0, symbol_order=1)

Introduced new external node (long unsigned int __builtin_object_size(const void*, int)/2).

Symbols to be put in SSA form
{ D.2610 }
Incremental SSA update started at block: 0
Number of blocks in CFG: 3
Number of blocks to update: 2 ( 67%)


int f() ()
{
  int * p;
  static int i = 1;
  int _3;
  long unsigned int _4;

  <bb 2> [0.00%] [count: INV]:
  p_1 = &i;
  UBSAN_NULL (p_1, 0B, 4);
  _4 = __builtin_object_size (p_1, 0);
  UBSAN_OBJECT_SIZE (p_1, 4, _4, 0);
  _3 = *p_1;
  return _3;

}



;; Function int f() (_Z1fv, funcdef_no=0, decl_uid=2604, cgraph_uid=0, symbol_order=1)

Removing basic block 4
Merging blocks 2 and 3
int f() ()
{
  static int i = 1;
  int _2;

  <bb 2> [100.00%] [count: INV]:
  _2 = i;
  return _2;

}
Comment 9 S. Davis Herring 2020-03-30 21:54:53 UTC
Another very similar failure (let me know if you want a separate bug for it):

  inline constexpr int x=0,y=0;  // -std=c++17
  static_assert(&x!=&y);  // error: '((& x) != (& y))' is not a constant expression

No failure without the 'inline'.
Comment 10 Andrew Pinski 2022-12-31 21:44:06 UTC
*** Bug 67762 has been marked as a duplicate of this bug. ***
Comment 11 Andrew Pinski 2023-06-30 04:32:17 UTC
*** Bug 110493 has been marked as a duplicate of this bug. ***
Comment 12 Barry Revzin 2024-03-25 15:40:35 UTC
Similar failure:

struct A {
    void f();
};

int main() {
    constexpr auto pmf = &A::f;
    static_assert(pmf != nullptr); // error with UBSAN only
}

This surfaces from attempting to implement function_ref (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p0792r14.html) which has a constructor that takes the callable as a non-type template parameter and static_asserts that it's not a null pointer.

Which apparently doesn't work with UBSAN.